Skip to content

Commit

Permalink
Creating Elliptic Curve key sometimes provides invalid values (#1025)
Browse files Browse the repository at this point in the history
- Normalizes EC key X and Y parameters by removing extra leading 0 which is only responsible for indicating that the BigInteger is positive

Updates #1024
{patch}

Signed-off-by: Esta Nagy <nagyesta@gmail.com>
  • Loading branch information
nagyesta committed Jun 14, 2024
1 parent a4b2fcd commit 7d19cde
Show file tree
Hide file tree
Showing 14 changed files with 129 additions and 257 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.Setter;
import lombok.ToString;

import java.util.List;

@Getter
@EqualsAndHashCode
@ToString
public class KeyBackupList implements BackupListContainer<KeyBackupListItem> {

@Valid
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.security.KeyPair;
import java.security.interfaces.ECPrivateKey;
import java.security.interfaces.ECPublicKey;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.Callable;

Expand Down Expand Up @@ -51,12 +52,12 @@ public KeyCreationInput<?> keyCreationInput() {

@Override
public byte[] getX() {
return ((ECPublicKey) getKey().getPublic()).getW().getAffineX().toByteArray();
return normalizeKeyParameter(((ECPublicKey) getKey().getPublic()).getW().getAffineX().toByteArray());
}

@Override
public byte[] getY() {
return ((ECPublicKey) getKey().getPublic()).getW().getAffineY().toByteArray();
return normalizeKeyParameter(((ECPublicKey) getKey().getPublic()).getW().getAffineY().toByteArray());
}

@Override
Expand Down Expand Up @@ -111,4 +112,16 @@ protected byte[] postProcessGeneratedSignature(final byte[] signature) throws Ex
protected byte[] preProcessVerifiableSignature(final byte[] rawSignature) throws Exception {
return Asn1ConverterUtil.convertFromRawToAsn1(rawSignature);
}

private byte[] normalizeKeyParameter(final byte[] byteArray) {
final int expectedLength = getKeyParam().getByteLength();
final int actualLength = byteArray.length;
//if the actual length is larger, then there is a leading 0 byte in front of the actual value
//this is added only because the next byte would be negative and the BigInteger would be negative as well
if (actualLength > expectedLength) {
return Arrays.copyOfRange(byteArray, actualLength - expectedLength, actualLength);
} else {
return byteArray;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
* @param <T> The type of the key.
* @param <S> The type of the key parameter.
*/
@SuppressWarnings("LombokGetterMayBeUsed")
public abstract class KeyVaultKeyEntity<T, S> extends KeyVaultBaseEntity<VersionedKeyEntityId> implements ReadOnlyKeyVaultKeyEntity {

@Getter
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package com.github.nagyesta.lowkeyvault.controller;

import com.github.nagyesta.lowkeyvault.model.common.backup.KeyBackupListItem;
import com.github.nagyesta.lowkeyvault.model.common.backup.KeyBackupModel;
import com.github.nagyesta.lowkeyvault.model.v7_2.common.constants.RecoveryLevel;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.KeyPropertiesModel;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.KeyVaultKeyModel;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.constants.KeyCurveName;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.constants.KeyOperation;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.constants.KeyType;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.request.JsonWebKeyImportRequest;
import com.github.nagyesta.lowkeyvault.service.key.id.VersionedKeyEntityId;
import com.github.nagyesta.lowkeyvault.service.key.util.KeyGenUtil;
import org.junit.jupiter.api.Assertions;

import java.net.URI;
import java.security.KeyPair;
import java.security.interfaces.ECPrivateKey;
import java.security.interfaces.ECPublicKey;
import java.security.spec.ECPoint;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

import static com.github.nagyesta.lowkeyvault.TestConstants.*;
import static com.github.nagyesta.lowkeyvault.TestConstantsKeys.KEY_NAME_1;

public class BaseKeyBackupRestoreControllerIntegrationTest {

@SuppressWarnings({"checkstyle:VisibilityModifier", "checkstyle:JavadocVariable"})
protected URI uri;

protected void assertRestoredKeyMatchesExpectations(
final KeyVaultKeyModel actualBody, final ECPublicKey publicKey,
final String version, final Map<String, String> expectedTags) {
Assertions.assertEquals(new VersionedKeyEntityId(uri, KEY_NAME_1, version).asUri(uri).toString(), actualBody.getKey().getId());
Assertions.assertEquals(KeyCurveName.P_256, actualBody.getKey().getCurveName());
Assertions.assertEquals(KeyType.EC, actualBody.getKey().getKeyType());
Assertions.assertIterableEquals(List.of(KeyOperation.SIGN, KeyOperation.VERIFY), actualBody.getKey().getKeyOps());
final byte[] expectedX = normalize(publicKey.getW().getAffineX().toByteArray(), KeyCurveName.P_256.getByteLength());
Assertions.assertArrayEquals(expectedX, actualBody.getKey().getX());
final byte[] expectedY = normalize(publicKey.getW().getAffineY().toByteArray(), KeyCurveName.P_256.getByteLength());
Assertions.assertArrayEquals(expectedY, actualBody.getKey().getY());
//do not return private key material in response
Assertions.assertNull(actualBody.getKey().getD());
Assertions.assertEquals(TIME_10_MINUTES_AGO, actualBody.getAttributes().getCreatedOn());
Assertions.assertEquals(NOW, actualBody.getAttributes().getUpdatedOn());
Assertions.assertEquals(TIME_IN_10_MINUTES, actualBody.getAttributes().getNotBefore());
Assertions.assertEquals(TIME_IN_10_MINUTES.plusDays(1), actualBody.getAttributes().getExpiresOn());
Assertions.assertEquals(RecoveryLevel.RECOVERABLE_AND_PURGEABLE, actualBody.getAttributes().getRecoveryLevel());
Assertions.assertEquals(RecoveryLevel.MAX_RECOVERABLE_DAYS_INCLUSIVE, actualBody.getAttributes().getRecoverableDays());
Assertions.assertTrue(actualBody.getAttributes().isEnabled());
Assertions.assertEquals(expectedTags, actualBody.getTags());
}

protected KeyPair addVersionToList(final URI baseUri, final String name, final String version,
final KeyBackupModel backupModel, final Map<String, String> tags) {
final KeyPair keyPair = KeyGenUtil.generateEc(KeyCurveName.P_256);
final JsonWebKeyImportRequest keyMaterial = new JsonWebKeyImportRequest();
keyMaterial.setKeyType(KeyType.EC);
keyMaterial.setCurveName(KeyCurveName.P_256);
keyMaterial.setKeyOps(List.of(KeyOperation.SIGN, KeyOperation.VERIFY));
keyMaterial.setD(((ECPrivateKey) keyPair.getPrivate()).getS().toByteArray());
final ECPoint w = ((ECPublicKey) keyPair.getPublic()).getW();
keyMaterial.setX(normalize(w.getAffineX().toByteArray(), KeyCurveName.P_256.getByteLength()));
keyMaterial.setY(normalize(w.getAffineY().toByteArray(), KeyCurveName.P_256.getByteLength()));
keyMaterial.setId(new VersionedKeyEntityId(baseUri, name, version).asUri(uri).toString());
final KeyBackupListItem listItem = new KeyBackupListItem();
listItem.setKeyMaterial(keyMaterial);
listItem.setVaultBaseUri(baseUri);
listItem.setId(name);
listItem.setVersion(version);
final KeyPropertiesModel propertiesModel = new KeyPropertiesModel();
propertiesModel.setCreatedOn(TIME_10_MINUTES_AGO);
propertiesModel.setUpdatedOn(NOW);
propertiesModel.setNotBefore(TIME_IN_10_MINUTES);
propertiesModel.setExpiresOn(TIME_IN_10_MINUTES.plusDays(1));
propertiesModel.setRecoveryLevel(RecoveryLevel.RECOVERABLE_AND_PURGEABLE);
propertiesModel.setRecoverableDays(RecoveryLevel.MAX_RECOVERABLE_DAYS_INCLUSIVE);
listItem.setAttributes(propertiesModel);
listItem.setTags(tags);
final List<KeyBackupListItem> list = new ArrayList<>(backupModel.getValue().getVersions());
list.add(listItem);
backupModel.getValue().setVersions(list);
return keyPair;
}

private byte[] normalize(final byte[] bytes, final int expectedLength) {
if (expectedLength < bytes.length) {
return Arrays.copyOfRange(bytes, bytes.length - expectedLength, bytes.length);
} else {
return bytes;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,19 @@

import com.github.nagyesta.abortmission.booster.jupiter.annotation.LaunchAbortArmed;
import com.github.nagyesta.lowkeyvault.TestConstantsUri;
import com.github.nagyesta.lowkeyvault.controller.BaseKeyBackupRestoreControllerIntegrationTest;
import com.github.nagyesta.lowkeyvault.mapper.common.registry.KeyConverterRegistry;
import com.github.nagyesta.lowkeyvault.model.common.backup.KeyBackupList;
import com.github.nagyesta.lowkeyvault.model.common.backup.KeyBackupListItem;
import com.github.nagyesta.lowkeyvault.model.common.backup.KeyBackupModel;
import com.github.nagyesta.lowkeyvault.model.v7_2.common.constants.RecoveryLevel;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.KeyPropertiesModel;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.KeyVaultKeyModel;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.constants.KeyCurveName;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.constants.KeyOperation;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.constants.KeyType;
import com.github.nagyesta.lowkeyvault.model.v7_2.key.request.JsonWebKeyImportRequest;
import com.github.nagyesta.lowkeyvault.service.exception.NotFoundException;
import com.github.nagyesta.lowkeyvault.service.key.KeyVaultFake;
import com.github.nagyesta.lowkeyvault.service.key.id.VersionedKeyEntityId;
import com.github.nagyesta.lowkeyvault.service.key.impl.EcKeyCreationInput;
import com.github.nagyesta.lowkeyvault.service.key.impl.KeyCreateDetailedInput;
import com.github.nagyesta.lowkeyvault.service.key.util.KeyGenUtil;
import com.github.nagyesta.lowkeyvault.service.vault.VaultService;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
Expand All @@ -35,11 +31,7 @@

import java.net.URI;
import java.security.KeyPair;
import java.security.interfaces.ECPrivateKey;
import java.security.interfaces.ECPublicKey;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.stream.Stream;

Expand All @@ -49,14 +41,13 @@

@LaunchAbortArmed
@SpringBootTest
class KeyBackupRestoreControllerIntegrationTest {
class KeyBackupRestoreControllerIntegrationTest extends BaseKeyBackupRestoreControllerIntegrationTest {

@Autowired
@Qualifier("KeyBackupRestoreControllerV72")
private KeyBackupRestoreController underTest;
@Autowired
private VaultService vaultService;
private URI uri;

public static Stream<Arguments> nullProvider() {
return Stream.<Arguments>builder()
Expand Down Expand Up @@ -229,55 +220,4 @@ void testBackupEntityShouldReturnTheOriginalBackupModelWhenCalledAfterRestoreEnt
Assertions.assertEquals(HttpStatus.OK, actual.getStatusCode());
}

private void assertRestoredKeyMatchesExpectations(
final KeyVaultKeyModel actualBody, final ECPublicKey publicKey,
final String version, final Map<String, String> expectedTags) {
Assertions.assertEquals(new VersionedKeyEntityId(uri, KEY_NAME_1, version).asUri(uri).toString(), actualBody.getKey().getId());
Assertions.assertEquals(KeyCurveName.P_256, actualBody.getKey().getCurveName());
Assertions.assertEquals(KeyType.EC, actualBody.getKey().getKeyType());
Assertions.assertIterableEquals(List.of(KeyOperation.SIGN, KeyOperation.VERIFY), actualBody.getKey().getKeyOps());
Assertions.assertArrayEquals(publicKey.getW().getAffineX().toByteArray(), actualBody.getKey().getX());
Assertions.assertArrayEquals(publicKey.getW().getAffineY().toByteArray(), actualBody.getKey().getY());
//do not return private key material in response
Assertions.assertNull(actualBody.getKey().getD());
Assertions.assertEquals(TIME_10_MINUTES_AGO, actualBody.getAttributes().getCreatedOn());
Assertions.assertEquals(NOW, actualBody.getAttributes().getUpdatedOn());
Assertions.assertEquals(TIME_IN_10_MINUTES, actualBody.getAttributes().getNotBefore());
Assertions.assertEquals(TIME_IN_10_MINUTES.plusDays(1), actualBody.getAttributes().getExpiresOn());
Assertions.assertEquals(RecoveryLevel.RECOVERABLE_AND_PURGEABLE, actualBody.getAttributes().getRecoveryLevel());
Assertions.assertEquals(RecoveryLevel.MAX_RECOVERABLE_DAYS_INCLUSIVE, actualBody.getAttributes().getRecoverableDays());
Assertions.assertTrue(actualBody.getAttributes().isEnabled());
Assertions.assertEquals(expectedTags, actualBody.getTags());
}

private KeyPair addVersionToList(final URI baseUri, final String name, final String version,
final KeyBackupModel backupModel, final Map<String, String> tags) {
final KeyPair keyPair = KeyGenUtil.generateEc(KeyCurveName.P_256);
final JsonWebKeyImportRequest keyMaterial = new JsonWebKeyImportRequest();
keyMaterial.setKeyType(KeyType.EC);
keyMaterial.setCurveName(KeyCurveName.P_256);
keyMaterial.setKeyOps(List.of(KeyOperation.SIGN, KeyOperation.VERIFY));
keyMaterial.setD(((ECPrivateKey) keyPair.getPrivate()).getS().toByteArray());
keyMaterial.setX(((ECPublicKey) keyPair.getPublic()).getW().getAffineX().toByteArray());
keyMaterial.setY(((ECPublicKey) keyPair.getPublic()).getW().getAffineY().toByteArray());
keyMaterial.setId(new VersionedKeyEntityId(baseUri, name, version).asUri(uri).toString());
final KeyBackupListItem listItem = new KeyBackupListItem();
listItem.setKeyMaterial(keyMaterial);
listItem.setVaultBaseUri(baseUri);
listItem.setId(name);
listItem.setVersion(version);
final KeyPropertiesModel propertiesModel = new KeyPropertiesModel();
propertiesModel.setCreatedOn(TIME_10_MINUTES_AGO);
propertiesModel.setUpdatedOn(NOW);
propertiesModel.setNotBefore(TIME_IN_10_MINUTES);
propertiesModel.setExpiresOn(TIME_IN_10_MINUTES.plusDays(1));
propertiesModel.setRecoveryLevel(RecoveryLevel.RECOVERABLE_AND_PURGEABLE);
propertiesModel.setRecoverableDays(RecoveryLevel.MAX_RECOVERABLE_DAYS_INCLUSIVE);
listItem.setAttributes(propertiesModel);
listItem.setTags(tags);
final List<KeyBackupListItem> list = new ArrayList<>(backupModel.getValue().getVersions());
list.add(listItem);
backupModel.getValue().setVersions(list);
return keyPair;
}
}
Loading

0 comments on commit 7d19cde

Please sign in to comment.