From a5e747d579b013aa4545913d714481665205ec83 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Tue, 23 May 2023 20:25:21 -0400 Subject: [PATCH 1/4] JCL-356: Prohibit weak authorization mechanisms --- .../client/auth/ReactiveAuthorization.java | 13 ++- .../inrupt/client/auth/BasicAuthProvider.java | 77 +++++++++++++ .../auth/ReactiveAuthorizationTest.java | 101 ++++++++++++++++++ ...m.inrupt.client.spi.AuthenticationProvider | 1 + 4 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 api/src/test/java/com/inrupt/client/auth/BasicAuthProvider.java create mode 100644 api/src/test/java/com/inrupt/client/auth/ReactiveAuthorizationTest.java create mode 100644 api/src/test/resources/META-INF/services/com.inrupt.client.spi.AuthenticationProvider diff --git a/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java b/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java index 6af07618e07..a0b25944c67 100644 --- a/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java +++ b/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java @@ -34,6 +34,7 @@ import java.util.ServiceLoader; import java.util.Set; import java.util.TreeMap; +import java.util.TreeSet; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; @@ -64,9 +65,12 @@ public ReactiveAuthorization() { final ServiceLoader loader = ServiceLoader.load(AuthenticationProvider.class, ReactiveAuthorization.class.getClassLoader()); + final Set prohibited = getProhibitedSchemes(); for (final AuthenticationProvider provider : loader) { for (final String scheme : provider.getSchemes()) { - registry.put(scheme, provider); + if (!prohibited.contains(scheme)) { + registry.put(scheme, provider); + } } } } @@ -117,5 +121,12 @@ static boolean sessionSupportsScheme(final Session session, final String scheme) } return session.supportedSchemes().contains(scheme); } + + static Set getProhibitedSchemes() { + final Set prohibited = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + prohibited.add("Basic"); + prohibited.add("Digest"); + return prohibited; + } } diff --git a/api/src/test/java/com/inrupt/client/auth/BasicAuthProvider.java b/api/src/test/java/com/inrupt/client/auth/BasicAuthProvider.java new file mode 100644 index 00000000000..aa2e8e5c326 --- /dev/null +++ b/api/src/test/java/com/inrupt/client/auth/BasicAuthProvider.java @@ -0,0 +1,77 @@ +/* + * Copyright 2023 Inrupt Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A + * PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.inrupt.client.auth; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.inrupt.client.Request; +import com.inrupt.client.spi.AuthenticationProvider; + +import java.net.URI; +import java.time.Instant; +import java.util.Base64; +import java.util.Collections; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; + +public class BasicAuthProvider implements AuthenticationProvider { + + private static final String BASIC = "Basic"; + + @Override + public String getScheme() { + return BASIC; + } + + @Override + public Set getSchemes() { + return Collections.singleton(BASIC); + } + + @Override + public Authenticator getAuthenticator(final Challenge challenge) { + return new BasicAuthenticator(); + } + + public class BasicAuthenticator implements Authenticator { + @Override + public String getName() { + return "BasicAuth"; + } + + @Override + public int getPriority() { + return 100; + } + + @Override + public CompletionStage authenticate(final Session session, final Request request, + final Set algorithms) { + final URI issuer = URI.create("https://issuer.test"); + final URI agent = URI.create("https://id.test/username"); + final Instant expiration = Instant.now().plusSeconds(300); + final String token = Base64.getUrlEncoder().withoutPadding() + .encodeToString("username:password".getBytes(UTF_8)); + return CompletableFuture.completedFuture(new Credential("Basic", issuer, token, expiration, agent, null)); + } + } +} diff --git a/api/src/test/java/com/inrupt/client/auth/ReactiveAuthorizationTest.java b/api/src/test/java/com/inrupt/client/auth/ReactiveAuthorizationTest.java new file mode 100644 index 00000000000..44b4de7bdd9 --- /dev/null +++ b/api/src/test/java/com/inrupt/client/auth/ReactiveAuthorizationTest.java @@ -0,0 +1,101 @@ +/* + * Copyright 2023 Inrupt Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A + * PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.inrupt.client.auth; + +import static org.junit.jupiter.api.Assertions.*; + +import com.inrupt.client.Request; + +import java.net.URI; +import java.util.Collection; +import java.util.Collections; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; + +import org.junit.jupiter.api.Test; + +class ReactiveAuthorizationTest { + + @Test + void testProhibitedAuth() { + final ReactiveAuthorization auth = new ReactiveAuthorization(); + final Session session = new BasicAuthSession(); + final Request req = Request.newBuilder(URI.create("https://storage.example")).build(); + final Optional credential = auth.negotiate(session, req, + Collections.singleton(Challenge.of("Basic"))).toCompletableFuture().join(); + assertFalse(credential.isPresent()); + } + + static class BasicAuthSession implements Session { + + @Override + public String getId() { + return UUID.randomUUID().toString(); + } + + @Override + public Optional getPrincipal() { + return Optional.empty(); + } + + @Override + public Set supportedSchemes() { + return Collections.singleton("Basic"); + } + + @Override + public Optional getCredential(final URI name, final URI uri) { + return Optional.empty(); + } + + @Override + public Optional fromCache(final Request request) { + return Optional.empty(); + } + + @Override + public Optional generateProof(final String jkt, final Request request) { + return Optional.empty(); + } + + @Override + public Optional selectThumbprint(final Collection algorithms) { + return Optional.empty(); + } + + @Override + public CompletionStage> authenticate(final Authenticator authenticator, + final Request request, final Set algorithms) { + return authenticator.authenticate(this, request, algorithms).thenApply(Optional::ofNullable); + } + + /* deprecated */ + @Override + public CompletionStage> authenticate(final Request request, + final Set algorithms) { + return CompletableFuture.completedFuture(Optional.empty()); + } + } + +} diff --git a/api/src/test/resources/META-INF/services/com.inrupt.client.spi.AuthenticationProvider b/api/src/test/resources/META-INF/services/com.inrupt.client.spi.AuthenticationProvider new file mode 100644 index 00000000000..1820382d683 --- /dev/null +++ b/api/src/test/resources/META-INF/services/com.inrupt.client.spi.AuthenticationProvider @@ -0,0 +1 @@ +com.inrupt.client.auth.BasicAuthProvider From 3150dcc39805426351baa4193f1768644db4593b Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Wed, 24 May 2023 07:44:41 -0400 Subject: [PATCH 2/4] Code review --- .../java/com/inrupt/client/auth/ReactiveAuthorization.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java b/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java index a0b25944c67..7f238695cff 100644 --- a/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java +++ b/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java @@ -69,7 +69,12 @@ public ReactiveAuthorization() { for (final AuthenticationProvider provider : loader) { for (final String scheme : provider.getSchemes()) { if (!prohibited.contains(scheme)) { + LOGGER.debug("Registering {} scheme via {} authentication provider", scheme, + provider.getClass().getSimpleName()); registry.put(scheme, provider); + } else { + LOGGER.debug("Omitting {} scheme via {} authentication provider", scheme, + provider.getClass().getSimpleName()); } } } From 32d258aa0906232e20e21f610693c85ca7f10155 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Wed, 24 May 2023 07:48:06 -0400 Subject: [PATCH 3/4] add reset method --- .../com/inrupt/client/auth/ReactiveAuthorizationTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/api/src/test/java/com/inrupt/client/auth/ReactiveAuthorizationTest.java b/api/src/test/java/com/inrupt/client/auth/ReactiveAuthorizationTest.java index 44b4de7bdd9..ef924b3678d 100644 --- a/api/src/test/java/com/inrupt/client/auth/ReactiveAuthorizationTest.java +++ b/api/src/test/java/com/inrupt/client/auth/ReactiveAuthorizationTest.java @@ -69,6 +69,11 @@ public Optional getCredential(final URI name, final URI uri) { return Optional.empty(); } + @Override + public void reset() { + /* no-op */ + } + @Override public Optional fromCache(final Request request) { return Optional.empty(); From df1a0283a72e7910fa08d1a27df446482ea949b7 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Wed, 24 May 2023 08:17:40 -0400 Subject: [PATCH 4/4] Code review --- .../java/com/inrupt/client/auth/ReactiveAuthorization.java | 6 ++++++ .../java/com/inrupt/client/spi/AuthenticationProvider.java | 3 +++ 2 files changed, 9 insertions(+) diff --git a/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java b/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java index 7f238695cff..ba11d7fcb6f 100644 --- a/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java +++ b/api/src/main/java/com/inrupt/client/auth/ReactiveAuthorization.java @@ -44,6 +44,10 @@ /** * A class for negotiating for a supported {@link AuthenticationProvider} based on the {@code WWW-Authenticate} * headers received from a resource server. + * + *

In general, any authorization mechanism loaded via the {@link ServiceLoader} will be available for use + * during the challenge-response negotiation with a server. There are, however, certain known weak mechanisms + * such as Basic auth and Digest auth that are explicitly excluded. */ public class ReactiveAuthorization { @@ -60,6 +64,8 @@ public class ReactiveAuthorization { /** * Create a new authorization handler, loading any {@link AuthenticationProvider} implementations * via the {@link ServiceLoader}. + * + *

Known weak authorization mechanisms such as {@code Basic} and {@code Digest} are explicitly omitted. */ public ReactiveAuthorization() { final ServiceLoader loader = ServiceLoader.load(AuthenticationProvider.class, diff --git a/api/src/main/java/com/inrupt/client/spi/AuthenticationProvider.java b/api/src/main/java/com/inrupt/client/spi/AuthenticationProvider.java index 9648ed1c58d..6dcf1d1c14b 100644 --- a/api/src/main/java/com/inrupt/client/spi/AuthenticationProvider.java +++ b/api/src/main/java/com/inrupt/client/spi/AuthenticationProvider.java @@ -27,6 +27,9 @@ /** * An authentication mechanism that knows how to authenticate over network connections. + * + *

Please note that the {@link com.inrupt.client.auth.ReactiveAuthorization} class + * explicitly prohibits the use of {@code Basic} and {@code Digest} authorization schemes. */ public interface AuthenticationProvider {