Skip to content

Commit

Permalink
move rotatable secret and add test
Browse files Browse the repository at this point in the history
  • Loading branch information
jakelandis committed Sep 6, 2023
1 parent 1617bf1 commit cc2e869
Show file tree
Hide file tree
Showing 8 changed files with 278 additions and 734 deletions.
690 changes: 0 additions & 690 deletions dump.txt

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.xpack.security.support;
package org.elasticsearch.common.settings;

import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.TimeValue;

import java.time.Instant;
import java.util.concurrent.locks.StampedLock;

/**
* Helper class to provide a secret that can be rotated. Once rotated the prior secret is available for a configured amount of time before
* it is invalidated. This allows for secrete rotation without temporary failures or the need to tightly orchestrate multiple parties.
* This class is threadsafe, however it is also assumes that matching secrets are frequent but rotation is a rare.
* Helper class to provide {@link SecureString} that can be rotated. Once rotated the prior secret is available for a configured amount
* of time before it is invalidated. This allows for secrete rotation without temporary failures or the need to tightly orchestrate
* multiple parties. This class is threadsafe, however it is also assumes that matching secrets are frequent but rotation is a rare.
*/
public class RotatableSecret {
private Secrets secrets;
Expand All @@ -28,7 +27,7 @@ public class RotatableSecret {
* @param secret The secret to rotate. {@code null} if the secret is not configured.
*/
public RotatableSecret(@Nullable SecureString secret) {
this.secrets = new Secrets(Strings.hasText(secret) ? secret : null, null, Instant.EPOCH);
this.secrets = new Secrets(Strings.hasText(secret) ? secret.clone() : null, null, Instant.EPOCH);
}

/**
Expand All @@ -42,7 +41,7 @@ public void rotate(SecureString newSecret, TimeValue gracePeriod) {
try {
if (this.secrets.current.equals(newSecret) == false) {
secrets = new Secrets(
Strings.hasText(newSecret) ? newSecret : null,
Strings.hasText(newSecret) ? newSecret.clone() : null,
secrets.current,
Instant.now().plusMillis(gracePeriod.getMillis())
);
Expand Down Expand Up @@ -74,11 +73,20 @@ public boolean matches(SecureString secret) {
return secrets.current.equals(secret) || (secrets.prior != null && secrets.prior.equals(secret));
}

// for testing purpose only
// for testing only
//TODO: adjust test so that i don't need to make this public
public Secrets getSecrets() {
return secrets;
}

// for testing only
boolean isWriteLocked() {
return stampedLock.isWriteLocked();
}

/**
* Checks to see if the prior secret TTL has expired. If expired, evict from the backing data structure.
*/
private void checkExpired() {
boolean needToUnlock = false;
long stamp = stampedLock.tryOptimisticRead();
Expand All @@ -91,7 +99,7 @@ private void checkExpired() {
}
try {
if (expired) {
stamp = stampedLock.tryConvertToWriteLock(stamp);// upgrade the read lock
stamp = stampedLock.tryConvertToWriteLock(stamp);
if (stamp == 0) {
// block until we can acquire the write lock
stamp = stampedLock.writeLock();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.common.settings;

import org.elasticsearch.core.TimeValue;
import org.elasticsearch.test.ESTestCase;
import org.mockito.stubbing.Answer;

import java.time.Instant;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicBoolean;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class RotatableSecretTests extends ESTestCase {

private final SecureString secret1 = new SecureString(randomAlphaOfLength(10));
private final SecureString secret2 = new SecureString(randomAlphaOfLength(10));
private final SecureString secret3 = new SecureString(randomAlphaOfLength(10));

public void testBasicRotation() throws Exception {
// initial state
RotatableSecret rotatableSecret = new RotatableSecret(secret1);
assertTrue(rotatableSecret.matches(secret1));
assertFalse(rotatableSecret.matches(secret2));
assertFalse(rotatableSecret.matches(new SecureString(randomAlphaOfLength(10))));
assertTrue(rotatableSecret.isSet());
assertEquals(secret1, rotatableSecret.getSecrets().current());
assertNull(rotatableSecret.getSecrets().prior());
assertEquals(Instant.EPOCH, rotatableSecret.getSecrets().priorValidTill());

// normal rotation
TimeValue expiresIn = TimeValue.timeValueDays(1);
rotatableSecret.rotate(secret2, expiresIn);
assertTrue(rotatableSecret.matches(secret1));
assertTrue(rotatableSecret.matches(secret2));
assertFalse(rotatableSecret.matches(new SecureString(randomAlphaOfLength(10))));
assertTrue(rotatableSecret.isSet());
assertEquals(secret2, rotatableSecret.getSecrets().current());
assertEquals(secret1, rotatableSecret.getSecrets().prior());
assertTrue(rotatableSecret.getSecrets().priorValidTill().isAfter(Instant.now()));
assertTrue(
rotatableSecret.getSecrets().priorValidTill().isBefore(Instant.now().plusMillis(TimeValue.timeValueDays(2).getMillis()))
);

// attempt to rotate same value does nothing
rotatableSecret.rotate(secret2, TimeValue.timeValueDays(99)); // ignores the new expiry since you can't rotate the same secret
assertTrue(rotatableSecret.matches(secret1));
assertTrue(rotatableSecret.matches(secret2));
assertFalse(rotatableSecret.matches(new SecureString(randomAlphaOfLength(10))));
assertTrue(rotatableSecret.isSet());
assertEquals(secret2, rotatableSecret.getSecrets().current());
assertEquals(secret1, rotatableSecret.getSecrets().prior());
assertTrue(rotatableSecret.getSecrets().priorValidTill().isAfter(Instant.now()));
assertTrue(
rotatableSecret.getSecrets().priorValidTill().isBefore(Instant.now().plusMillis(TimeValue.timeValueDays(2).getMillis()))
);

// rotate with expiry
rotatableSecret.rotate(secret3, TimeValue.timeValueMillis(1));
Thread.sleep(2); // ensure secret2 is expired
assertTrue(rotatableSecret.matches(secret3));
assertFalse(rotatableSecret.matches(secret1));
assertFalse(rotatableSecret.matches(secret2));
assertFalse(rotatableSecret.matches(new SecureString(randomAlphaOfLength(10))));
assertTrue(rotatableSecret.isSet());
assertEquals(secret3, rotatableSecret.getSecrets().current());
assertNull(rotatableSecret.getSecrets().prior());
assertTrue(rotatableSecret.getSecrets().priorValidTill().isBefore(Instant.now()));

// unset current and prior
rotatableSecret.rotate(null, TimeValue.ZERO);
assertFalse(rotatableSecret.matches(secret3));
assertFalse(rotatableSecret.matches(secret1));
assertFalse(rotatableSecret.matches(secret2));
assertFalse(rotatableSecret.matches(new SecureString(randomAlphaOfLength(10))));
assertFalse(rotatableSecret.isSet());
assertNull(rotatableSecret.getSecrets().current());
assertNull(rotatableSecret.getSecrets().prior());
assertTrue(rotatableSecret.getSecrets().priorValidTill().isBefore(Instant.now()));
}

public void testConcurrentReadWhileLocked() throws Exception {
// initial state
RotatableSecret rotatableSecret = new RotatableSecret(secret1);
assertTrue(rotatableSecret.matches(secret1));
assertFalse(rotatableSecret.matches(secret2));
assertEquals(secret1, rotatableSecret.getSecrets().current());
assertNull(rotatableSecret.getSecrets().prior());

CountDownLatch latch = new CountDownLatch(1);
TimeValue mockGracePeriod = mock(TimeValue.class); // use a mock to force a long rotation to exercise the concurrency
when(mockGracePeriod.getMillis()).then((Answer<Long>) invocation -> {
latch.await();
return Long.MAX_VALUE;
});

// start writer thread
Thread t1 = new Thread(() -> rotatableSecret.rotate(secret2, mockGracePeriod));
t1.start();
assertBusy(() -> assertEquals(Thread.State.WAITING, t1.getState())); // waiting on countdown latch, holds write lock

// start reader threads
int readers = randomIntBetween(0, 16);
Set<Thread> readerThreads = new HashSet<>(readers);
for (int i = 0; i <= readers; i++) {
Thread t = new Thread(() -> {
if (randomBoolean()) { // either matches or isSet can block
assertTrue(rotatableSecret.matches(secret1));
assertTrue(rotatableSecret.matches(secret2));
} else {
assertTrue(rotatableSecret.isSet());
}
});
readerThreads.add(t);
t.start();
}
for (Thread t : readerThreads) {
assertBusy(() -> assertEquals(Thread.State.WAITING, t.getState())); // waiting on write lock from thread 1 to be released
}
assertTrue(rotatableSecret.isWriteLocked());
latch.countDown(); // let thread1 finish, which also unblocks the reader threads
assertBusy(() -> assertEquals(Thread.State.TERMINATED, t1.getState())); // done with work
assertFalse(rotatableSecret.isWriteLocked());
for (Thread t : readerThreads) {
assertBusy(() -> assertEquals(Thread.State.TERMINATED, t.getState())); // done with work
t.join();
}
t1.join();
}

public void testConcurrentRotations() throws Exception {
// initial state
RotatableSecret rotatableSecret = new RotatableSecret(secret1);
assertTrue(rotatableSecret.matches(secret1));
assertFalse(rotatableSecret.matches(secret2));
assertEquals(secret1, rotatableSecret.getSecrets().current());
assertNull(rotatableSecret.getSecrets().prior());

// start first rotation
AtomicBoolean latch1 = new AtomicBoolean(false); // using boolean as latch to differentiate the kinds of waiting
TimeValue mockGracePeriod1 = mock(TimeValue.class); // use a mock to force a long rotation to exercise the concurrency
when(mockGracePeriod1.getMillis()).then((Answer<Long>) invocation -> {
while (latch1.get() == false) {
Thread.sleep(10); // thread in TIMED_WAITING
}
return Long.MAX_VALUE;
});
Thread t1 = new Thread(() -> rotatableSecret.rotate(secret2, mockGracePeriod1));
t1.start();
assertBusy(() -> assertEquals(Thread.State.TIMED_WAITING, t1.getState())); // waiting on latch, holds write lock

// start second rotation
AtomicBoolean latch2 = new AtomicBoolean(false);
TimeValue mockGracePeriod2 = mock(TimeValue.class); // use a mock to force a long rotation to exercise the concurrency
when(mockGracePeriod2.getMillis()).then((Answer<Long>) invocation -> {
while (latch2.get() == false) {
Thread.sleep(10); // thread in TIMED_WAITING
}
return Long.MAX_VALUE;
});
Thread t2 = new Thread(() -> rotatableSecret.rotate(secret3, mockGracePeriod2));
t2.start();
assertBusy(() -> assertEquals(Thread.State.WAITING, t2.getState())); // waiting on write lock from thread 1

// start third rotation
AtomicBoolean latch3 = new AtomicBoolean(false);
TimeValue mockGracePeriod3 = mock(TimeValue.class); // use a mock to force a long rotation to exercise the concurrency
when(mockGracePeriod3.getMillis()).then((Answer<Long>) invocation -> {
while (latch3.get() == false) {
Thread.sleep(10); // thread in TIMED_WAITING
}
return Long.MAX_VALUE;
});
Thread t3 = new Thread(() -> rotatableSecret.rotate(null, mockGracePeriod3));
t3.start();
assertBusy(() -> assertEquals(Thread.State.WAITING, t3.getState())); // waiting on write lock from thread 1

// initial state
assertEquals(rotatableSecret.getSecrets().current(), secret1);
assertNull(rotatableSecret.getSecrets().prior());
assertBusy(() -> assertEquals(Thread.State.TIMED_WAITING, t1.getState())); // waiting on latch
assertBusy(() -> assertEquals(Thread.State.WAITING, t2.getState())); // waiting on lock
assertBusy(() -> assertEquals(Thread.State.WAITING, t3.getState())); // waiting on lock

latch1.set(true); // let first rotation succeed
assertBusy(() -> assertEquals(Thread.State.TERMINATED, t1.getState())); // work done
assertBusy(() -> assertEquals(Thread.State.TIMED_WAITING, t2.getState())); // waiting on latch
assertBusy(() -> assertEquals(Thread.State.WAITING, t3.getState())); // waiting lock
assertEquals(rotatableSecret.getSecrets().current(), secret2);
assertEquals(rotatableSecret.getSecrets().prior(), secret1);

latch2.set(true); // let second rotation succeed
assertBusy(() -> assertEquals(Thread.State.TERMINATED, t1.getState())); // work done
assertBusy(() -> assertEquals(Thread.State.TERMINATED, t2.getState())); // work done
assertBusy(() -> assertEquals(Thread.State.TIMED_WAITING, t3.getState())); // waiting on latch
assertEquals(rotatableSecret.getSecrets().current(), secret3);
assertEquals(rotatableSecret.getSecrets().prior(), secret2);

latch3.set(true); // let third rotation succeed
assertBusy(() -> assertEquals(Thread.State.TERMINATED, t1.getState())); // work done
assertBusy(() -> assertEquals(Thread.State.TERMINATED, t2.getState())); // work done
assertBusy(() -> assertEquals(Thread.State.TERMINATED, t3.getState())); // work done
assertEquals(rotatableSecret.getSecrets().current(), null);
assertEquals(rotatableSecret.getSecrets().prior(), secret3);

t1.join();
t2.join();
t3.join();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -352,13 +352,13 @@ public Iterator<Setting<?>> settings() {
)
);

//TODO: document that this is reloadable
// TODO: document that this is reloadable
public static final Setting.AffixSetting<SecureString> CLIENT_AUTHENTICATION_SHARED_SECRET = RealmSettings.secureString(
TYPE,
"client_authentication.shared_secret"
);

//TODO: document this !
// TODO: document this !
public static final Setting.AffixSetting<TimeValue> CLIENT_AUTHENTICATION_SHARED_SECRET_ROTATION_GRACE = Setting.affixKeySetting(
RealmSettings.realmSettingPrefix(TYPE),
"client_authentication.rotation_grace_period",
Expand Down
Loading

0 comments on commit cc2e869

Please sign in to comment.