From 56835b7fd10875fa631a3722643d319b4e6faac1 Mon Sep 17 00:00:00 2001 From: Jun Nemoto <35618893+jnmt@users.noreply.github.com> Date: Tue, 19 Aug 2025 09:17:20 +0000 Subject: [PATCH 1/2] Empty commit [skip ci] From 64c57211600a4f9271fe34718e9b2fe4c2ff9de8 Mon Sep 17 00:00:00 2001 From: Jun Nemoto <35618893+jnmt@users.noreply.github.com> Date: Tue, 19 Aug 2025 18:16:56 +0900 Subject: [PATCH 2/2] Fix to correctly validate authentication settings (#222) --- .../scalar/dl/client/config/ClientConfig.java | 10 ++-- .../service/LedgerServiceEndToEndTest.java | 20 +++++--- .../scalar/dl/ledger/config/LedgerConfig.java | 19 ++++--- .../dl/ledger/config/LedgerConfigTest.java | 51 +++++++++++++------ 4 files changed, 63 insertions(+), 37 deletions(-) diff --git a/client/src/main/java/com/scalar/dl/client/config/ClientConfig.java b/client/src/main/java/com/scalar/dl/client/config/ClientConfig.java index dcad417a..1d8e3021 100644 --- a/client/src/main/java/com/scalar/dl/client/config/ClientConfig.java +++ b/client/src/main/java/com/scalar/dl/client/config/ClientConfig.java @@ -154,8 +154,9 @@ public class ClientConfig { public static final String DS_PRIVATE_KEY_PEM = DS_IDENTITY_PREFIX + "private_key_pem"; /** * scalar.dl.client.authentication_method (Optional)
- * The authentication method for a client and servers. Use {@code "digital-signature"} (default) - * or {@code "hmac"}. + * The authentication method for clients and Ledger/Auditor servers. {@code "digital-signature"} + * (default) or {@code "hmac"} can be specified. This must be consistent with the Ledger/Auditor + * configuration. * * @deprecated This variable will be deleted in release 5.0.0. Use {@code * scalar.dl.client.authentication.method} instead. @@ -163,8 +164,9 @@ public class ClientConfig { public static final String DEPRECATED_AUTHENTICATION_METHOD = PREFIX + "authentication_method"; /** * scalar.dl.client.authentication.method (Optional)
- * The authentication method for a client and servers. Use {@code "digital-signature"} (default) - * or {@code "hmac"}. + * The authentication method for clients and Ledger/Auditor servers. {@code "digital-signature"} + * (default) or {@code "hmac"} can be specified. This must be consistent with the Ledger/Auditor + * configuration. */ public static final String AUTHENTICATION_METHOD = PREFIX + "authentication.method"; /** diff --git a/ledger/src/integration-test/java/com/scalar/dl/ledger/service/LedgerServiceEndToEndTest.java b/ledger/src/integration-test/java/com/scalar/dl/ledger/service/LedgerServiceEndToEndTest.java index a6692cab..cd061c7d 100644 --- a/ledger/src/integration-test/java/com/scalar/dl/ledger/service/LedgerServiceEndToEndTest.java +++ b/ledger/src/integration-test/java/com/scalar/dl/ledger/service/LedgerServiceEndToEndTest.java @@ -2425,7 +2425,9 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut Properties props2 = createProperties(); props2.put(LedgerConfig.AUDITOR_ENABLED, "true"); props2.put(LedgerConfig.PROOF_ENABLED, "true"); - props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_A); + props2.put(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod()); + props2.put(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY); + props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_B); createServices(new LedgerConfig(props2)); String nonce = UUID.randomUUID().toString(); JsonNode contractArgument = @@ -2436,18 +2438,18 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut String argument = Argument.format(contractArgument, nonce, Collections.emptyList()); byte[] serialized = - ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_A, KEY_VERSION); + ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_C, KEY_VERSION); ContractExecutionRequest request = new ContractExecutionRequest( nonce, - ENTITY_ID_A, + ENTITY_ID_C, KEY_VERSION, CREATE_CONTRACT_ID3, argument, Collections.emptyList(), null, - dsSigner1.sign(serialized), - hmacSigner1.sign(nonce.getBytes(StandardCharsets.UTF_8))); + hmacSigner1.sign(serialized), + hmacSigner2.sign(nonce.getBytes(StandardCharsets.UTF_8))); // Act Throwable thrown = catchThrowable(() -> ledgerService.execute(request)); @@ -2463,6 +2465,8 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut Properties props2 = createProperties(); props2.put(LedgerConfig.AUDITOR_ENABLED, "true"); props2.put(LedgerConfig.PROOF_ENABLED, "true"); + props2.put(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod()); + props2.put(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY); props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_A); createServices(new LedgerConfig(props2)); String nonce = UUID.randomUUID().toString(); @@ -2474,17 +2478,17 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut String argument = Argument.format(contractArgument, nonce, Collections.emptyList()); byte[] serialized = - ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_A, KEY_VERSION); + ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_C, KEY_VERSION); ContractExecutionRequest request = new ContractExecutionRequest( nonce, - ENTITY_ID_A, + ENTITY_ID_C, KEY_VERSION, CREATE_CONTRACT_ID3, argument, Collections.emptyList(), null, - dsSigner1.sign(serialized), + hmacSigner1.sign(serialized), hmacSigner2.sign(nonce.getBytes(StandardCharsets.UTF_8))); // invalid HMAC signature // Act diff --git a/ledger/src/main/java/com/scalar/dl/ledger/config/LedgerConfig.java b/ledger/src/main/java/com/scalar/dl/ledger/config/LedgerConfig.java index 1e9274cc..6cfc3fc2 100644 --- a/ledger/src/main/java/com/scalar/dl/ledger/config/LedgerConfig.java +++ b/ledger/src/main/java/com/scalar/dl/ledger/config/LedgerConfig.java @@ -64,8 +64,8 @@ public class LedgerConfig implements ServerConfig, ServersHmacAuthenticatable { public static final String NAMESPACE = PREFIX + "namespace"; /** * scalar.dl.ledger.authentication.method (Optional)
- * The authentication method for a client and servers. ("digital-signature" by default) This has - * to be consistent with the client configuration. + * The authentication method for clients and Ledger servers. {@code "digital-signature"} (default) + * or {@code "hmac"} can be specified. */ public static final String AUTHENTICATION_METHOD = PREFIX + "authentication.method"; /** @@ -446,20 +446,19 @@ private void load() { } serversAuthHmacSecretKey = ConfigUtils.getString(props, SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, null); - if (serversAuthHmacSecretKey == null && proofPrivateKey == null) { + if (authenticationMethod == AuthenticationMethod.DIGITAL_SIGNATURE + && proofPrivateKey == null) { throw new IllegalArgumentException( String.format( "Authentication between Ledger and Auditor is not correctly configured." - + "Set %s or set a private key with %s or %s.", - SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, - PROOF_PRIVATE_KEY_PATH, - PROOF_PRIVATE_KEY_PEM)); - } - if (authenticationMethod == AuthenticationMethod.HMAC && serversAuthHmacSecretKey == null) { + + " Set a private key with %s or %s if you use digital signature authentication with Auditor enabled.", + PROOF_PRIVATE_KEY_PATH, PROOF_PRIVATE_KEY_PEM)); + } else if (authenticationMethod == AuthenticationMethod.HMAC + && serversAuthHmacSecretKey == null) { throw new IllegalArgumentException( String.format( "Authentication between Ledger and Auditor is not correctly configured." - + "Set %s if you use HMAC authentication with Auditor enabled.", + + " Set %s if you use HMAC authentication with Auditor enabled.", SERVERS_AUTHENTICATION_HMAC_SECRET_KEY)); } } else { // Auditor disabled diff --git a/ledger/src/test/java/com/scalar/dl/ledger/config/LedgerConfigTest.java b/ledger/src/test/java/com/scalar/dl/ledger/config/LedgerConfigTest.java index a61f2d14..d3f46ef0 100644 --- a/ledger/src/test/java/com/scalar/dl/ledger/config/LedgerConfigTest.java +++ b/ledger/src/test/java/com/scalar/dl/ledger/config/LedgerConfigTest.java @@ -449,20 +449,6 @@ public void constructor_AuditorAndProofEnabledAndPrivateKeyGiven_ShouldConstruct assertThat(thrown).doesNotThrowAnyException(); } - @Test - public void constructor_AuditorAndProofEnabledAndSecretKeyGiven_ShouldConstructProperly() { - // Arrange - props.setProperty(LedgerConfig.AUDITOR_ENABLED, "true"); - props.setProperty(LedgerConfig.PROOF_ENABLED, "true"); - props.setProperty(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SOME_KEY); - - // Act - Throwable thrown = catchThrowable(() -> new LedgerConfig(props)); - - // Assert - assertThat(thrown).doesNotThrowAnyException(); - } - @Test public void constructor_AuditorAndProofEnabledButNeitherPrivateKeyNorSecretKeyGiven_ShouldThrowIllegalArgumentException() { @@ -524,12 +510,13 @@ public void constructor_AuditorEnabledButProofDisabled_ShouldThrowIllegalArgumen @Test public void - constructor_AuditorAndProofEnabledAndHmacConfiguredButSecretKeyNotGiven_ShouldThrowIllegalArgumentException() { + constructor_AuditorEnabledButProofDisabledWithHmacConfiguration_ShouldThrowIllegalArgumentException() { // Arrange props.setProperty(LedgerConfig.AUDITOR_ENABLED, "true"); props.setProperty(LedgerConfig.PROOF_ENABLED, "false"); props.setProperty(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod()); props.setProperty(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY); + props.setProperty(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SOME_SECRET_KEY); // Act Throwable thrown = catchThrowable(() -> new LedgerConfig(props)); @@ -552,4 +539,38 @@ public void constructor_HmacEnabledButCipherKeyNotGiven_ShouldThrowIllegalArgume // Assert assertThat(thrown).isExactlyInstanceOf(IllegalArgumentException.class); } + + @Test + public void + constructor_AuditorAndProofEnabledInDigitalSignatureConfigurationButPrivateKeyNotGiven_ShouldThrowIllegalArgumentException() { + // Arrange + props.setProperty(LedgerConfig.AUDITOR_ENABLED, "true"); + props.setProperty(LedgerConfig.PROOF_ENABLED, "true"); + props.setProperty( + LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.DIGITAL_SIGNATURE.getMethod()); + props.setProperty(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SOME_SECRET_KEY); + + // Act + Throwable thrown = catchThrowable(() -> new LedgerConfig(props)); + + // Assert + assertThat(thrown).isExactlyInstanceOf(IllegalArgumentException.class); + } + + @Test + public void + constructor_AuditorAndProofEnabledInHmacConfigurationButSecretKeyNotGiven_ShouldThrowIllegalArgumentException() { + // Arrange + props.setProperty(LedgerConfig.AUDITOR_ENABLED, "true"); + props.setProperty(LedgerConfig.PROOF_ENABLED, "true"); + props.setProperty(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod()); + props.setProperty(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY); + props.setProperty(LedgerConfig.PROOF_PRIVATE_KEY_PEM, SOME_PEM); + + // Act + Throwable thrown = catchThrowable(() -> new LedgerConfig(props)); + + // Assert + assertThat(thrown).isExactlyInstanceOf(IllegalArgumentException.class); + } }