From 5fac38741c2d35cf23b82e57e63fc10e92ef373b Mon Sep 17 00:00:00 2001 From: chris-j-h Date: Tue, 18 Dec 2018 13:45:32 +0000 Subject: [PATCH] Add AzureKeyPair validation to prevent only one key version being set --- .../AzureVaultKeyPairValidator.java | 29 +++++++++ .../constraints/ValidAzureVaultKeyPair.java | 25 +++++++ .../config/keypairs/AzureVaultKeyPair.java | 3 + .../resources/ValidationMessages.properties | 1 + .../quorum/tessera/config/ValidationTest.java | 24 +++++++ .../config/adapters/KeyDataAdapterTest.java | 16 ++++- .../AzureVaultKeyPairValidatorTest.java | 65 +++++++++++++++++++ .../keypairs/UnsupportedKeyPairTest.java | 8 ++- 8 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 config/src/main/java/com/quorum/tessera/config/constraints/AzureVaultKeyPairValidator.java create mode 100644 config/src/main/java/com/quorum/tessera/config/constraints/ValidAzureVaultKeyPair.java create mode 100644 config/src/test/java/com/quorum/tessera/config/constraints/AzureVaultKeyPairValidatorTest.java diff --git a/config/src/main/java/com/quorum/tessera/config/constraints/AzureVaultKeyPairValidator.java b/config/src/main/java/com/quorum/tessera/config/constraints/AzureVaultKeyPairValidator.java new file mode 100644 index 0000000000..4c2bbbddf4 --- /dev/null +++ b/config/src/main/java/com/quorum/tessera/config/constraints/AzureVaultKeyPairValidator.java @@ -0,0 +1,29 @@ +package com.quorum.tessera.config.constraints; + +import com.quorum.tessera.config.keypairs.AzureVaultKeyPair; + +import javax.validation.ConstraintValidator; +import javax.validation.ConstraintValidatorContext; +import java.util.Objects; + +public class AzureVaultKeyPairValidator implements ConstraintValidator { + + private ValidAzureVaultKeyPair annotation; + + @Override + public void initialize(ValidAzureVaultKeyPair annotation) { + this.annotation = annotation; + } + + @Override + public boolean isValid(AzureVaultKeyPair azureVaultKeyPair, ConstraintValidatorContext cvc) { + + if (azureVaultKeyPair == null) { + return true; + } + + return Objects.isNull(azureVaultKeyPair.getPublicKeyVersion()) == Objects.isNull(azureVaultKeyPair.getPrivateKeyVersion()); + + } + +} diff --git a/config/src/main/java/com/quorum/tessera/config/constraints/ValidAzureVaultKeyPair.java b/config/src/main/java/com/quorum/tessera/config/constraints/ValidAzureVaultKeyPair.java new file mode 100644 index 0000000000..4abafc6fa8 --- /dev/null +++ b/config/src/main/java/com/quorum/tessera/config/constraints/ValidAzureVaultKeyPair.java @@ -0,0 +1,25 @@ +package com.quorum.tessera.config.constraints; + +import javax.validation.Constraint; +import javax.validation.Payload; +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.*; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +@Target({FIELD, PARAMETER, ANNOTATION_TYPE, ElementType.TYPE}) +@Retention(RUNTIME) +@Constraint(validatedBy = AzureVaultKeyPairValidator.class) +@Documented +public @interface ValidAzureVaultKeyPair { + + String message() default "{AzureVaultKeyData.message}"; + + Class[] groups() default {}; + + Class[] payload() default {}; + +} diff --git a/config/src/main/java/com/quorum/tessera/config/keypairs/AzureVaultKeyPair.java b/config/src/main/java/com/quorum/tessera/config/keypairs/AzureVaultKeyPair.java index 23ffea8bde..44e5990e12 100644 --- a/config/src/main/java/com/quorum/tessera/config/keypairs/AzureVaultKeyPair.java +++ b/config/src/main/java/com/quorum/tessera/config/keypairs/AzureVaultKeyPair.java @@ -1,10 +1,13 @@ package com.quorum.tessera.config.keypairs; +import com.quorum.tessera.config.constraints.ValidAzureVaultKeyPair; + import javax.validation.constraints.NotNull; import javax.validation.constraints.Pattern; import javax.validation.constraints.Size; import javax.xml.bind.annotation.XmlElement; +@ValidAzureVaultKeyPair public class AzureVaultKeyPair implements ConfigKeyPair { @NotNull diff --git a/config/src/main/resources/ValidationMessages.properties b/config/src/main/resources/ValidationMessages.properties index a4eba4c2bf..1abe27bf2c 100644 --- a/config/src/main/resources/ValidationMessages.properties +++ b/config/src/main/resources/ValidationMessages.properties @@ -20,6 +20,7 @@ UnsupportedKeyPair.allHashicorpKeyDataRequired.message=Invalid Hashicorp Key Vau UnsupportedKeyPair.bothFilesystemKeysRequired.message=Invalid filesystem key-pair. Ensure that both the public key path and private key path for the key-pair have been provided. ValidKeyDataConfig.message=A locked key was provided without a password.\n Please ensure the same number of passwords are provided as there are keys and remember to include empty passwords for unlocked keys InlineKeyData.message=A locked key was provided without a password.\n Please ensure the same number of passwords are provided as there are keys and remember to include empty passwords for unlocked keys +AzureVaultKeyData.message=Only one key version was provided for the Azure vault key pair. Either set the version for both the public and private key, or leave both unset ValidKeyConfiguration.message=A password file and inline passwords were provided. Please choose one or the other ValidKeyVaultConfiguration.message=No key vault configuration was specified but vault key data was provided ValidKeyVaultConfiguration.azure.message=No azureKeyVaultConfig was specified but azureVaultPublicKeyId and azureVaultPrivateKeyId were provided diff --git a/config/src/test/java/com/quorum/tessera/config/ValidationTest.java b/config/src/test/java/com/quorum/tessera/config/ValidationTest.java index 2ec5e76bdc..6b404a9041 100644 --- a/config/src/test/java/com/quorum/tessera/config/ValidationTest.java +++ b/config/src/test/java/com/quorum/tessera/config/ValidationTest.java @@ -307,6 +307,30 @@ public void azureKeyPairKeyVersionShorterThan32CharsCreatesViolation() { .containsExactly("length must be 32 characters", "length must be 32 characters"); } + @Test + public void azureKeyPairOnlyPublicKeyVersionSetCreatesViolation() { + String is32Chars = "12345678901234567890123456789012"; + + AzureVaultKeyPair azureVaultKeyPair = new AzureVaultKeyPair("pubId", "privId", is32Chars, null); + + Set> violations = validator.validate(azureVaultKeyPair); + assertThat(violations).hasSize(1); + + assertThat(violations.iterator().next().getMessage()).isEqualTo("Only one key version was provided for the Azure vault key pair. Either set the version for both the public and private key, or leave both unset"); + } + + @Test + public void azureKeyPairOnlyPrivateKeyVersionSetCreatesViolation() { + String is32Chars = "12345678901234567890123456789012"; + + AzureVaultKeyPair azureVaultKeyPair = new AzureVaultKeyPair("pubId", "privId", null, is32Chars); + + Set> violations = validator.validate(azureVaultKeyPair); + assertThat(violations).hasSize(1); + + assertThat(violations.iterator().next().getMessage()).isEqualTo("Only one key version was provided for the Azure vault key pair. Either set the version for both the public and private key, or leave both unset"); + } + @Test public void azureKeyPairProvidedWithoutKeyVaultConfigCreatesViolation() { AzureVaultKeyPair keyPair = new AzureVaultKeyPair("publicVauldId", "privateVaultId", null, null); diff --git a/config/src/test/java/com/quorum/tessera/config/adapters/KeyDataAdapterTest.java b/config/src/test/java/com/quorum/tessera/config/adapters/KeyDataAdapterTest.java index 0db4183c3c..5fd8339989 100644 --- a/config/src/test/java/com/quorum/tessera/config/adapters/KeyDataAdapterTest.java +++ b/config/src/test/java/com/quorum/tessera/config/adapters/KeyDataAdapterTest.java @@ -182,10 +182,24 @@ public void unmarshallingFilesystemKeysGivesCorrectKeypair() { } @Test - public void unmarshallingAzureKeysGivesCorrectKeyPair() { + public void unmarshallingAzureKeysWithNoVersionsGivesCorrectKeyPair() { final KeyData input = new KeyData(); input.setAzureVaultPublicKeyId("pubId"); input.setAzureVaultPrivateKeyId("privId"); + input.setAzureVaultPublicKeyVersion(null); + input.setAzureVaultPrivateKeyVersion(null); + + final ConfigKeyPair result = this.adapter.unmarshal(input); + assertThat(result).isInstanceOf(AzureVaultKeyPair.class); + } + + @Test + public void unmarshallingAzureKeysWithVersionsGivesCorrectKeyPair() { + final KeyData input = new KeyData(); + input.setAzureVaultPublicKeyId("pubId"); + input.setAzureVaultPrivateKeyId("privId"); + input.setAzureVaultPublicKeyVersion("pubVer"); + input.setAzureVaultPrivateKeyVersion("privVer"); final ConfigKeyPair result = this.adapter.unmarshal(input); assertThat(result).isInstanceOf(AzureVaultKeyPair.class); diff --git a/config/src/test/java/com/quorum/tessera/config/constraints/AzureVaultKeyPairValidatorTest.java b/config/src/test/java/com/quorum/tessera/config/constraints/AzureVaultKeyPairValidatorTest.java new file mode 100644 index 0000000000..7341bbb491 --- /dev/null +++ b/config/src/test/java/com/quorum/tessera/config/constraints/AzureVaultKeyPairValidatorTest.java @@ -0,0 +1,65 @@ +package com.quorum.tessera.config.constraints; + +import com.quorum.tessera.config.keypairs.AzureVaultKeyPair; +import org.junit.Before; +import org.junit.Test; + +import javax.validation.ConstraintValidatorContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class AzureVaultKeyPairValidatorTest { + + private AzureVaultKeyPairValidator validator; + + private AzureVaultKeyPair keyPair; + + private ConstraintValidatorContext cvc; + + @Before + public void setUp() { + this.validator = new AzureVaultKeyPairValidator(); + this.keyPair = mock(AzureVaultKeyPair.class); + this.cvc = mock(ConstraintValidatorContext.class); + } + + @Test + public void nullKeyPairIsValid() { + assertThat(validator.isValid(null, cvc)).isTrue(); + } + + @Test + public void publicAndPrivateKeyVersionsNotSetIsValid() { + when(keyPair.getPublicKeyVersion()).thenReturn(null); + when(keyPair.getPrivateKeyVersion()).thenReturn(null); + + assertThat(validator.isValid(keyPair, cvc)).isTrue(); + } + + @Test + public void publicAndPrivateKeyVersionsAreSetIsValid() { + when(keyPair.getPublicKeyVersion()).thenReturn("pubVer"); + when(keyPair.getPrivateKeyVersion()).thenReturn("privVer"); + + assertThat(validator.isValid(keyPair, cvc)).isTrue(); + } + + @Test + public void onlyPublicKeyVersionSetIsNotValid() { + when(keyPair.getPublicKeyVersion()).thenReturn("pubVer"); + when(keyPair.getPrivateKeyVersion()).thenReturn(null); + + assertThat(validator.isValid(keyPair, cvc)).isFalse(); + } + + @Test + public void onlyPrivateKeyVersionSetIsNotValid() { + when(keyPair.getPublicKeyVersion()).thenReturn(null); + when(keyPair.getPrivateKeyVersion()).thenReturn("privVer"); + + assertThat(validator.isValid(keyPair, cvc)).isFalse(); + } + +} diff --git a/config/src/test/java/com/quorum/tessera/config/keypairs/UnsupportedKeyPairTest.java b/config/src/test/java/com/quorum/tessera/config/keypairs/UnsupportedKeyPairTest.java index c8c7966ee9..916e5056db 100644 --- a/config/src/test/java/com/quorum/tessera/config/keypairs/UnsupportedKeyPairTest.java +++ b/config/src/test/java/com/quorum/tessera/config/keypairs/UnsupportedKeyPairTest.java @@ -24,12 +24,18 @@ public void getPasswordAlwaysReturnsNull() { } @Test - public void setHashicorpVaultSecretVersion() { + public void versionSetters() { assertThat(keyPair.getHashicorpVaultSecretVersion()).isNull(); + assertThat(keyPair.getAzureVaultPublicKeyVersion()).isNull(); + assertThat(keyPair.getAzureVaultPrivateKeyVersion()).isNull(); keyPair.setHashicorpVaultSecretVersion("1"); + keyPair.setAzureVaultPublicKeyVersion("pubVer"); + keyPair.setAzureVaultPrivateKeyVersion("privVer"); assertThat(keyPair.getHashicorpVaultSecretVersion()).isEqualTo("1"); + assertThat(keyPair.getAzureVaultPublicKeyVersion()).isEqualTo("pubVer"); + assertThat(keyPair.getAzureVaultPrivateKeyVersion()).isEqualTo("privVer"); } }