GUACAMOLE-210: Properly generate and validate nonces.
Project: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/commit/aaf1b796 Tree: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/tree/aaf1b796 Diff: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/diff/aaf1b796 Branch: refs/heads/master Commit: aaf1b796f3201916b9a5e8269cefd9b88df183bc Parents: 4f8c853 Author: Michael Jumper <[email protected]> Authored: Sun Aug 27 23:58:15 2017 -0700 Committer: Michael Jumper <[email protected]> Committed: Mon Sep 25 13:06:45 2017 -0700 ---------------------------------------------------------------------- .../openid/AuthenticationProviderService.java | 10 +- .../OpenIDAuthenticationProviderModule.java | 2 + .../guacamole/auth/openid/form/TokenField.java | 44 ++---- .../auth/openid/token/NonceService.java | 135 +++++++++++++++++++ .../openid/token/TokenValidationService.java | 20 +++ 5 files changed, 180 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/aaf1b796/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java index 1423b8d..46e8b02 100644 --- a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java +++ b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/AuthenticationProviderService.java @@ -25,6 +25,7 @@ import java.util.Arrays; import javax.servlet.http.HttpServletRequest; import org.apache.guacamole.auth.openid.conf.ConfigurationService; import org.apache.guacamole.auth.openid.form.TokenField; +import org.apache.guacamole.auth.openid.token.NonceService; import org.apache.guacamole.auth.openid.token.TokenValidationService; import org.apache.guacamole.auth.openid.user.AuthenticatedUser; import org.apache.guacamole.GuacamoleException; @@ -53,6 +54,12 @@ public class AuthenticationProviderService { private ConfigurationService confService; /** + * Service for validating and generating unique nonce values. + */ + @Inject + private NonceService nonceService; + + /** * Service for validating received ID tokens. */ @Inject @@ -112,7 +119,8 @@ public class AuthenticationProviderService { new TokenField( confService.getAuthorizationEndpoint(), confService.getClientID(), - confService.getRedirectURI() + confService.getRedirectURI(), + nonceService.generate(30000 /* FIXME: Calculate appropriate value based on configuration */) ) })) http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/aaf1b796/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProviderModule.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProviderModule.java b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProviderModule.java index 9abd666..17510cb 100644 --- a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/OpenIDAuthenticationProviderModule.java @@ -21,6 +21,7 @@ package org.apache.guacamole.auth.openid; import com.google.inject.AbstractModule; import org.apache.guacamole.auth.openid.conf.ConfigurationService; +import org.apache.guacamole.auth.openid.token.NonceService; import org.apache.guacamole.auth.openid.token.TokenValidationService; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.environment.Environment; @@ -74,6 +75,7 @@ public class OpenIDAuthenticationProviderModule extends AbstractModule { // Bind openid-specific services bind(ConfigurationService.class); + bind(NonceService.class); bind(TokenValidationService.class); } http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/aaf1b796/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/form/TokenField.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/form/TokenField.java b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/form/TokenField.java index 3ef5d94..3f7c454 100644 --- a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/form/TokenField.java +++ b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/form/TokenField.java @@ -20,15 +20,12 @@ package org.apache.guacamole.auth.openid.form; import java.io.UnsupportedEncodingException; -import java.math.BigInteger; import java.net.URLEncoder; -import java.security.SecureRandom; import org.apache.guacamole.form.Field; /** - * Field definition which represents the token returned by an OpenID service. - * Within the user interface, this will be rendered as an appropriate "Log in - * with ..." button which links to the OpenID service. + * Field definition which represents the token returned by an OpenID Connect + * service. */ public class TokenField extends Field { @@ -44,29 +41,12 @@ public class TokenField extends Field { private final String authorizationURI; /** - * Cryptographically-secure random number generator for generating the - * required nonce. - */ - private static final SecureRandom random = new SecureRandom(); - - /** - * Generates a cryptographically-secure nonce value. The nonce is intended - * to be used to prevent replay attacks. - * - * @return - * A cryptographically-secure nonce value. - */ - private static String generateNonce() { - return new BigInteger(130, random).toString(32); - } - - /** - * Creates a new OpenID "id_token" field which links to the given OpenID - * service using the provided client ID. Successful authentication at the - * OpenID service will result in the client being redirected to the specified - * redirect URI. The OpenID token will be embedded in the fragment (the part - * following the hash symbol) of that URI, which the JavaScript side of - * this extension will move to the query parameters. + * Creates a new field which requests authentication via OpenID connect. + * Successful authentication at the OpenID Connect service will result in + * the client being redirected to the specified redirect URI. The OpenID + * token will be embedded in the fragment (the part following the hash + * symbol) of that URI, which the JavaScript side of this extension will + * move to the query parameters. * * @param authorizationEndpoint * The full URL of the endpoint accepting OpenID authentication @@ -80,9 +60,13 @@ public class TokenField extends Field { * @param redirectURI * The URI that the OpenID service should redirect to upon successful * authentication. + * + * @param nonce + * A random string unique to this request. To defend against replay + * attacks, this value must cease being valid after its first use. */ public TokenField(String authorizationEndpoint, String clientID, - String redirectURI) { + String redirectURI, String nonce) { // Init base field properties super(PARAMETER_NAME, "GUAC_OPENID_TOKEN"); @@ -94,7 +78,7 @@ public class TokenField extends Field { + "&response_type=id_token" + "&client_id=" + URLEncoder.encode(clientID, "UTF-8") + "&redirect_uri=" + URLEncoder.encode(redirectURI, "UTF-8") - + "&nonce=" + generateNonce(); + + "&nonce=" + nonce; } // Java is required to provide UTF-8 support http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/aaf1b796/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/NonceService.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/NonceService.java b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/NonceService.java new file mode 100644 index 0000000..778112a --- /dev/null +++ b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/NonceService.java @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.auth.openid.token; + +import com.google.inject.Singleton; +import java.math.BigInteger; +import java.security.SecureRandom; +import java.util.Iterator; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Service for generating and validating single-use random tokens (nonces). + */ +@Singleton +public class NonceService { + + /** + * Cryptographically-secure random number generator for generating the + * required nonce. + */ + private final SecureRandom random = new SecureRandom(); + + /** + * Map of all generated nonces to their corresponding expiration timestamps. + * This Map must be periodically swept of expired nonces to avoid growing + * without bound. + */ + private final Map<String, Long> nonces = new ConcurrentHashMap<String, Long>(); + + /** + * The timestamp of the last expired nonce sweep. + */ + private long lastSweep = System.currentTimeMillis(); + + /** + * The minimum amount of time to wait between sweeping expired nonces from + * the Map. + */ + private static final long SWEEP_INTERVAL = 60000; + + /** + * Iterates through the entire Map of generated nonces, removing any nonce + * that has exceeded its expiration timestamp. If insufficient time has + * elapsed since the last sweep, as dictated by SWEEP_INTERVAL, this + * function has no effect. + */ + private void sweepExpiredNonces() { + + // Do not sweep until enough time has elapsed since the last sweep + long currentTime = System.currentTimeMillis(); + if (currentTime - lastSweep < SWEEP_INTERVAL) + return; + + // Record time of sweep + lastSweep = currentTime; + + // For each stored nonce + Iterator<Map.Entry<String, Long>> entries = nonces.entrySet().iterator(); + while (entries.hasNext()) { + + // Remove all entries which have expired + Map.Entry<String, Long> current = entries.next(); + if (current.getValue() <= System.currentTimeMillis()) + entries.remove(); + + } + + } + + /** + * Generates a cryptographically-secure nonce value. The nonce is intended + * to be used to prevent replay attacks. + * + * @param maxAge + * The maximum amount of time that the generated nonce should remain + * valid, in milliseconds. + * + * @return + * A cryptographically-secure nonce value. + */ + public String generate(long maxAge) { + + // Sweep expired nonces if enough time has passed + sweepExpiredNonces(); + + // Generate and store nonce, along with expiration timestamp + String nonce = new BigInteger(130, random).toString(32); + nonces.put(nonce, System.currentTimeMillis() + maxAge); + return nonce; + + } + + /** + * Returns whether the give nonce value is valid. A nonce is valid if and + * only if it was generated by this instance of the NonceService. Testing + * nonce validity through this function immediately and permanently + * invalidates that nonce. + * + * @param nonce + * The nonce value to test. + * + * @return + * true if the provided nonce is valid, false otherwise. + */ + public boolean isValid(String nonce) { + + // Remove nonce, verifying whether it was present at all + Long expires = nonces.remove(nonce); + if (expires == null) + return false; + + // Nonce is only valid if it hasn't expired + return expires > System.currentTimeMillis(); + + } + +} http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/aaf1b796/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java ---------------------------------------------------------------------- diff --git a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java index 3e1a58d..3d41eba 100644 --- a/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java +++ b/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java @@ -52,6 +52,12 @@ public class TokenValidationService { private ConfigurationService confService; /** + * Service for validating and generating unique nonce values. + */ + @Inject + private NonceService nonceService; + + /** * Validates and parses the given ID token, returning the username contained * therein, as defined by the username claim type given in * guacamole.properties. If the username claim type is missing or the ID @@ -91,6 +97,20 @@ public class TokenValidationService { // Validate JWT JwtClaims claims = jwtConsumer.processToClaims(token); + // Verify a nonce is present + String nonce = claims.getStringClaimValue("nonce"); + if (nonce == null) { + logger.info("Rejected OpenID token without nonce."); + return null; + } + + // Verify that we actually generated the nonce, and that it has not + // already been used + if (!nonceService.isValid(nonce)) { + logger.debug("Rejected OpenID token with invalid/old nonce."); + return null; + } + // Pull username from claims String username = claims.getStringClaimValue(usernameClaim); if (username != null)
