Skip to content

HDDS-13234. Expired secret key can abort leader OM startup. #8601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jojochuang merged 4 commits into from
Jun 17, 2025
Merged

HDDS-13234. Expired secret key can abort leader OM startup. #8601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1fa533f
HDDS-13234. Expired secret key can abort leader OM startup.
jojochuang Jun 10, 2025
6f2a6e7
Checkstyle
jojochuang Jun 10, 2025
2257242
Handle the corner case where delegation token expires.
jojochuang Jun 13, 2025
a6ab41e
Checkstyle
jojochuang Jun 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion ...ger/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,11 @@ private void loadTokenSecretState(
LOG.info("Loading token state into token manager.");
for (Map.Entry<OzoneTokenIdentifier, Long> entry :
state.getTokenState().entrySet()) {
addPersistedDelegationToken(entry.getKey(), entry.getValue());
try {
addPersistedDelegationToken(entry.getKey(), entry.getValue());
} catch (Exception e) {
LOG.error("exception while loading delegation token from DB... ignored to continue startup", e);
}
}
}

Expand All @@ -528,6 +532,16 @@ private void addPersistedDelegationToken(OzoneTokenIdentifier identifier,
byte[] password;
if (StringUtils.isNotEmpty(identifier.getSecretKeyId())) {
ManagedSecretKey signKey = secretKeyClient.getSecretKey(UUID.fromString(identifier.getSecretKeyId()));
if (signKey == null) {
Copy link
Contributor

@ChenSammi ChenSammi Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jojochuang , thanks for working on this improvements. For these token whose sign key is not available, we'd better put them in a map/list, and delete them at then end of loadTokenSecretState.

BTW, do you have idea how long a secret key will be deleted from SCM after it's creation?

Copy link
Contributor Author

@jojochuang jojochuang Jun 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secret key default life time is 1 day.
The background thread checks for validity every 10 minutes so a secret key can be removed at most 1 day + 10 minutes after.

Copy link
Contributor Author

@jojochuang jojochuang Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A delegation token associated with expired secret key does not mean the dt expired.

Secret key life time is 1 day. Delegation token life time is 1 day but can be renewed for up to 7 days.

Also, what if SCM is unreachable? Exception doesn't imply the secret key is invalid and does not imply dt is invalid.

I'd suggest to let the OM background thread (OzoneDelegationTokenSecretManager) to take care of expired dt later.

Copy link
Contributor Author

@jojochuang jojochuang Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: secret life time is 7 days. hdds.secret.key.expiry.duration
there's a property hdds.secret.key.rotate.duration which let SCM create a new secret key every 1 day. (SCM will keep a few secret keys in memory)
The SCM background checks for expired secret key every 10 minutes.

Copy link
Contributor Author

@jojochuang jojochuang Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's possible a dt is created near the secret key rotation duration (1 day), in which case the dt could stay valid much longer than the secret key.

Say a secret key is created at June 1st, 00:00am.
A dt is created using this secret key at June 1st 23:59pm.

The secret key would expire at June 8th, 00:00am and removed from SCM memory by June 8th, 00:10am.
The dt would expire at June 8th, 23:59pm and removed by June 9th 00:09am.

Copy link
Contributor

@ChenSammi ChenSammi Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to let the OM background thread (OzoneDelegationTokenSecretManager) to take care of expired dt later.

It makes sense. Besides, for those dt whose fetcing secret key failed and also it is expired, those dt will be not put int memory map, so will not cleaned up by background thread, we delete them directly during load, thoughts?

// if delegation token expired, remove it from the store.
if (renewDate < Time.now()) {
LOG.info("Removing expired persisted delegation token {} from DB", identifier);
this.store.removeToken(identifier);
}

throw new IOException("Secret key " + UUID.fromString(identifier.getSecretKeyId()) +
" not found for token " + formatTokenId(identifier));
}
password = signKey.sign(identifier.getBytes());
} else {
if (LOG.isDebugEnabled()) {
Expand Down
41 changes: 41 additions & 0 deletions ...src/test/java/org/apache/hadoop/ozone/security/TestOzoneDelegationTokenSecretManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -231,6 +235,43 @@ public void testCreateToken() throws Exception {
validateHash(token.getPassword(), token.getIdentifier());
}

@Test
public void testExpiredSecretKey() throws Exception {
SecretKeyClient old = secretKeyClient;
secretKeyClient = spy(secretKeyClient);
doReturn(null).when(secretKeyClient).getSecretKey(any());

Text tester = new Text("tester");
OzoneTokenIdentifier identifier =
new OzoneTokenIdentifier(tester, tester, tester);
identifier.setSecretKeyId(UUID.randomUUID().toString());
identifier.setOmServiceId(OzoneConsts.OM_SERVICE_ID_DEFAULT);

// case 1: Secret key not found, and delegation token is valid.
om.getMetadataManager().getDelegationTokenTable().put(identifier, Long.MAX_VALUE);
try {
secretManager = createSecretManager(conf, TOKEN_MAX_LIFETIME,
expiryTime, TOKEN_REMOVER_SCAN_INTERVAL);
om.getMetadataManager().getDelegationTokenTable().delete(identifier);

// case 2: Secret key not found, and delegation token is expired.
OzoneTokenIdentifier identifier2 =
new OzoneTokenIdentifier(tester, tester, tester);
identifier2.setSecretKeyId(UUID.randomUUID().toString());
identifier2.setOmServiceId(OzoneConsts.OM_SERVICE_ID_DEFAULT);

om.getMetadataManager().getDelegationTokenTable().put(identifier2, Time.now() - 1);
secretManager = createSecretManager(conf, TOKEN_MAX_LIFETIME,
expiryTime, TOKEN_REMOVER_SCAN_INTERVAL);
// expired token should be removed from the table.
assertFalse(om.getMetadataManager().getDelegationTokenTable().isExist(identifier2),
"Expired token " + identifier2 + " should be removed from the table");
} finally {
verify(secretKeyClient, times(2)).getSecretKey(any());
secretKeyClient = old;
}
}

private void restartSecretManager() throws IOException {
secretManager.stop();
secretManager = null;
Expand Down
Loading