Skip to content

Commit 204c11c

Browse files
feeblefakiejnmt
feeblefakie
and
jnmt
authored
Backport to branch(3.10) : Fix to correctly validate authentication settings (#233)
Co-authored-by: Jun Nemoto <[email protected]>
1 parent 546176c commit 204c11c

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
@@ -2513,7 +2513,9 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
25132513
Properties props2 = createProperties();
25142514
props2.put(LedgerConfig.AUDITOR_ENABLED, "true");
25152515
props2.put(LedgerConfig.PROOF_ENABLED, "true");
2516-
props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_A);
2516+
props2.put(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
2517+
props2.put(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY);
2518+
props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_B);
25172519
createServices(new LedgerConfig(props2));
25182520
String nonce = UUID.randomUUID().toString();
25192521
JsonNode contractArgument =
@@ -2524,18 +2526,18 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
25242526
String argument = Argument.format(contractArgument, nonce, Collections.emptyList());
25252527

25262528
byte[] serialized =
2527-
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_A, KEY_VERSION);
2529+
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_C, KEY_VERSION);
25282530
ContractExecutionRequest request =
25292531
new ContractExecutionRequest(
25302532
nonce,
2531-
ENTITY_ID_A,
2533+
ENTITY_ID_C,
25322534
KEY_VERSION,
25332535
CREATE_CONTRACT_ID3,
25342536
argument,
25352537
Collections.emptyList(),
25362538
null,
2537-
dsSigner1.sign(serialized),
2538-
hmacSigner1.sign(nonce.getBytes(StandardCharsets.UTF_8)));
2539+
hmacSigner1.sign(serialized),
2540+
hmacSigner2.sign(nonce.getBytes(StandardCharsets.UTF_8)));
25392541

25402542
// Act
25412543
Throwable thrown = catchThrowable(() -> ledgerService.execute(request));
@@ -2551,6 +2553,8 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
25512553
Properties props2 = createProperties();
25522554
props2.put(LedgerConfig.AUDITOR_ENABLED, "true");
25532555
props2.put(LedgerConfig.PROOF_ENABLED, "true");
2556+
props2.put(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
2557+
props2.put(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY);
25542558
props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_A);
25552559
createServices(new LedgerConfig(props2));
25562560
String nonce = UUID.randomUUID().toString();
@@ -2562,17 +2566,17 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
25622566
String argument = Argument.format(contractArgument, nonce, Collections.emptyList());
25632567

25642568
byte[] serialized =
2565-
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_A, KEY_VERSION);
2569+
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_C, KEY_VERSION);
25662570
ContractExecutionRequest request =
25672571
new ContractExecutionRequest(
25682572
nonce,
2569-
ENTITY_ID_A,
2573+
ENTITY_ID_C,
25702574
KEY_VERSION,
25712575
CREATE_CONTRACT_ID3,
25722576
argument,
25732577
Collections.emptyList(),
25742578
null,
2575-
dsSigner1.sign(serialized),
2579+
hmacSigner1.sign(serialized),
25762580
hmacSigner2.sign(nonce.getBytes(StandardCharsets.UTF_8))); // invalid HMAC signature
25772581

25782582
// 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)