Skip to content

Commit 9f3ec46

Browse files
committed
Harden AWS credential providers
S3 Express identity cache: - Raise MAX_SIZE 25 -> 100 - Add proactive idle eviction: a periodic background sweep removes and closes entries not accessed within a 5-minute grace period. STS: - Fix NPE in StsAssumeRoleResolver Map.of(). IMDS: - Document ImdsClient's threading contract. - Remove the unused DisabledResolver.
1 parent efa03ea commit 9f3ec46

10 files changed

Lines changed: 217 additions & 77 deletions

File tree

aws/aws-credentials-imds/src/main/java/software/amazon/smithy/java/aws/credentials/imds/ImdsClient.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616

1717
/**
1818
* Minimal IMDSv2 client. Handles token acquisition, caching, and retries with exponential backoff.
19+
*
20+
* <p>Intended for use behind {@link software.amazon.smithy.java.auth.api.identity.CachingIdentityResolver},
21+
* which serializes refreshes so a given instance is never re-entered concurrently. All mutable state is
22+
* {@code volatile} and the cached token/profile/path flags only ever advance monotonically, so sharing an
23+
* instance across threads is still safe — concurrent callers will at worst issue redundant token or profile
24+
* fetches, never return incorrect results.
1925
*/
2026
final class ImdsClient {
2127

aws/aws-credentials-imds/src/main/java/software/amazon/smithy/java/aws/credentials/imds/ImdsCredentialProvider.java

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import software.amazon.smithy.java.aws.credentials.chain.CredentialFeatureId;
2424
import software.amazon.smithy.java.aws.credentials.chain.OrderingConstraint;
2525
import software.amazon.smithy.java.aws.credentials.chain.StandardProvider;
26-
import software.amazon.smithy.java.context.Context;
2726
import software.amazon.smithy.java.core.serde.document.Document;
2827
import software.amazon.smithy.java.json.JsonCodec;
2928
import software.amazon.smithy.java.logging.InternalLogger;
@@ -188,16 +187,4 @@ private static String getProfileProperty(AwsProfileFile profileFile, String key)
188187
AwsProfile profile = profileFile.profile(profileName);
189188
return profile != null ? profile.property(key) : null;
190189
}
191-
192-
/** Resolver returned when IMDS is disabled via configuration. */
193-
private static final class DisabledResolver implements AwsCredentialsResolver {
194-
private static final IdentityResult<AwsCredentialsIdentity> DISABLED = IdentityResult.ofError(
195-
ImdsCredentialProvider.class,
196-
"IMDS credential fetching is disabled");
197-
198-
@Override
199-
public IdentityResult<AwsCredentialsIdentity> resolveIdentity(Context requestProperties) {
200-
return DISABLED;
201-
}
202-
}
203190
}

aws/aws-credentials-sts/src/main/java/software/amazon/smithy/java/aws/credentials/sts/StsAssumeRoleResolver.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package software.amazon.smithy.java.aws.credentials.sts;
77

8+
import java.util.HashMap;
89
import java.util.HashSet;
910
import java.util.Map;
1011
import java.util.Set;
@@ -144,13 +145,13 @@ private IdentityResult<AwsCredentialsIdentity> callAssumeRole(
144145
var sourceResolver = createSourceResolver(sourceCredentials);
145146

146147
try (DynamicClient client = StsClientFactory.create(sourceResolver, endpoint)) {
147-
Map<String, Object> input = Map.of(
148-
"RoleArn",
149-
roleArn,
150-
"RoleSessionName",
151-
"smithy-java-" + System.currentTimeMillis(),
152-
"ExternalId",
153-
externalId);
148+
// ExternalId is optional; Map.of rejects null values, so only include it when present.
149+
Map<String, Object> input = new HashMap<>();
150+
input.put("RoleArn", roleArn);
151+
input.put("RoleSessionName", "smithy-java-" + System.currentTimeMillis());
152+
if (externalId != null) {
153+
input.put("ExternalId", externalId);
154+
}
154155
return StsWebIdentityResolver.parseCredentials(client.call("AssumeRole", input));
155156
}
156157
}

aws/aws-sigv4-s3express/src/main/java/software/amazon/smithy/java/aws/client/auth/scheme/s3express/S3ExpressIdentityCache.java

Lines changed: 90 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,12 @@
55

66
package software.amazon.smithy.java.aws.client.auth.scheme.s3express;
77

