Skip to content

Commit 4398244

Browse files
feeblefakiejnmt
feeblefakie
and
jnmt
authored
Backport to branch(3.11) : Fix to correctly validate authentication settings (#232)
Co-authored-by: Jun Nemoto <[email protected]>
1 parent 84c4b72 commit 4398244

File tree

5 files changed

+62
-36
lines changed

5 files changed

+62
-36
lines changed

‎client/src/main/java/com/scalar/dl/client/config/ClientConfig.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,17 +155,19 @@ public class ClientConfig {
155155
public static final String DS_PRIVATE_KEY_PEM = DS_IDENTITY_PREFIX + "private_key_pem";
156156
/**
157157
* <code>scalar.dl.client.authentication_method</code> (Optional)<br>
158-
* The authentication method for a client and servers. Use {@code "digital-signature"} (default)
159-
* or {@code "hmac"}.
158+
* The authentication method for clients and Ledger/Auditor servers. {@code "digital-signature"}
159+
* (default) or {@code "hmac"} can be specified. This must be consistent with the Ledger/Auditor
160+
* configuration.
160161
*
161162
* @deprecated This variable will be deleted in release 5.0.0. Use {@code
162163
* scalar.dl.client.authentication.method} instead.
163164
*/
164165
public static final String DEPRECATED_AUTHENTICATION_METHOD = PREFIX + "authentication_method";
165166
/**
166167
* <code>scalar.dl.client.authentication.method</code> (Optional)<br>
167-
* The authentication method for a client and servers. Use {@code "digital-signature"} (default)
168-
* or {@code "hmac"}.
168+
* The authentication method for clients and Ledger/Auditor servers. {@code "digital-signature"}
169+
* (default) or {@code "hmac"} can be specified. This must be consistent with the Ledger/Auditor
170+
* configuration.
169171
*/
170172
public static final String AUTHENTICATION_METHOD = PREFIX + "authentication.method";
171173
/**

‎ledger/src/integration-test/java/com/scalar/dl/ledger/service/LedgerServiceEndToEndTest.java‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2536,7 +2536,9 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
25362536
Properties props2 = createProperties();
25372537
props2.put(LedgerConfig.AUDITOR_ENABLED, "true");
25382538
props2.put(LedgerConfig.PROOF_ENABLED, "true");
2539-
props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_A);
2539+
props2.put(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
2540+
props2.put(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY);
2541+
props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_B);
25402542
createServices(new LedgerConfig(props2));
25412543
String nonce = UUID.randomUUID().toString();
25422544
JsonNode contractArgument =
@@ -2547,18 +2549,18 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
25472549
String argument = Argument.format(contractArgument, nonce, Collections.emptyList());
25482550

25492551
byte[] serialized =
2550-
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_A, KEY_VERSION);
2552+
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_C, KEY_VERSION);
25512553
ContractExecutionRequest request =
25522554
new ContractExecutionRequest(
25532555
nonce,
2554-
ENTITY_ID_A,
2556+
ENTITY_ID_C,
25552557
KEY_VERSION,
25562558
CREATE_CONTRACT_ID3,
25572559
argument,
25582560
Collections.emptyList(),
25592561
null,
2560-
dsSigner1.sign(serialized),
2561-
hmacSigner1.sign(nonce.getBytes(StandardCharsets.UTF_8)));
2562+
hmacSigner1.sign(serialized),
2563+
hmacSigner2.sign(nonce.getBytes(StandardCharsets.UTF_8)));
25622564

25632565
// Act
25642566
Throwable thrown = catchThrowable(() -> ledgerService.execute(request));
@@ -2574,6 +2576,8 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
25742576
Properties props2 = createProperties();
25752577
props2.put(LedgerConfig.AUDITOR_ENABLED, "true");
25762578
props2.put(LedgerConfig.PROOF_ENABLED, "true");
2579+
props2.put(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
2580+
props2.put(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY);
25772581
props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_A);
25782582
createServices(new LedgerConfig(props2));
25792583
String nonce = UUID.randomUUID().toString();
@@ -2585,17 +2589,17 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
25852589
String argument = Argument.format(contractArgument, nonce, Collections.emptyList());
25862590

25872591
byte[] serialized =
2588-
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_A, KEY_VERSION);
2592+
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_C, KEY_VERSION);
25892593
ContractExecutionRequest request =
25902594
new ContractExecutionRequest(
25912595
nonce,
2592-
ENTITY_ID_A,
2596+
ENTITY_ID_C,
25932597
KEY_VERSION,
25942598
CREATE_CONTRACT_ID3,
25952599
argument,
25962600
Collections.emptyList(),
25972601
null,
2598-
dsSigner1.sign(serialized),
2602+
hmacSigner1.sign(serialized),
25992603
hmacSigner2.sign(nonce.getBytes(StandardCharsets.UTF_8))); // invalid HMAC signature
26002604

26012605
// Act

‎ledger/src/main/java/com/scalar/dl/ledger/config/LedgerConfig.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ public class LedgerConfig implements ServerConfig, ServersHmacAuthenticatable {
6666
public static final String NAMESPACE = PREFIX + "namespace";
6767
/**
6868
* <code>scalar.dl.ledger.authentication.method</code> (Optional)<br>
69-
* The authentication method for a client and servers. ("digital-signature" by default) This has
70-
* to be consistent with the client configuration.
69+
* The authentication method for clients and Ledger servers. {@code "digital-signature"} (default)
70+
* or {@code "hmac"} can be specified.
7171
*/
7272
public static final String AUTHENTICATION_METHOD = PREFIX + "authentication.method";
7373
/**
@@ -449,14 +449,13 @@ private void load() {
449449
}
450450
serversAuthHmacSecretKey =
451451
ConfigUtils.getString(props, SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, null);
452-
if (serversAuthHmacSecretKey == null && proofPrivateKey == null) {
452+
if (authenticationMethod == AuthenticationMethod.DIGITAL_SIGNATURE
453+
&& proofPrivateKey == null) {
453454
throw new IllegalArgumentException(
454455
LedgerError.CONFIG_INVALID_AUTHENTICATION_SETTING_BETWEEN_LEDGER_AUDITOR.buildMessage(
455-
SERVERS_AUTHENTICATION_HMAC_SECRET_KEY,
456-
PROOF_PRIVATE_KEY_PATH,
457-
PROOF_PRIVATE_KEY_PEM));
458-
}
459-
if (authenticationMethod == AuthenticationMethod.HMAC && serversAuthHmacSecretKey == null) {
456+
PROOF_PRIVATE_KEY_PATH, PROOF_PRIVATE_KEY_PEM));
457+
} else if (authenticationMethod == AuthenticationMethod.HMAC
458+
&& serversAuthHmacSecretKey == null) {
460459
throw new IllegalArgumentException(
461460
LedgerError.CONFIG_INVALID_AUTHENTICATION_SETTING_BETWEEN_LEDGER_AUDITOR_HMAC
462461
.buildMessage(SERVERS_AUTHENTICATION_HMAC_SECRET_KEY));

‎ledger/src/main/java/com/scalar/dl/ledger/error/LedgerError.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ public enum LedgerError implements ScalarDlError {
137137
CONFIG_INVALID_AUTHENTICATION_SETTING_BETWEEN_LEDGER_AUDITOR(
138138
StatusCode.INVALID_ARGUMENT,
139139
"003",
140-
"Authentication between Ledger and Auditor is not correctly configured. Set %s or set a private key with %s or %s.",
140+
"Authentication between Ledger and Auditor is not correctly configured. Set a private key with %s or %s if you use digital signature authentication with Auditor enabled.",
141141
"",
142142
""),
143143
CONFIG_INVALID_AUTHENTICATION_SETTING_BETWEEN_LEDGER_AUDITOR_HMAC(

‎ledger/src/test/java/com/scalar/dl/ledger/config/LedgerConfigTest.java‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -450,20 +450,6 @@ public void constructor_AuditorAndProofEnabledAndPrivateKeyGiven_ShouldConstruct
450450
assertThat(thrown).doesNotThrowAnyException();
451451
}
452452

453-
@Test
454-
public void constructor_AuditorAndProofEnabledAndSecretKeyGiven_ShouldConstructProperly() {
455-
// Arrange
456-
props.setProperty(LedgerConfig.AUDITOR_ENABLED, "true");
457-
props.setProperty(LedgerConfig.PROOF_ENABLED, "true");
458-
props.setProperty(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SOME_KEY);
459-
460-
// Act
461-
Throwable thrown = catchThrowable(() -> new LedgerConfig(props));
462-
463-
// Assert
464-
assertThat(thrown).doesNotThrowAnyException();
465-
}
466-
467453
@Test
468454
public void
469455
constructor_AuditorAndProofEnabledButNeitherPrivateKeyNorSecretKeyGiven_ShouldThrowIllegalArgumentException() {
@@ -525,12 +511,13 @@ public void constructor_AuditorEnabledButProofDisabled_ShouldThrowIllegalArgumen
525511

526512
@Test
527513
public void
528-
constructor_AuditorAndProofEnabledAndHmacConfiguredButSecretKeyNotGiven_ShouldThrowIllegalArgumentException() {
514+
constructor_AuditorEnabledButProofDisabledWithHmacConfiguration_ShouldThrowIllegalArgumentException() {
529515
// Arrange
530516
props.setProperty(LedgerConfig.AUDITOR_ENABLED, "true");
531517
props.setProperty(LedgerConfig.PROOF_ENABLED, "false");
532518
props.setProperty(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
533519
props.setProperty(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY);
520+
props.setProperty(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SOME_SECRET_KEY);
534521

535522
// Act
536523
Throwable thrown = catchThrowable(() -> new LedgerConfig(props));
@@ -554,6 +541,40 @@ public void constructor_HmacEnabledButCipherKeyNotGiven_ShouldThrowIllegalArgume
554541
assertThat(thrown).isExactlyInstanceOf(IllegalArgumentException.class);
555542
}
556543

544+
@Test
545+
public void
546+
constructor_AuditorAndProofEnabledInDigitalSignatureConfigurationButPrivateKeyNotGiven_ShouldThrowIllegalArgumentException() {
547+
// Arrange
548+
props.setProperty(LedgerConfig.AUDITOR_ENABLED, "true");
549+
props.setProperty(LedgerConfig.PROOF_ENABLED, "true");
550+
props.setProperty(
551+
LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.DIGITAL_SIGNATURE.getMethod());
552+
props.setProperty(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SOME_SECRET_KEY);
553+
554+
// Act
555+
Throwable thrown = catchThrowable(() -> new LedgerConfig(props));
556+
557+
// Assert
558+
assertThat(thrown).isExactlyInstanceOf(IllegalArgumentException.class);
559+
}
560+
561+
@Test
562+
public void
563+
constructor_AuditorAndProofEnabledInHmacConfigurationButSecretKeyNotGiven_ShouldThrowIllegalArgumentException() {
564+
// Arrange
565+
props.setProperty(LedgerConfig.AUDITOR_ENABLED, "true");
566+
props.setProperty(LedgerConfig.PROOF_ENABLED, "true");
567+
props.setProperty(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
568+
props.setProperty(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY);
569+
props.setProperty(LedgerConfig.PROOF_PRIVATE_KEY_PEM, SOME_PEM);
570+
571+
// Act
572+
Throwable thrown = catchThrowable(() -> new LedgerConfig(props));
573+
574+
// Assert
575+
assertThat(thrown).isExactlyInstanceOf(IllegalArgumentException.class);
576+
}
577+
557578
@Test
558579
public void
559580
constructor_ConsensusCommitWithGroupCommitDisabledSpecified_ShouldConstructProperly() {

0 commit comments

Comments
 (0)