Skip to content

Commit 0a98bec

Browse files
feeblefakiejnmt
feeblefakie
and
jnmt
authored
Backport to branch(3.9) : Fix to correctly validate authentication settings (#234)
Co-authored-by: Jun Nemoto <[email protected]>
1 parent 5ae5861 commit 0a98bec

File tree

4 files changed

+63
-37
lines changed

4 files changed

+63
-37
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
@@ -154,17 +154,19 @@ public class ClientConfig {
154154
public static final String DS_PRIVATE_KEY_PEM = DS_IDENTITY_PREFIX + "private_key_pem";
155155
/**
156156
* <code>scalar.dl.client.authentication_method</code> (Optional)<br>
157-
* The authentication method for a client and servers. Use {@code "digital-signature"} (default)
158-
* or {@code "hmac"}.
157+
* The authentication method for clients and Ledger/Auditor servers. {@code "digital-signature"}
158+
* (default) or {@code "hmac"} can be specified. This must be consistent with the Ledger/Auditor
159+
* configuration.
159160
*
160161
* @deprecated This variable will be deleted in release 5.0.0. Use {@code
161162
* scalar.dl.client.authentication.method} instead.
162163
*/
163164
public static final String DEPRECATED_AUTHENTICATION_METHOD = PREFIX + "authentication_method";
164165
/**
165166
* <code>scalar.dl.client.authentication.method</code> (Optional)<br>
166-
* The authentication method for a client and servers. Use {@code "digital-signature"} (default)
167-
* or {@code "hmac"}.
167+
* The authentication method for clients and Ledger/Auditor servers. {@code "digital-signature"}
168+
* (default) or {@code "hmac"} can be specified. This must be consistent with the Ledger/Auditor
169+
* configuration.
168170
*/
169171
public static final String AUTHENTICATION_METHOD = PREFIX + "authentication.method";
170172
/**

‎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
@@ -2425,7 +2425,9 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
24252425
Properties props2 = createProperties();
24262426
props2.put(LedgerConfig.AUDITOR_ENABLED, "true");
24272427
props2.put(LedgerConfig.PROOF_ENABLED, "true");
2428-
props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_A);
2428+
props2.put(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
2429+
props2.put(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY);
2430+
props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_B);
24292431
createServices(new LedgerConfig(props2));
24302432
String nonce = UUID.randomUUID().toString();
24312433
JsonNode contractArgument =
@@ -2436,18 +2438,18 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
24362438
String argument = Argument.format(contractArgument, nonce, Collections.emptyList());
24372439

24382440
byte[] serialized =
2439-
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_A, KEY_VERSION);
2441+
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_C, KEY_VERSION);
24402442
ContractExecutionRequest request =
24412443
new ContractExecutionRequest(
24422444
nonce,
2443-
ENTITY_ID_A,
2445+
ENTITY_ID_C,
24442446
KEY_VERSION,
24452447
CREATE_CONTRACT_ID3,
24462448
argument,
24472449
Collections.emptyList(),
24482450
null,
2449-
dsSigner1.sign(serialized),
2450-
hmacSigner1.sign(nonce.getBytes(StandardCharsets.UTF_8)));
2451+
hmacSigner1.sign(serialized),
2452+
hmacSigner2.sign(nonce.getBytes(StandardCharsets.UTF_8)));
24512453

24522454
// Act
24532455
Throwable thrown = catchThrowable(() -> ledgerService.execute(request));
@@ -2463,6 +2465,8 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
24632465
Properties props2 = createProperties();
24642466
props2.put(LedgerConfig.AUDITOR_ENABLED, "true");
24652467
props2.put(LedgerConfig.PROOF_ENABLED, "true");
2468+
props2.put(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
2469+
props2.put(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY);
24662470
props2.put(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SECRET_KEY_A);
24672471
createServices(new LedgerConfig(props2));
24682472
String nonce = UUID.randomUUID().toString();
@@ -2474,17 +2478,17 @@ public void execute_AuditorEnabledAndValidAuditorHmacSignatureGiven_ShouldExecut
24742478
String argument = Argument.format(contractArgument, nonce, Collections.emptyList());
24752479

24762480
byte[] serialized =
2477-
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_A, KEY_VERSION);
2481+
ContractExecutionRequest.serialize(CREATE_CONTRACT_ID3, argument, ENTITY_ID_C, KEY_VERSION);
24782482
ContractExecutionRequest request =
24792483
new ContractExecutionRequest(
24802484
nonce,
2481-
ENTITY_ID_A,
2485+
ENTITY_ID_C,
24822486
KEY_VERSION,
24832487
CREATE_CONTRACT_ID3,
24842488
argument,
24852489
Collections.emptyList(),
24862490
null,
2487-
dsSigner1.sign(serialized),
2491+
hmacSigner1.sign(serialized),
24882492
hmacSigner2.sign(nonce.getBytes(StandardCharsets.UTF_8))); // invalid HMAC signature
24892493

24902494
// Act

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ public class LedgerConfig implements ServerConfig, ServersHmacAuthenticatable {
6464
public static final String NAMESPACE = PREFIX + "namespace";
6565
/**
6666
* <code>scalar.dl.ledger.authentication.method</code> (Optional)<br>
67-
* The authentication method for a client and servers. ("digital-signature" by default) This has
68-
* to be consistent with the client configuration.
67+
* The authentication method for clients and Ledger servers. {@code "digital-signature"} (default)
68+
* or {@code "hmac"} can be specified.
6969
*/
7070
public static final String AUTHENTICATION_METHOD = PREFIX + "authentication.method";
7171
/**
@@ -446,20 +446,19 @@ private void load() {
446446
}
447447
serversAuthHmacSecretKey =
448448
ConfigUtils.getString(props, SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, null);
449-
if (serversAuthHmacSecretKey == null && proofPrivateKey == null) {
449+
if (authenticationMethod == AuthenticationMethod.DIGITAL_SIGNATURE
450+
&& proofPrivateKey == null) {
450451
throw new IllegalArgumentException(
451452
String.format(
452453
"Authentication between Ledger and Auditor is not correctly configured."
453-
+ "Set %s or set a private key with %s or %s.",
454-
SERVERS_AUTHENTICATION_HMAC_SECRET_KEY,
455-
PROOF_PRIVATE_KEY_PATH,
456-
PROOF_PRIVATE_KEY_PEM));
457-
}
458-
if (authenticationMethod == AuthenticationMethod.HMAC && serversAuthHmacSecretKey == null) {
454+
+ " Set a private key with %s or %s if you use digital signature authentication with Auditor enabled.",
455+
PROOF_PRIVATE_KEY_PATH, PROOF_PRIVATE_KEY_PEM));
456+
} else if (authenticationMethod == AuthenticationMethod.HMAC
457+
&& serversAuthHmacSecretKey == null) {
459458
throw new IllegalArgumentException(
460459
String.format(
461460
"Authentication between Ledger and Auditor is not correctly configured."
462-
+ "Set %s if you use HMAC authentication with Auditor enabled.",
461+
+ " Set %s if you use HMAC authentication with Auditor enabled.",
463462
SERVERS_AUTHENTICATION_HMAC_SECRET_KEY));
464463
}
465464
} else { // Auditor disabled

‎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
@@ -449,20 +449,6 @@ public void constructor_AuditorAndProofEnabledAndPrivateKeyGiven_ShouldConstruct
449449
assertThat(thrown).doesNotThrowAnyException();
450450
}
451451

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

525511
@Test
526512
public void
527-
constructor_AuditorAndProofEnabledAndHmacConfiguredButSecretKeyNotGiven_ShouldThrowIllegalArgumentException() {
513+
constructor_AuditorEnabledButProofDisabledWithHmacConfiguration_ShouldThrowIllegalArgumentException() {
528514
// Arrange
529515
props.setProperty(LedgerConfig.AUDITOR_ENABLED, "true");
530516
props.setProperty(LedgerConfig.PROOF_ENABLED, "false");
531517
props.setProperty(LedgerConfig.AUTHENTICATION_METHOD, AuthenticationMethod.HMAC.getMethod());
532518
props.setProperty(LedgerConfig.AUTHENTICATION_HMAC_CIPHER_KEY, SOME_CIPHER_KEY);
519+
props.setProperty(LedgerConfig.SERVERS_AUTHENTICATION_HMAC_SECRET_KEY, SOME_SECRET_KEY);
533520

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

0 commit comments

Comments
 (0)