8+
import java.time.Clock;
9+
import java.time.Duration;
810
import java.util.concurrent.ConcurrentHashMap;
11+
import java.util.concurrent.ScheduledExecutorService;
12+
import java.util.concurrent.ScheduledFuture;
13+
import java.util.concurrent.TimeUnit;
914
import java.util.concurrent.atomic.AtomicLong;
1015
import java.util.concurrent.locks.ReentrantLock;
1116
import java.util.function.Function;
@@ -32,46 +37,90 @@
3237
* is O(N) but only runs on insert when the cache is full — eviction does not happen on the
3338
* read path.
3439
*
40+
* <h3>Idle eviction</h3>
41+
*
42+
* <p>The S3 Express SEP requires that cached session credentials for buckets that are no longer
43+
* accessed be reaped, to bound their lifetime. A periodic background sweep removes (and closes)
44+
* any entry that has not been accessed by a request for longer than {@link #idleTimeoutMillis}
45+
* (default 5 minutes). Closing an entry stops its {@link CachingIdentityResolver}'s background
46+
* {@code CreateSession} refresh loop, so an unused bucket stops re-minting credentials.
47+
*
3548
* <h3>Bounding</h3>
3649
*
37-
* <p>{@link #MAX_SIZE} is a small constant (25). The S3 Express SEP describes a recommended
38-
* upper bound of 100; v2's SDK uses 25 pending real-world data, and we follow that. Not
39-
* configurable today.
50+
* <p>{@link #MAX_SIZE} is a small constant (100), the recommended upper bound from the S3 Express
51+
* SEP. Not configurable today.
4052
*/
4153
final class S3ExpressIdentityCache implements AutoCloseable {
4254

43-
private static final int MAX_SIZE = 25;
55+
private static final int MAX_SIZE = 100;
56+
private static final Duration DEFAULT_IDLE_TIMEOUT = Duration.ofMinutes(5);
4457

4558
private final ConcurrentHashMap<S3ExpressIdentityKey, Entry> entries = new ConcurrentHashMap<>();
4659
private final ReentrantLock writeLock = new ReentrantLock();
4760
private final AtomicLong tick = new AtomicLong();
4861
private final Function<S3ExpressIdentityKey, CachingIdentityResolver<AwsCredentialsIdentity>> factory;
62+
private final Clock clock;
63+
private final long idleTimeoutMillis;
64+
private final ScheduledFuture<?> sweepTask;
4965

5066
S3ExpressIdentityCache(Function<S3ExpressIdentityKey, CachingIdentityResolver<AwsCredentialsIdentity>> factory) {
67+
this(factory, null, DEFAULT_IDLE_TIMEOUT, Clock.systemUTC());
68+
}
69+
70+
/**
71+
* @param factory builds the per-bucket resolver on cache miss.
72+
* @param sweepExecutor executor for the periodic idle-eviction sweep, or {@code null} to disable
73+
* the background sweep (callers may still invoke {@link #evictIdle()} directly).
74+
* @param idleTimeout how long an entry may go unaccessed before it is eligible for eviction.
75+
* @param clock clock used for idle tracking (injectable for tests).
76+
*/
77+
S3ExpressIdentityCache(
78+
Function<S3ExpressIdentityKey, CachingIdentityResolver<AwsCredentialsIdentity>> factory,
79+
ScheduledExecutorService sweepExecutor,
80+
Duration idleTimeout,
81+
Clock clock
82+
) {
5183
this.factory = factory;
84+
this.clock = clock;
85+
this.idleTimeoutMillis = idleTimeout.toMillis();
86+
if (sweepExecutor != null) {
87+
long sweepMillis = idleTimeoutMillis;
88+
this.sweepTask = sweepExecutor.scheduleWithFixedDelay(
89+
this::evictIdle,
90+
sweepMillis,
91+
sweepMillis,
92+
TimeUnit.MILLISECONDS);
93+
} else {
94+
this.sweepTask = null;
95+
}
5296
}
5397

5498
CachingIdentityResolver<AwsCredentialsIdentity> get(S3ExpressIdentityKey key) {
5599
Entry e = entries.get(key);
56100
if (e != null) {
57-
e.lastAccess = tick.incrementAndGet();
101+
touch(e);
58102
return e.resolver;
59103
}
60104
return getOrCreate(key);
61105
}
62106

107+
private void touch(Entry e) {
108+
e.lastAccessTick = tick.incrementAndGet();
109+
e.lastAccessMillis = clock.millis();
110+
}
111+
63112
private CachingIdentityResolver<AwsCredentialsIdentity> getOrCreate(S3ExpressIdentityKey key) {
64113
writeLock.lock();
65114
try {
66115
Entry e = entries.get(key);
67116
if (e != null) {
68-
e.lastAccess = tick.incrementAndGet();
117+
touch(e);
69118
return e.resolver;
70119
}
71120
if (entries.size() >= MAX_SIZE) {
72121
evictLeastRecentlyUsed();
73122
}
74-
Entry created = new Entry(factory.apply(key), tick.incrementAndGet());
123+
Entry created = new Entry(factory.apply(key), tick.incrementAndGet(), clock.millis());
75124
entries.put(key, created);
76125
return created.resolver;
77126
} finally {
@@ -83,7 +132,7 @@ private void evictLeastRecentlyUsed() {
83132
S3ExpressIdentityKey victim = null;
84133
long minTick = Long.MAX_VALUE;
85134
for (var entry : entries.entrySet()) {
86-
long t = entry.getValue().lastAccess;
135+
long t = entry.getValue().lastAccessTick;
87136
if (t < minTick) {
88137
minTick = t;
89138
victim = entry.getKey();
@@ -97,6 +146,27 @@ private void evictLeastRecentlyUsed() {
97146
}
98147
}
99148

149+
/**
150+
* Evict (and close) every entry that has not been accessed within the idle timeout. Invoked
151+
* periodically by the background sweep; also callable directly for testing.
152+
*/
153+
void evictIdle() {
154+
long now = clock.millis();
155+
writeLock.lock();
156+
try {
157+
var it = entries.entrySet().iterator();
158+
while (it.hasNext()) {
159+
Entry entry = it.next().getValue();
160+
if (now - entry.lastAccessMillis > idleTimeoutMillis) {
161+
it.remove();
162+
closeQuietly(entry.resolver);
163+
}
164+
}
165+
} finally {
166+
writeLock.unlock();
167+
}
168+
}
169+
100170
void invalidateAll() {
101171
writeLock.lock();
102172
try {
@@ -110,6 +180,9 @@ void invalidateAll() {
110180

111181
@Override
112182
public void close() {
183+
if (sweepTask != null) {
184+
sweepTask.cancel(false);
185+
}
113186
writeLock.lock();
114187
try {
115188
for (var entry : entries.values()) {
@@ -133,12 +206,17 @@ private static final class Entry {
133206
final CachingIdentityResolver<AwsCredentialsIdentity> resolver;
134207

135208
// Volatile so concurrent readers see updated values; races on simultaneous writes are
136-
// benign (we only use this for approximate ordering of eviction).
137-
volatile long lastAccess;
209+
// benign (we only use these for approximate ordering of eviction).
210+
//
211+
// lastAccessTick is a strictly monotonic counter used to pick the LRU victim on overflow.
212+
// lastAccessMillis is wall-clock time used to detect idle entries for proactive eviction.
213+
volatile long lastAccessTick;
214+
volatile long lastAccessMillis;
138215

139-
Entry(CachingIdentityResolver<AwsCredentialsIdentity> resolver, long lastAccess) {
216+
Entry(CachingIdentityResolver<AwsCredentialsIdentity> resolver, long lastAccessTick, long lastAccessMillis) {
140217
this.resolver = resolver;
141-
this.lastAccess = lastAccess;
218+
this.lastAccessTick = lastAccessTick;
219+
this.lastAccessMillis = lastAccessMillis;
142220
}
143221
}
144222
}

aws/aws-sigv4-s3express/src/main/java/software/amazon/smithy/java/aws/client/auth/scheme/s3express/S3ExpressIdentityProvider.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
package software.amazon.smithy.java.aws.client.auth.scheme.s3express;
77

8+
import java.time.Clock;
9+
import java.time.Duration;
810
import java.util.Objects;
911
import java.util.concurrent.Executors;
1012
import java.util.concurrent.ScheduledExecutorService;
@@ -35,6 +37,7 @@
3537
*/
3638
final class S3ExpressIdentityProvider implements IdentityResolver<AwsCredentialsIdentity> {
3739

40+
private static final Duration IDLE_TIMEOUT = Duration.ofMinutes(5);
3841
private static final AtomicInteger THREAD_COUNTER = new AtomicInteger();
3942
private static final ScheduledExecutorService REFRESH_EXECUTOR = Executors.newSingleThreadScheduledExecutor(r -> {
4043
Thread t = new Thread(r, "smithy-s3express-session-refresh-" + THREAD_COUNTER.incrementAndGet());
@@ -57,7 +60,12 @@ final class S3ExpressIdentityProvider implements IdentityResolver<AwsCredentials
5760
) {
5861
this.baseIdentityProvider = Objects.requireNonNull(baseIdentityProvider, "baseIdentityProvider");
5962
Objects.requireNonNull(createSession, "createSession");
60-
this.cache = new S3ExpressIdentityCache(key -> buildPerBucketResolver(key, createSession));
63+
// Reuse the shared refresh executor to also drive periodic idle-entry eviction.
64+
this.cache = new S3ExpressIdentityCache(
65+
key -> buildPerBucketResolver(key, createSession),
66+
REFRESH_EXECUTOR,
67+
IDLE_TIMEOUT,
68+
Clock.systemUTC());
6169
}
6270

6371
private CachingIdentityResolver<AwsCredentialsIdentity> buildPerBucketResolver(

0 commit comments

Comments
 (0)