diff --git a/src/main/java/land/oras/auth/AbstractUsernamePasswordProvider.java b/src/main/java/land/oras/auth/AbstractUsernamePasswordProvider.java index de7d7c72..e95912a6 100644 --- a/src/main/java/land/oras/auth/AbstractUsernamePasswordProvider.java +++ b/src/main/java/land/oras/auth/AbstractUsernamePasswordProvider.java @@ -74,4 +74,9 @@ public String getAuthHeader(ContainerRef registry) { public AuthScheme getAuthScheme() { return AuthScheme.BASIC; } + + @Override + public String getIdentity(ContainerRef registry) { + return "BASIC:" + username; + } } diff --git a/src/main/java/land/oras/auth/AuthProvider.java b/src/main/java/land/oras/auth/AuthProvider.java index 5758f59a..ae18087c 100644 --- a/src/main/java/land/oras/auth/AuthProvider.java +++ b/src/main/java/land/oras/auth/AuthProvider.java @@ -42,4 +42,14 @@ public interface AuthProvider { * @return The authentication scheme */ AuthScheme getAuthScheme(); + + /** + * Get an opaque identity marker for the credentials this provider resolves for the given registry. This marker is + * used to key {@link TokenCache} entries alongside the requested {@link Scopes} + * @param registry The registry + * @return A non-null identity marker + */ + default String getIdentity(ContainerRef registry) { + return getAuthScheme().name(); + } } diff --git a/src/main/java/land/oras/auth/AuthStoreAuthenticationProvider.java b/src/main/java/land/oras/auth/AuthStoreAuthenticationProvider.java index 990a3027..5793c763 100644 --- a/src/main/java/land/oras/auth/AuthStoreAuthenticationProvider.java +++ b/src/main/java/land/oras/auth/AuthStoreAuthenticationProvider.java @@ -64,4 +64,10 @@ public String getAuthHeader(ContainerRef registry) { public AuthScheme getAuthScheme() { return AuthScheme.BASIC; } + + @Override + public String getIdentity(ContainerRef registry) { + Credential credential = authStore.get(registry); + return credential == null ? "BASIC:anonymous" : "BASIC:" + credential.username(); + } } diff --git a/src/main/java/land/oras/auth/BearerTokenProvider.java b/src/main/java/land/oras/auth/BearerTokenProvider.java index 87ea0c1c..53d9d840 100644 --- a/src/main/java/land/oras/auth/BearerTokenProvider.java +++ b/src/main/java/land/oras/auth/BearerTokenProvider.java @@ -20,6 +20,9 @@ package land.oras.auth; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import land.oras.ContainerRef; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -84,4 +87,25 @@ public void setToken(HttpClient.TokenResponse token) { public AuthScheme getAuthScheme() { return AuthScheme.BEARER; } + + @Override + public String getIdentity(ContainerRef registry) { + if (token == null) { + return "BEARER:none"; + } + return "BEARER:" + fingerprint(token.token()); + } + + private static String fingerprint(String secret) { + try { + byte[] hash = MessageDigest.getInstance("SHA-256").digest(secret.getBytes(StandardCharsets.UTF_8)); + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 6; i++) { + sb.append(String.format("%02x", hash[i])); + } + return sb.toString(); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException(e); + } + } } diff --git a/src/main/java/land/oras/auth/HttpClient.java b/src/main/java/land/oras/auth/HttpClient.java index 9cd03911..47b9301f 100644 --- a/src/main/java/land/oras/auth/HttpClient.java +++ b/src/main/java/land/oras/auth/HttpClient.java @@ -694,8 +694,10 @@ private ResponseWrapper executeRequest( case "DELETE" -> scopes.withAddedRegistryScopes(Scope.DELETE); default -> throw new OrasException("Unsupported HTTP method: " + method); }; + newScopes = newScopes.withIdentity(authProvider.getIdentity(containerRef)); LOG.debug("Existing scopes: {}", scopes.getScopes()); LOG.debug("New scopes: {}", newScopes.getScopes()); + LOG.debug("With identity {}", newScopes.getIdentity()); int maxAttempts = retryEnabled ? this.maxRetries : 1; diff --git a/src/main/java/land/oras/auth/Scopes.java b/src/main/java/land/oras/auth/Scopes.java index cee4101b..051c08d9 100644 --- a/src/main/java/land/oras/auth/Scopes.java +++ b/src/main/java/land/oras/auth/Scopes.java @@ -49,25 +49,37 @@ public final class Scopes { private final ContainerRef containerRef; /** - * Private constructor + * Opaque identity marker for the {@link AuthProvider} that resolved (or will resolve) this scope. Two + * {@link AuthProvider} instances that are not equivalent + */ + private final @Nullable String identity; + + /** + * Canonical private constructor * @param containerRef The container reference * @param service The service + * @param identity The auth provider identity marker * @param scopes The scopes */ - private Scopes(ContainerRef containerRef, @Nullable String service, Scope... scopes) { - this(containerRef, service, ScopeUtils.appendRepositoryScope(List.of(), containerRef, scopes)); + private Scopes( + ContainerRef containerRef, @Nullable String service, @Nullable String identity, List scopes) { + this.containerRef = containerRef; + this.service = service; + this.identity = identity; + this.scopes = scopes; } /** - * Private constructor + * Return Scopes with new scopes * @param containerRef The container reference * @param service The service + * @param identity The auth provider identity marker * @param scopes The scopes */ - private Scopes(ContainerRef containerRef, @Nullable String service, List scopes) { - this.containerRef = containerRef; - this.service = service; - this.scopes = scopes; + private static Scopes withRepositoryScopes( + ContainerRef containerRef, @Nullable String service, @Nullable String identity, Scope... scopes) { + return new Scopes( + containerRef, service, identity, ScopeUtils.appendRepositoryScope(List.of(), containerRef, scopes)); } /** @@ -77,7 +89,7 @@ private Scopes(ContainerRef containerRef, @Nullable String service, List * @return A new Scopes object */ public static Scopes of(ContainerRef containerRef, Scope... scopes) { - return new Scopes(containerRef, null, scopes); + return withRepositoryScopes(containerRef, null, null, scopes); } /** @@ -88,7 +100,7 @@ public static Scopes of(ContainerRef containerRef, Scope... scopes) { * @return A new Scopes object */ public static Scopes of(String service, ContainerRef containerRef, Scope... scopes) { - return new Scopes(containerRef, service, scopes); + return withRepositoryScopes(containerRef, service, null, scopes); } /** @@ -98,7 +110,7 @@ public static Scopes of(String service, ContainerRef containerRef, Scope... scop * @return A new Scopes object with no scopes */ public static Scopes empty(ContainerRef containerRef, String service) { - return new Scopes(containerRef, service, List.of()); + return new Scopes(containerRef, service, null, List.of()); } /** @@ -107,7 +119,7 @@ public static Scopes empty(ContainerRef containerRef, String service) { * @return A new Scopes object with the given scopes */ public Scopes withRegistryScopes(Scope... scopes) { - return new Scopes(containerRef, service, scopes); + return withRepositoryScopes(containerRef, service, identity, scopes); } /** @@ -117,7 +129,10 @@ public Scopes withRegistryScopes(Scope... scopes) { */ public Scopes withAddedRegistryScopes(Scope... newScopes) { return new Scopes( - containerRef, service, ScopeUtils.appendRepositoryScope(this.scopes, containerRef, newScopes)); + containerRef, + service, + identity, + ScopeUtils.appendRepositoryScope(this.scopes, containerRef, newScopes)); } /** @@ -129,7 +144,10 @@ public Scopes withAddedGlobalScopes(String... globalScopes) { List newScopes = new LinkedList<>(scopes); newScopes.addAll(List.of(globalScopes)); return new Scopes( - containerRef, service, newScopes.stream().sorted().distinct().toList()); + containerRef, + service, + identity, + newScopes.stream().sorted().distinct().toList()); } /** @@ -142,7 +160,7 @@ public Scopes withOnlyGlobalScopes() { .sorted() .distinct() .toList(); - return new Scopes(containerRef, service, globalScopes); + return new Scopes(containerRef, service, identity, globalScopes); } /** @@ -155,7 +173,7 @@ public Scopes withoutGlobalScopes() { .sorted() .distinct() .toList(); - return new Scopes(containerRef, service, nonGlobalScopes); + return new Scopes(containerRef, service, identity, nonGlobalScopes); } /** @@ -166,7 +184,7 @@ public Scopes withoutGlobalScopes() { public Scopes withNewScope(String scope) { List newScopes = new LinkedList<>(scopes); newScopes.add(scope); - return new Scopes(containerRef, service, ScopeUtils.cleanScopes(newScopes)); + return new Scopes(containerRef, service, identity, ScopeUtils.cleanScopes(newScopes)); } /** @@ -175,7 +193,16 @@ public Scopes withNewScope(String scope) { * @return A new Scopes object with the given service */ public Scopes withService(@Nullable String service) { - return new Scopes(containerRef, service, scopes); + return new Scopes(containerRef, service, identity, scopes); + } + + /** + * Return a new copy of the Scopes object bound to the given auth provider identity + * @param identity The identity + * @return The new Scopes object + */ + public Scopes withIdentity(@Nullable String identity) { + return new Scopes(containerRef, service, identity, scopes); } /** @@ -186,6 +213,14 @@ public Scopes withService(@Nullable String service) { return service; } + /** + * Get the auth provider identity marker bound to this scope, or {@code null} if not bound to any. + * @return The identity marker + */ + public @Nullable String getIdentity() { + return identity; + } + /** * Get the scopes * @return The scopes @@ -241,6 +276,7 @@ public boolean equals(Object o) { Scopes scopes1 = (Scopes) o; return Objects.equals(getScopes(), scopes1.getScopes()) && Objects.equals(getService(), scopes1.getService()) + && Objects.equals(getIdentity(), scopes1.getIdentity()) && Objects.equals( getContainerRef().getRegistry(), scopes1.getContainerRef().getRegistry()); @@ -248,14 +284,16 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(getScopes(), getService(), getContainerRef().getRegistry()); + return Objects.hash( + getScopes(), getService(), getIdentity(), getContainerRef().getRegistry()); } @Override public String toString() { return "Scopes{" + "scopes=" + scopes + ", service='" - + service + '\'' + ", registry=" + + service + '\'' + ", identity='" + + identity + '\'' + ", registry=" + containerRef.getRegistry() + '}'; } } diff --git a/src/test/java/land/oras/auth/AuthStoreAuthenticationProviderTest.java b/src/test/java/land/oras/auth/AuthStoreAuthenticationProviderTest.java index 17f01153..dfde8673 100644 --- a/src/test/java/land/oras/auth/AuthStoreAuthenticationProviderTest.java +++ b/src/test/java/land/oras/auth/AuthStoreAuthenticationProviderTest.java @@ -85,4 +85,28 @@ void testGetNullAuthHeader() throws Exception { void testDefaultLocation() { new AuthStoreAuthenticationProvider(); } + + @Test + void identityShouldFollowTheResolvedCredentialPerRegistry() { + ContainerRef alpine = ContainerRef.parse("%s/%s".formatted(REGISTRY, "alpine")); + ContainerRef busybox = ContainerRef.parse("%s/%s".formatted(REGISTRY, "busybox")); + + doReturn(new Credential("alice", "alice-pass")).when(mockAuthStore).get(alpine); + doReturn(new Credential("bob", "bob-pass")).when(mockAuthStore).get(busybox); + + AuthStoreAuthenticationProvider authProvider = new AuthStoreAuthenticationProvider(mockAuthStore); + + assertEquals("BASIC:alice", authProvider.getIdentity(alpine)); + assertEquals("BASIC:bob", authProvider.getIdentity(busybox)); + } + + @Test + void identityShouldFallBackToAnonymousWhenNoCredentialIsStored() { + doReturn(null).when(mockAuthStore).get(any(ContainerRef.class)); + + AuthStoreAuthenticationProvider authProvider = new AuthStoreAuthenticationProvider(mockAuthStore); + + assertEquals( + "BASIC:anonymous", authProvider.getIdentity(ContainerRef.parse("%s/%s".formatted(REGISTRY, "alpine")))); + } } diff --git a/src/test/java/land/oras/auth/BearerTokenProviderTest.java b/src/test/java/land/oras/auth/BearerTokenProviderTest.java index 842d5f70..5ab50f55 100644 --- a/src/test/java/land/oras/auth/BearerTokenProviderTest.java +++ b/src/test/java/land/oras/auth/BearerTokenProviderTest.java @@ -20,6 +20,9 @@ package land.oras.auth; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNull; import land.oras.ContainerRef; @@ -39,4 +42,27 @@ void shouldHaveNoAuthHeader() { BearerTokenProvider provider = new BearerTokenProvider(); assertNull(provider.getAuthHeader(containerRef), "No token should be returned"); } + + @Test + void identityShouldDependOnTheTokenValueAndNeverLeakIt() { + BearerTokenProvider noToken = new BearerTokenProvider(); + assertEquals("BEARER:none", noToken.getIdentity(containerRef)); + + BearerTokenProvider tokenA = new BearerTokenProvider("secret-token-a"); + BearerTokenProvider sameTokenA = new BearerTokenProvider("secret-token-a"); + BearerTokenProvider tokenB = new BearerTokenProvider("secret-token-b"); + + assertNotEquals(noToken.getIdentity(containerRef), tokenA.getIdentity(containerRef)); + assertEquals( + tokenA.getIdentity(containerRef), + sameTokenA.getIdentity(containerRef), + "Same token value should yield the same identity"); + assertNotEquals( + tokenA.getIdentity(containerRef), + tokenB.getIdentity(containerRef), + "Different token values are different identities"); + assertFalse( + tokenA.getIdentity(containerRef).contains("secret-token-a"), + "Identity must never contain the raw token"); + } } diff --git a/src/test/java/land/oras/auth/NoAuthProviderTest.java b/src/test/java/land/oras/auth/NoAuthProviderTest.java index c70ec03a..44773405 100644 --- a/src/test/java/land/oras/auth/NoAuthProviderTest.java +++ b/src/test/java/land/oras/auth/NoAuthProviderTest.java @@ -20,6 +20,7 @@ package land.oras.auth; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import land.oras.ContainerRef; @@ -35,4 +36,15 @@ void shouldHaveNoAuthHeader() { NoAuthProvider authProvider = new NoAuthProvider(); assertNull(authProvider.getAuthHeader(ContainerRef.parse("localhost:5000/foo/bar"))); } + + @Test + void shouldHaveDistinctIdentityFromCredentialedProviders() { + NoAuthProvider authProvider = new NoAuthProvider(); + ContainerRef registry = ContainerRef.parse("localhost:5000/foo/bar"); + assertEquals(AuthScheme.NONE.name(), authProvider.getIdentity(registry)); + assertEquals( + authProvider.getIdentity(registry), + authProvider.getIdentity(ContainerRef.parse("localhost:5000/other/repo")), + "Anonymous identity should not depend on the target repository"); + } } diff --git a/src/test/java/land/oras/auth/ScopesTest.java b/src/test/java/land/oras/auth/ScopesTest.java index ab6d9efc..66c89f05 100644 --- a/src/test/java/land/oras/auth/ScopesTest.java +++ b/src/test/java/land/oras/auth/ScopesTest.java @@ -42,11 +42,12 @@ void shouldBuildScopes() { assertSame(containerRef, scopes.getContainerRef()); assertEquals("localhost:5000", scopes.getRegistry()); assertEquals("docker", scopes.withService("docker").getService()); + assertNull(scopes.getIdentity(), "Identity should be null by default"); assertEquals( - "Scopes{scopes=[repository:library/test:pull], service='null', registry=localhost:5000}", + "Scopes{scopes=[repository:library/test:pull], service='null', identity='null', registry=localhost:5000}", scopes.toString()); assertEquals( - "Scopes{scopes=[repository:library/test:pull], service='docker', registry=localhost:5000}", + "Scopes{scopes=[repository:library/test:pull], service='docker', identity='null', registry=localhost:5000}", scopes.withService("docker").toString()); assertFalse(scopes.isGlobal(), "Scopes should not be global"); assertTrue( @@ -68,4 +69,43 @@ void shouldBuildScopes() { assertEquals(1, newScopes2.getScopes().size()); assertEquals("repository:library/test:pull,push", newScopes2.getScopes().get(0)); } + + @Test + void shouldCarryIdentityThroughAllWithers() { + ContainerRef containerRef = ContainerRef.parse("localhost:5000/library/test:latest"); + Scopes scopes = Scopes.of(containerRef, Scope.PULL).withIdentity("BASIC:alice"); + assertEquals("BASIC:alice", scopes.getIdentity()); + + // Every wither must preserve the identity bound on the original instance + assertEquals("BASIC:alice", scopes.withService("docker").getIdentity()); + assertEquals("BASIC:alice", scopes.withRegistryScopes(Scope.PUSH).getIdentity()); + assertEquals("BASIC:alice", scopes.withAddedRegistryScopes(Scope.PUSH).getIdentity()); + assertEquals("BASIC:alice", scopes.withAddedGlobalScopes("aws").getIdentity()); + assertEquals( + "BASIC:alice", + scopes.withAddedGlobalScopes("aws").withOnlyGlobalScopes().getIdentity()); + assertEquals( + "BASIC:alice", + scopes.withAddedGlobalScopes("aws").withoutGlobalScopes().getIdentity()); + assertEquals("BASIC:alice", scopes.withNewScope("aws").getIdentity()); + + // Overriding the identity replaces it, everything else stays the same + Scopes rebound = scopes.withIdentity("BASIC:bob"); + assertEquals("BASIC:bob", rebound.getIdentity()); + assertEquals(scopes.getScopes(), rebound.getScopes()); + } + + @Test + void scopesWithDifferentIdentityShouldNotBeEqual() { + ContainerRef containerRef = ContainerRef.parse("localhost:5000/library/test:latest"); + Scopes anonymous = Scopes.of(containerRef, Scope.PULL).withIdentity("NONE"); + Scopes alice = Scopes.of(containerRef, Scope.PULL).withIdentity("BASIC:alice"); + Scopes bob = Scopes.of(containerRef, Scope.PULL).withIdentity("BASIC:bob"); + Scopes aliceAgain = Scopes.of(containerRef, Scope.PULL).withIdentity("BASIC:alice"); + + assertNotEquals(anonymous, alice, "Anonymous and a credentialed identity must not collide"); + assertNotEquals(alice, bob, "Two distinct credentialed identities must not collide"); + assertEquals(alice, aliceAgain, "Same identity for the same scope should still be equal"); + assertEquals(alice.hashCode(), aliceAgain.hashCode(), "Equal scopes must have equal hash codes"); + } } diff --git a/src/test/java/land/oras/auth/TokenCacheTest.java b/src/test/java/land/oras/auth/TokenCacheTest.java index 71cf8809..1d8aba54 100644 --- a/src/test/java/land/oras/auth/TokenCacheTest.java +++ b/src/test/java/land/oras/auth/TokenCacheTest.java @@ -22,6 +22,7 @@ import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import io.micrometer.core.instrument.FunctionCounter; @@ -122,4 +123,34 @@ void shouldRetrieveTokenPullTokenUsingAllScope() throws InterruptedException { Scopes pullOnlyScopes = Scopes.of(containerRef, Scope.PULL); // Pull only assertEquals(tokenResponse, TokenCache.get(pullOnlyScopes), "Should retrieve the token using pull-only scopes"); } + + /** + * Regression test for a token cache poisoning scenario: a first, anonymous, request for a given + * registry/repository/action scope is answered with an (anonymously-scoped) token, which gets cached. A later + * request for the exact same scope, but resolved with real credentials, must NOT be served that stale anonymous + * token from the cache — it must be treated as a cache miss so the caller falls back to its own AuthProvider. + */ + @Test + void shouldNotServeATokenCachedForOneIdentityToADifferentIdentity() { + ContainerRef containerRef = ContainerRef.parse("docker.io/library/jenkins-common-configuration:main"); + HttpClient.TokenResponse anonymousToken = + new HttpClient.TokenResponse("anonymous-token", null, "prj-sso-oci", 3600, null); + + Scopes anonymousScope = + Scopes.of(containerRef, Scope.PULL).withService("prj-sso-oci").withIdentity("NONE"); + TokenCache.put(anonymousScope, anonymousToken); + + // Same registry, same repository, same pull action, but a different (credentialed) identity + Scopes credentialedScope = + Scopes.of(containerRef, Scope.PULL).withService("prj-sso-oci").withIdentity("BASIC:sa_elca-samples"); + + assertEquals( + anonymousToken, + TokenCache.get(anonymousScope), + "The anonymous identity should still get its own cached token"); + assertNull( + TokenCache.get(credentialedScope), + "A different identity must never be served a token cached for another identity, " + + "even for the exact same registry/repository/action scope"); + } } diff --git a/src/test/java/land/oras/auth/UsernamePasswordProviderTest.java b/src/test/java/land/oras/auth/UsernamePasswordProviderTest.java index a1cb4e22..87d7181d 100644 --- a/src/test/java/land/oras/auth/UsernamePasswordProviderTest.java +++ b/src/test/java/land/oras/auth/UsernamePasswordProviderTest.java @@ -21,6 +21,8 @@ package land.oras.auth; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import land.oras.ContainerRef; import org.junit.jupiter.api.Test; @@ -48,4 +50,22 @@ void shouldReturnCorrectValues() { assertEquals("user", authProvider.getUsername(), "Username should be correct"); assertEquals("pass", authProvider.getPassword(), "Password should be correct"); } + + @Test + void identityShouldDependOnUsernameOnlyAndNeverLeakThePassword() { + ContainerRef registry = ContainerRef.parse("localhost:5000/foo"); + AbstractUsernamePasswordProvider user = new UsernamePasswordProvider("user", "pass"); + AbstractUsernamePasswordProvider sameUserOtherPassword = new UsernamePasswordProvider("user", "other-pass"); + AbstractUsernamePasswordProvider otherUser = new UsernamePasswordProvider("other-user", "pass"); + + assertEquals( + user.getIdentity(registry), + sameUserOtherPassword.getIdentity(registry), + "Identity should not depend on the password"); + assertNotEquals( + user.getIdentity(registry), + otherUser.getIdentity(registry), + "Different usernames are different identities"); + assertFalse(user.getIdentity(registry).contains("pass"), "Identity must never contain the raw password"); + } }