Skip to content

Commit 532641e

Browse files
authored
Ensure intended key is selected in SamlAuthenticatorTests (#30993)
* Ensure that a purposefully wrong key is used Uses a specific keypair for tests that require a purposefully wrong keypair instead of selecting one randomly from the same pull from which the correct one is selected. Entropy is low because of the small space and the same key can be randomly selected as both the correct one and the wrong one, causing the tests to fail. The purposefully wrong key is also used in testSigningKeyIsReloadedForEachRequest and needs to be cleaned up afterwards so the rest of the tests don't use that for signing. Resolves #30970
1 parent 46e8d97 commit 532641e

File tree

2 files changed

+6
-5
lines changed

2 files changed

+6
-5
lines changed

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ public void testFailWhenAssertionsCannotBeDecrypted() throws Exception {
374374
final String xml = getSimpleResponse(now);
375375

376376
// Encrypting with different cert instead of sp cert will mean that the SP cannot decrypt
377-
final String encrypted = encryptAssertions(xml, readKeyPair("RSA_1024"));
377+
final String encrypted = encryptAssertions(xml, readKeyPair("RSA_4096_updated"));
378378
assertThat(encrypted, not(equalTo(xml)));
379379

380380
final String signed = signDoc(encrypted);
@@ -896,7 +896,6 @@ public void testIdpInitiatedLoginIsAllowed() throws Exception {
896896
assertThat(attributes.attributes(), iterableWithSize(1));
897897
}
898898

899-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/30970")
900899
public void testIncorrectSigningKeyIsRejected() throws Exception {
901900
final CryptoTransform signer = randomBoolean() ? this::signDoc : this::signAssertions;
902901
Instant now = clock.instant();
@@ -938,7 +937,7 @@ public void testIncorrectSigningKeyIsRejected() throws Exception {
938937
assertThat(authenticator.authenticate(token(signer.transform(xml, idpSigningCertificatePair))), notNullValue());
939938

940939
// check is rejected when signed by a different key-pair
941-
final Tuple<X509Certificate, PrivateKey> wrongKey = readRandomKeyPair(randomSigningAlgorithm());
940+
final Tuple<X509Certificate, PrivateKey> wrongKey = readKeyPair("RSA_4096_updated");
942941
final ElasticsearchSecurityException exception = expectThrows(ElasticsearchSecurityException.class,
943942
() -> authenticator.authenticate(token(signer.transform(xml, wrongKey))));
944943
assertThat(exception.getMessage(), containsString("SAML Signature"));
@@ -954,10 +953,12 @@ public void testSigningKeyIsReloadedForEachRequest() throws Exception {
954953
assertThat(authenticator.authenticate(token(signer.transform(xml, idpSigningCertificatePair))), notNullValue());
955954

956955
final Tuple<X509Certificate, PrivateKey> oldKeyPair = idpSigningCertificatePair;
957-
//Ensure we won't read any of the ones we could have picked randomly before
956+
// Ensure we won't read any of the ones we could have picked randomly before
958957
idpSigningCertificatePair = readKeyPair("RSA_4096_updated");
959958
assertThat(idpSigningCertificatePair.v2(), not(equalTo(oldKeyPair.v2())));
960959
assertThat(authenticator.authenticate(token(signer.transform(xml, idpSigningCertificatePair))), notNullValue());
960+
// Restore the keypair to one from the keypair pool of all algorithms and keys
961+
idpSigningCertificatePair = readRandomKeyPair(randomSigningAlgorithm());
961962
}
962963

963964
public void testParsingRejectsTamperedContent() throws Exception {

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlTestCase.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ protected static Tuple<X509Certificate, PrivateKey> readRandomKeyPair() throws E
7171
}
7272

7373
/**
74-
* Generates key pair for given algorithm and then associates with a certificate.
74+
* Reads a key pair and associated certificate for given algorithm and key length
7575
* For testing, for "EC" algorithm 256 key size is used, others use 2048 as default.
7676
* @param algorithm
7777
* @return X509Certificate a signed certificate, it's PrivateKey {@link Tuple}

0 commit comments

Comments
 (0)