diff --git a/documentation/jetty/modules/programming-guide/pages/security/siwe-support.adoc b/documentation/jetty/modules/programming-guide/pages/security/siwe-support.adoc index bd9cc690677d..9b403bc222d0 100644 --- a/documentation/jetty/modules/programming-guide/pages/security/siwe-support.adoc +++ b/documentation/jetty/modules/programming-guide/pages/security/siwe-support.adoc @@ -77,7 +77,7 @@ Authenticate New Users:: Domains:: * Init Param: org.eclipse.jetty.security.siwe.domains -* Description: This list of allowed domains to be declared in the `domain` field of the SIWE Message. If left blank this will allow all domains. +* Description: The list of allowed domains to be declared in the `domain` field of the SIWE Message. If left unconfigured, the `domain` field must match the authority (`host[:port]`) of the request being authenticated. Chain IDs:: * Init Param: org.eclipse.jetty.security.siwe.chainIds diff --git a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java index c5dba9d6626b..a5973f84235d 100644 --- a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java +++ b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java @@ -94,21 +94,9 @@ public void setLoginPath(String loginPath) } } - public boolean isLoginPage(String uri) + public boolean isLoginPage(String pathInContext) { - return matchURI(uri, _loginPath); - } - - private boolean matchURI(String uri, String path) - { - int jsc = uri.indexOf(path); - if (jsc < 0) - return false; - int e = jsc + path.length(); - if (e == uri.length()) - return true; - char c = uri.charAt(e); - return c == ';' || c == '#' || c == '/' || c == '?'; + return pathInContext != null && pathInContext.equals(_loginPath); } public void setDispatch(boolean dispatch) diff --git a/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java b/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java index 4cfa1a5551da..8993abb7e29b 100644 --- a/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java +++ b/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java @@ -17,11 +17,14 @@ import java.io.InputStream; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashSet; import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.function.Function; +import java.util.function.Predicate; import org.eclipse.jetty.http.HttpException; import org.eclipse.jetty.http.HttpHeader; @@ -49,12 +52,12 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Session; +import org.eclipse.jetty.util.AsciiLowerCaseSet; import org.eclipse.jetty.util.Blocker; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.CharsetStringBuilder.Iso88591StringBuilder; import org.eclipse.jetty.util.Fields; -import org.eclipse.jetty.util.IncludeExcludeSet; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.UrlEncoded; @@ -86,8 +89,8 @@ public class EthereumAuthenticator extends LoginAuthenticator implements Dumpabl private static final String DEFAULT_NONCE_PATH = "/auth/nonce"; private static final String NONCE_SET_ATTR = "org.eclipse.jetty.security.siwe.nonce"; - private final IncludeExcludeSet _chainIds = new IncludeExcludeSet<>(); - private final IncludeExcludeSet _domains = new IncludeExcludeSet<>(); + private final Set _chainIds = new HashSet<>(); + private final Set _domains = new AsciiLowerCaseSet(); private String _loginPath; private String _authenticationPath = DEFAULT_AUTHENTICATION_PATH; @@ -105,12 +108,12 @@ public EthereumAuthenticator() public void includeDomains(String... domains) { - _domains.include(domains); + _domains.addAll(Arrays.asList(domains)); } public void includeChainIds(String... chainIds) { - _chainIds.include(chainIds); + _chainIds.addAll(Arrays.asList(chainIds)); } @Override @@ -522,7 +525,7 @@ private AuthenticationState validateSignInWithEthereumToken(SignInWithEthereumTo try { - siwe.validate(signedMessage, nonce -> redeemNonce(session, nonce), _domains, _chainIds); + siwe.validate(signedMessage, nonce -> redeemNonce(session, nonce), getDomainValidator(request), getChainIdValidator()); } catch (Throwable t) { @@ -532,15 +535,36 @@ private AuthenticationState validateSignInWithEthereumToken(SignInWithEthereumTo return null; } + private Predicate getChainIdValidator() + { + if (_chainIds.isEmpty()) + return chainId -> true; + return _chainIds::contains; + } + + private Predicate getDomainValidator(Request request) + { + if (!_domains.isEmpty()) + return _domains::contains; + + // Default to use the authority of the request. + String host = Request.getServerName(request); + int port = Request.getServerPort(request); + int defaultPort = request.isSecure() ? 443 : 80; + String authority = StringUtil.isBlank(host) ? null + : (port <= 0 || port == defaultPort) ? host : host + ":" + port; + return domain -> authority != null && StringUtil.asciiEqualsIgnoreCase(authority, domain); + } + @Override public AuthenticationState validateRequest(Request request, Response response, Callback callback) throws ServerAuthException { if (LOG.isDebugEnabled()) LOG.debug("validateRequest({},{})", request, response); - String uri = request.getHttpURI().toString(); - if (uri == null) - uri = "/"; + String pathInContext = Request.getPathInContext(request); + if (pathInContext == null) + pathInContext = "/"; try { @@ -552,9 +576,9 @@ public AuthenticationState validateRequest(Request request, Response response, C return sendError(request, response, callback, "session could not be created"); } - if (isNonceRequest(uri)) + if (isNonceRequest(pathInContext)) return handleNonceRequest(request, response, callback); - if (isAuthenticationRequest(uri)) + if (isAuthenticationRequest(pathInContext)) { if (LOG.isDebugEnabled()) LOG.debug("authentication request"); @@ -704,31 +728,29 @@ protected Fields getParameters(Request request) } } - public boolean isLoginPage(String uri) + public boolean isLoginPage(String pathInContext) { - return matchURI(uri, _loginPath); + return pathInContext != null && pathInContext.equals(_loginPath); } - public boolean isAuthenticationRequest(String uri) + public boolean isAuthenticationRequest(String pathInContext) { - return matchURI(uri, _authenticationPath); + return matchURI(pathInContext, _authenticationPath); } - public boolean isNonceRequest(String uri) + public boolean isNonceRequest(String pathInContext) { - return matchURI(uri, _noncePath); + return matchURI(pathInContext, _noncePath); } - private boolean matchURI(String uri, String path) + private boolean matchURI(String pathInContext, String path) { - int jsc = uri.indexOf(path); - if (jsc < 0) + if (pathInContext == null || !pathInContext.startsWith(path)) return false; - int e = jsc + path.length(); - if (e == uri.length()) + int e = path.length(); + if (e == pathInContext.length()) return true; - char c = uri.charAt(e); - return c == ';' || c == '#' || c == '/' || c == '?'; + return pathInContext.charAt(e) == '/'; } public boolean isErrorPage(String pathInContext) diff --git a/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/internal/SignInWithEthereumToken.java b/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/internal/SignInWithEthereumToken.java index a4e2a2780e1e..dd303c96b1c4 100644 --- a/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/internal/SignInWithEthereumToken.java +++ b/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/internal/SignInWithEthereumToken.java @@ -21,7 +21,6 @@ import org.eclipse.jetty.security.ServerAuthException; import org.eclipse.jetty.security.siwe.EthereumAuthenticator; -import org.eclipse.jetty.util.IncludeExcludeSet; import org.eclipse.jetty.util.StringUtil; /** @@ -104,13 +103,14 @@ public static SignInWithEthereumToken from(String message) /** * @param signedMessage the {@link EthereumAuthenticator.SignedMessage}. * @param validateNonce a {@link Predicate} used to validate the nonce. - * @param domains the {@link IncludeExcludeSet} used to validate the domain. - * @param chainIds the {@link IncludeExcludeSet} used to validate the chainId. + * @param validateDomain a {@link Predicate} used to validate the domain. + * @param validateChainId a {@link Predicate} used to validate the chainId. * @throws ServerAuthException if the {@link EthereumAuthenticator.SignedMessage} fails validation. */ - public void validate(EthereumAuthenticator.SignedMessage signedMessage, Predicate validateNonce, - IncludeExcludeSet domains, - IncludeExcludeSet chainIds) throws ServerAuthException + public void validate(EthereumAuthenticator.SignedMessage signedMessage, + Predicate validateNonce, + Predicate validateDomain, + Predicate validateChainId) throws ServerAuthException { if (validateNonce != null && !validateNonce.test(nonce())) throw new ServerAuthException("invalid nonce " + nonce); @@ -136,9 +136,9 @@ public void validate(EthereumAuthenticator.SignedMessage signedMessage, Predicat throw new ServerAuthException("SIWE message not yet valid"); } - if (domains != null && !domains.test(domain())) + if (validateDomain != null && !validateDomain.test(domain())) throw new ServerAuthException("unregistered domain: " + domain()); - if (chainIds != null && !chainIds.test(chainId())) + if (validateChainId != null && !validateChainId.test(chainId())) throw new ServerAuthException("unregistered chainId: " + chainId()); } } diff --git a/jetty-integrations/jetty-ethereum/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumTest.java b/jetty-integrations/jetty-ethereum/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumTest.java index 20b88d377449..75ad687a33b4 100644 --- a/jetty-integrations/jetty-ethereum/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumTest.java +++ b/jetty-integrations/jetty-ethereum/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumTest.java @@ -232,26 +232,69 @@ public void testEnforceDomain() throws Exception assertThat(response.getContentAsString(), containsString("UserPrincipal: " + _credentials.getAddress())); } + @Test + public void testDomainBoundToRequestByDefault() throws Exception + { + // A message asserting an unknown domain is rejected. + String nonce = getNonce(); + String siweMessage = SignInWithEthereumGenerator.generateMessage(null, "example.com", _credentials.getAddress(), nonce); + ContentResponse response = sendAuthRequest(_credentials.signMessage(siweMessage)); + assertThat(response.getStatus(), equalTo(HttpStatus.FORBIDDEN_403)); + assertThat(response.getContentAsString(), containsString("unregistered domain")); + + // A message asserting the request authority is accepted. + nonce = getNonce(); + siweMessage = SignInWithEthereumGenerator.generateMessage(null, "localhost:" + _connector.getLocalPort(), _credentials.getAddress(), nonce); + response = sendAuthRequest(_credentials.signMessage(siweMessage)); + assertThat(response.getStatus(), equalTo(HttpStatus.OK_200)); + assertThat(response.getContentAsString(), containsString("UserPrincipal: " + _credentials.getAddress())); + } + @Test public void testEnforceChainId() throws Exception { _authenticator.includeChainIds("1"); + String domain = "localhost:" + _connector.getLocalPort(); // Test login with invalid chainId. String nonce = getNonce(); - String siweMessage = SignInWithEthereumGenerator.generateMessage(null, "localhost", _credentials.getAddress(), nonce, "2"); + String siweMessage = SignInWithEthereumGenerator.generateMessage(null, domain, _credentials.getAddress(), nonce, "2"); ContentResponse response = sendAuthRequest(_credentials.signMessage(siweMessage)); assertThat(response.getStatus(), equalTo(HttpStatus.FORBIDDEN_403)); assertThat(response.getContentAsString(), containsString("unregistered chainId")); // Test login with valid chainId. nonce = getNonce(); - siweMessage = SignInWithEthereumGenerator.generateMessage(null, "localhost", _credentials.getAddress(), nonce, "1"); + siweMessage = SignInWithEthereumGenerator.generateMessage(null, domain, _credentials.getAddress(), nonce, "1"); response = sendAuthRequest(_credentials.signMessage(siweMessage)); assertThat(response.getStatus(), equalTo(HttpStatus.OK_200)); assertThat(response.getContentAsString(), containsString("UserPrincipal: " + _credentials.getAddress())); } + @Test + public void testPathMatching() throws Exception + { + _client.setFollowRedirects(false); + + // Protected resources containing substring of a protected path must redirect to /login. + for (String path : new String[]{"/admin/login", "/login/secret", "/admin/auth/login", "/admin/auth/nonce", "/admin/error"}) + { + ContentResponse response = _client.GET("http://localhost:" + _connector.getLocalPort() + path); + assertTrue(HttpStatus.isRedirection(response.getStatus()), path + " HttpStatus was not redirect: " + response.getStatus()); + assertThat(path, response.getHeaders().get(HttpHeader.LOCATION), equalTo("/login")); + } + + // The exact login path is allowed without authentication. + ContentResponse response = _client.GET("http://localhost:" + _connector.getLocalPort() + "/login"); + assertThat(response.getStatus(), equalTo(HttpStatus.OK_200)); + assertThat(response.getContentAsString(), equalTo("Please Login")); + + // The exact error path is allowed without authentication. + response = _client.GET("http://localhost:" + _connector.getLocalPort() + "/error?" + EthereumAuthenticator.ERROR_PARAMETER + "=oops"); + assertThat(response.getStatus(), equalTo(HttpStatus.FORBIDDEN_403)); + assertThat(response.getContentAsString(), equalTo("oops")); + } + private ContentResponse sendAuthRequest(EthereumAuthenticator.SignedMessage signedMessage) throws ExecutionException, InterruptedException, TimeoutException { MultiPartRequestContent content = new MultiPartRequestContent();