Procházet zdrojové kódy

Fix NullPointerException in RotableSecret (#100779)

This commit fixes two things:
1) RotatableSecret#matches could throw a NullPointerException when the current secret is null but the prior secret is not.
2) RotatableSecret#checkExpired would not expire a prior secret when checking the same millisecond the prior secret was due to expire.

Both of these would cause intermittent test failures, the first based on randomization, the second based on timing.
Athena Brown před 2 roky
rodič
revize
be136c8f57

+ 6 - 0
docs/changelog/100779.yaml

@@ -0,0 +1,6 @@
+pr: 100779
+summary: Fix NullPointerException in RotableSecret
+area: Security
+type: bug
+issues:
+ - 99759

+ 10 - 4
server/src/main/java/org/elasticsearch/common/settings/RotatableSecret.java

@@ -69,10 +69,16 @@ public class RotatableSecret {
      */
     public boolean matches(SecureString secret) {
         checkExpired();
-        if ((Strings.hasText(secrets.current) == false && Strings.hasText(secrets.prior) == false) || Strings.hasText(secret) == false) {
+        if (Strings.hasText(secret) == false) {
             return false;
         }
-        return secrets.current.equals(secret) || (secrets.prior != null && secrets.prior.equals(secret));
+        boolean currentSecretValid = Strings.hasText(secrets.current);
+        boolean priorSecretValid = Strings.hasText(secrets.prior);
+        if (currentSecretValid && secrets.current.equals(secret)) {
+            return true;
+        } else {
+            return priorSecretValid && secrets.prior.equals(secret);
+        }
     }
 
     // for testing only
@@ -92,12 +98,12 @@ public class RotatableSecret {
     private void checkExpired() {
         boolean needToUnlock = false;
         long stamp = stampedLock.tryOptimisticRead();
-        boolean expired = secrets.prior != null && secrets.priorValidTill.isBefore(Instant.now()); // optimistic read
+        boolean expired = secrets.prior != null && secrets.priorValidTill.compareTo(Instant.now()) <= 0; // optimistic read
         if (stampedLock.validate(stamp) == false) {
             // optimism failed...potentially block to obtain the read lock and try the read again
             stamp = stampedLock.readLock();
             needToUnlock = true;
-            expired = secrets.prior != null && secrets.priorValidTill.isBefore(Instant.now()); // locked read
+            expired = secrets.prior != null && secrets.priorValidTill.compareTo(Instant.now()) <= 0; // locked read
         }
         try {
             if (expired) {

+ 0 - 1
server/src/test/java/org/elasticsearch/common/settings/RotatableSecretTests.java

@@ -27,7 +27,6 @@ public class RotatableSecretTests extends ESTestCase {
     private final SecureString secret2 = new SecureString(randomAlphaOfLength(10));
     private final SecureString secret3 = new SecureString(randomAlphaOfLength(10));
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/99759")
     public void testBasicRotation() throws Exception {
         // initial state
         RotatableSecret rotatableSecret = new RotatableSecret(secret1);