KEYCLOAK-2106 HTTP 500 for unparsable refresh tokens

keycloak · Nov 27, 2015 · c83e3bd · c83e3bd
1 parent 5ea880c
commit c83e3bd
Show file tree

Hide file tree

Showing 20 changed files with 192 additions and 140 deletions.
diff --git a/broker/oidc/src/main/java/org/keycloak/broker/oidc/KeycloakOIDCIdentityProvider.java b/broker/oidc/src/main/java/org/keycloak/broker/oidc/KeycloakOIDCIdentityProvider.java
@@ -5,6 +5,7 @@
 import org.keycloak.constants.AdapterConstants;
 import org.keycloak.events.EventBuilder;
 import org.keycloak.jose.jws.JWSInput;
+import org.keycloak.jose.jws.JWSInputException;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserSessionModel;
 import org.keycloak.representations.AccessTokenResponse;
@@ -51,7 +52,13 @@ public KeycloakEndpoint(AuthenticationCallback callback, RealmModel realm, Event
         @POST
         @Path(AdapterConstants.K_LOGOUT)
         public Response backchannelLogout(String input) {
-            JWSInput token = new JWSInput(input);
+            JWSInput token = null;
+            try {
+                token = new JWSInput(input);
+            } catch (JWSInputException e) {
+                logger.warn("Failed to verify logout request");
+                return Response.status(400).build();
+            }
             PublicKey key = getExternalIdpKey();
             if (key != null) {
                 if (!verify(token, key)) {

diff --git a/broker/oidc/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java b/broker/oidc/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java
@@ -29,6 +29,7 @@
 import org.keycloak.events.EventBuilder;
 import org.keycloak.events.EventType;
 import org.keycloak.jose.jws.JWSInput;
+import org.keycloak.jose.jws.JWSInputException;
 import org.keycloak.jose.jws.crypto.RSAProvider;
 import org.keycloak.models.ClientSessionModel;
 import org.keycloak.models.RealmModel;
@@ -282,40 +283,41 @@ protected JsonWebToken validateToken(PublicKey key, String encodedToken) {
             throw new IdentityBrokerException("No token from server.");
         }
 
+        JsonWebToken token;
         try {
             JWSInput jws = new JWSInput(encodedToken);
             if (!verify(jws, key)) {
                 throw new IdentityBrokerException("token signature validation failed");
             }
-            JsonWebToken token = jws.readJsonContent(JsonWebToken.class);
+            token = jws.readJsonContent(JsonWebToken.class);
+        } catch (JWSInputException e) {
+            throw new IdentityBrokerException("Invalid token", e);
+        }
 
-            String iss = token.getIssuer();
+        String iss = token.getIssuer();
 
-            if (!token.hasAudience(getConfig().getClientId())) {
-                throw new IdentityBrokerException("Wrong audience from token.");
-            }
+        if (!token.hasAudience(getConfig().getClientId())) {
+            throw new IdentityBrokerException("Wrong audience from token.");
+        }
 
-            if (!token.isActive()) {
-                throw new IdentityBrokerException("Token is no longer valid");
-            }
+        if (!token.isActive()) {
+            throw new IdentityBrokerException("Token is no longer valid");
+        }
 
-            String trustedIssuers = getConfig().getIssuer();
+        String trustedIssuers = getConfig().getIssuer();
 
-            if (trustedIssuers != null) {
-                String[] issuers = trustedIssuers.split(",");
+        if (trustedIssuers != null) {
+            String[] issuers = trustedIssuers.split(",");
 
-                for (String trustedIssuer : issuers) {
-                    if (iss != null && iss.equals(trustedIssuer.trim())) {
-                        return token;
-                    }
+            for (String trustedIssuer : issuers) {
+                if (iss != null && iss.equals(trustedIssuer.trim())) {
+                    return token;
                 }
-
-                throw new IdentityBrokerException("Wrong issuer from token. Got: " + iss + " expected: " + getConfig().getIssuer());
             }
-            return token;
-        } catch (IOException e) {
-            throw new IdentityBrokerException("Could not decode token.", e);
+
+            throw new IdentityBrokerException("Wrong issuer from token. Got: " + iss + " expected: " + getConfig().getIssuer());
         }
+        return token;
     }
 
     @Override

diff --git a/core/src/main/java/org/keycloak/RSATokenVerifier.java b/core/src/main/java/org/keycloak/RSATokenVerifier.java
@@ -2,6 +2,7 @@
 
 import org.keycloak.common.VerificationException;
 import org.keycloak.jose.jws.JWSInput;
+import org.keycloak.jose.jws.JWSInputException;
 import org.keycloak.jose.jws.crypto.RSAProvider;
 import org.keycloak.representations.AccessToken;
 import org.keycloak.util.TokenUtil;
@@ -22,15 +23,15 @@ public static AccessToken verifyToken(String tokenString, PublicKey realmKey, St
         JWSInput input = null;
         try {
             input = new JWSInput(tokenString);
-        } catch (Exception e) {
+        } catch (JWSInputException e) {
             throw new VerificationException("Couldn't parse token", e);
         }
         if (!isPublicKeyValid(input, realmKey)) throw new VerificationException("Invalid token signature.");
 
         AccessToken token;
         try {
             token = input.readJsonContent(AccessToken.class);
-        } catch (IOException e) {
+        } catch (JWSInputException e) {
             throw new VerificationException("Couldn't parse token signature", e);
         }
         String user = token.getSubject();

diff --git a/core/src/main/java/org/keycloak/jose/jws/JWSInput.java b/core/src/main/java/org/keycloak/jose/jws/JWSInput.java
@@ -21,14 +21,14 @@ public class JWSInput {
     byte[] signature;
 
 
-    public JWSInput(String wire) {
-        this.wireString = wire;
-        String[] parts = wire.split("\\.");
-        if (parts.length < 2 || parts.length > 3) throw new IllegalArgumentException("Parsing error");
-        encodedHeader = parts[0];
-        encodedContent = parts[1];
-        encodedSignatureInput = encodedHeader + '.' + encodedContent;
+    public JWSInput(String wire) throws JWSInputException {
         try {
+            this.wireString = wire;
+            String[] parts = wire.split("\\.");
+            if (parts.length < 2 || parts.length > 3) throw new IllegalArgumentException("Parsing error");
+            encodedHeader = parts[0];
+            encodedContent = parts[1];
+            encodedSignatureInput = encodedHeader + '.' + encodedContent;
             content = Base64Url.decode(encodedContent);
             if (parts.length > 2) {
                 encodedSignature = parts[2];
@@ -37,8 +37,8 @@ public JWSInput(String wire) {
             }
             byte[] headerBytes = Base64Url.decode(encodedHeader);
             header = JsonSerialization.readValue(headerBytes, JWSHeader.class);
-        } catch (Exception e) {
-            throw new RuntimeException(e);
+        } catch (Throwable t) {
+            throw new JWSInputException(t);
         }
     }
 
@@ -80,8 +80,12 @@ public boolean verify(String key) {
         return header.getAlgorithm().getProvider().verify(this, key);
     }
 
-    public <T> T readJsonContent(Class<T> type) throws IOException {
-        return JsonSerialization.readValue(content, type);
+    public <T> T readJsonContent(Class<T> type) throws JWSInputException {
+        try {
+            return JsonSerialization.readValue(content, type);
+        } catch (IOException e) {
+            throw new JWSInputException(e);
+        }
     }
 
     public String readContentAsString() {

diff --git a/core/src/main/java/org/keycloak/jose/jws/JWSInputException.java b/core/src/main/java/org/keycloak/jose/jws/JWSInputException.java
@@ -0,0 +1,18 @@
+package org.keycloak.jose.jws;
+
+/**
+ * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
+ */
+public class JWSInputException extends Exception {
+
+    public JWSInputException(String s) {
+        super(s);
+    }
+
+    public JWSInputException() {
+    }
+
+    public JWSInputException(Throwable throwable) {
+        super(throwable);
+    }
+}
diff --git a/core/src/main/java/org/keycloak/jose/jws/crypto/RSAProvider.java b/core/src/main/java/org/keycloak/jose/jws/crypto/RSAProvider.java
@@ -64,7 +64,7 @@ public static boolean verify(JWSInput input, PublicKey publicKey) {
             verifier.update(input.getEncodedSignatureInput().getBytes("UTF-8"));
             return verifier.verify(input.getSignature());
         } catch (Exception e) {
-            throw new RuntimeException(e);
+            return false;
         }
 
     }

diff --git a/core/src/main/java/org/keycloak/util/TokenUtil.java b/core/src/main/java/org/keycloak/util/TokenUtil.java
@@ -4,6 +4,7 @@
 
 import org.keycloak.OAuth2Constants;
 import org.keycloak.jose.jws.JWSInput;
+import org.keycloak.jose.jws.JWSInputException;
 import org.keycloak.representations.RefreshToken;
 
 /**
@@ -41,11 +42,15 @@ public static boolean isOfflineTokenRequested(String scopeParam) {
      * @param decodedToken
      * @return
      */
-    public static RefreshToken getRefreshToken(byte[] decodedToken) throws IOException {
-        return JsonSerialization.readValue(decodedToken, RefreshToken.class);
+    public static RefreshToken getRefreshToken(byte[] decodedToken) throws JWSInputException {
+        try {
+            return JsonSerialization.readValue(decodedToken, RefreshToken.class);
+        } catch (IOException e) {
+            throw new JWSInputException(e);
+        }
     }
 
-    public static RefreshToken getRefreshToken(String refreshToken) throws IOException {
+    public static RefreshToken getRefreshToken(String refreshToken) throws JWSInputException {
         byte[] encodedContent = new JWSInput(refreshToken).getContent();
         return getRefreshToken(encodedContent);
     }
@@ -56,13 +61,9 @@ public static RefreshToken getRefreshToken(String refreshToken) throws IOExcepti
      * @param refreshToken
      * @return
      */
-    public static boolean isOfflineToken(String refreshToken) {
-        try {
-            RefreshToken token = getRefreshToken(refreshToken);
-            return token.getType().equals(TOKEN_TYPE_OFFLINE);
-        } catch (IOException ioe) {
-            throw new RuntimeException(ioe);
-        }
+    public static boolean isOfflineToken(String refreshToken) throws JWSInputException {
+        RefreshToken token = getRefreshToken(refreshToken);
+        return token.getType().equals(TOKEN_TYPE_OFFLINE);
     }
 
 }
diff --git a/...ate/offline-access-app/src/main/java/org/keycloak/example/OfflineAccessPortalServlet.java b/...ate/offline-access-app/src/main/java/org/keycloak/example/OfflineAccessPortalServlet.java
@@ -23,6 +23,7 @@
 import org.keycloak.adapters.RefreshableKeycloakSecurityContext;
 import org.keycloak.adapters.ServerRequest;
 import org.keycloak.adapters.spi.LogoutError;
+import org.keycloak.jose.jws.JWSInputException;
 import org.keycloak.representations.AccessTokenResponse;
 import org.keycloak.representations.RefreshToken;
 import org.keycloak.util.JsonSerialization;
@@ -49,40 +50,44 @@ public void destroy() {
 
     @Override
     protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+        try {
+            if (req.getRequestURI().endsWith("/login")) {
+                storeToken(req);
+                req.getRequestDispatcher("/WEB-INF/pages/loginCallback.jsp").forward(req, resp);
+                return;
+            }
 
-        if (req.getRequestURI().endsWith("/login")) {
-            storeToken(req);
-            req.getRequestDispatcher("/WEB-INF/pages/loginCallback.jsp").forward(req, resp);
-            return;
-        }
+            String refreshToken = RefreshTokenDAO.loadToken();
+            String refreshTokenInfo;
+            boolean savedTokenAvailable;
+            if (refreshToken == null) {
+                refreshTokenInfo = "No token saved in database. Please login first";
+                savedTokenAvailable = false;
+            } else {
+                RefreshToken refreshTokenDecoded = null;
+                    refreshTokenDecoded = TokenUtil.getRefreshToken(refreshToken);
+                String exp = (refreshTokenDecoded.getExpiration() == 0) ? "NEVER" : Time.toDate(refreshTokenDecoded.getExpiration()).toString();
+                refreshTokenInfo = String.format("<p>Type: %s</p><p>ID: %s</p><p>Expires: %s</p>", refreshTokenDecoded.getType(), refreshTokenDecoded.getId(), exp);
+                savedTokenAvailable = true;
+            }
+            req.setAttribute("tokenInfo", refreshTokenInfo);
+            req.setAttribute("savedTokenAvailable", savedTokenAvailable);
 
-        String refreshToken = RefreshTokenDAO.loadToken();
-        String refreshTokenInfo;
-        boolean savedTokenAvailable;
-        if (refreshToken == null) {
-            refreshTokenInfo = "No token saved in database. Please login first";
-            savedTokenAvailable = false;
-        } else {
-            RefreshToken refreshTokenDecoded = TokenUtil.getRefreshToken(refreshToken);
-            String exp = (refreshTokenDecoded.getExpiration() == 0) ? "NEVER" : Time.toDate(refreshTokenDecoded.getExpiration()).toString();
-            refreshTokenInfo = String.format("<p>Type: %s</p><p>ID: %s</p><p>Expires: %s</p>", refreshTokenDecoded.getType(), refreshTokenDecoded.getId(), exp);
-            savedTokenAvailable = true;
-        }
-        req.setAttribute("tokenInfo", refreshTokenInfo);
-        req.setAttribute("savedTokenAvailable", savedTokenAvailable);
-
-        String customers;
-        if (req.getRequestURI().endsWith("/loadCustomers")) {
-            customers = loadCustomers(req, refreshToken);
-        } else {
-            customers = "";
-        }
-        req.setAttribute("customers", customers);
+            String customers;
+            if (req.getRequestURI().endsWith("/loadCustomers")) {
+                customers = loadCustomers(req, refreshToken);
+            } else {
+                customers = "";
+            }
+            req.setAttribute("customers", customers);
 
-        req.getRequestDispatcher("/WEB-INF/pages/view.jsp").forward(req, resp);
+            req.getRequestDispatcher("/WEB-INF/pages/view.jsp").forward(req, resp);
+        } catch (JWSInputException e) {
+            throw new ServletException(e);
+        }
     }
 
-    private void storeToken(HttpServletRequest req) throws IOException {
+    private void storeToken(HttpServletRequest req) throws IOException, JWSInputException {
         RefreshableKeycloakSecurityContext ctx = (RefreshableKeycloakSecurityContext) req.getAttribute(KeycloakSecurityContext.class.getName());
         String refreshToken = ctx.getRefreshToken();
 

diff --git a/integration/adapter-core/src/main/java/org/keycloak/adapters/CookieTokenStore.java b/integration/adapter-core/src/main/java/org/keycloak/adapters/CookieTokenStore.java
@@ -9,6 +9,7 @@
 import org.keycloak.common.VerificationException;
 import org.keycloak.constants.AdapterConstants;
 import org.keycloak.jose.jws.JWSInput;
+import org.keycloak.jose.jws.JWSInputException;
 import org.keycloak.representations.AccessToken;
 import org.keycloak.representations.IDToken;
 import org.keycloak.common.util.KeycloakUriBuilder;
@@ -58,10 +59,10 @@ public static KeycloakPrincipal<RefreshableKeycloakSecurityContext> getPrincipal
             AccessToken accessToken = RSATokenVerifier.verifyToken(accessTokenString, deployment.getRealmKey(), deployment.getRealmInfoUrl(), false, true);
             IDToken idToken;
             if (idTokenString != null && idTokenString.length() > 0) {
-                JWSInput input = new JWSInput(idTokenString);
                 try {
+                    JWSInput input = new JWSInput(idTokenString);
                     idToken = input.readJsonContent(IDToken.class);
-                } catch (IOException e) {
+                } catch (JWSInputException e) {
                     throw new VerificationException(e);
                 }
             } else {

diff --git a/integration/adapter-core/src/main/java/org/keycloak/adapters/OAuthRequestAuthenticator.java b/integration/adapter-core/src/main/java/org/keycloak/adapters/OAuthRequestAuthenticator.java
@@ -11,6 +11,7 @@
 import org.keycloak.constants.AdapterConstants;
 import org.keycloak.enums.TokenStore;
 import org.keycloak.jose.jws.JWSInput;
+import org.keycloak.jose.jws.JWSInputException;
 import org.keycloak.representations.AccessToken;
 import org.keycloak.representations.AccessTokenResponse;
 import org.keycloak.representations.IDToken;
@@ -313,10 +314,10 @@ protected AuthChallenge resolveCode(String code) {
         try {
             token = RSATokenVerifier.verifyToken(tokenString, deployment.getRealmKey(), deployment.getRealmInfoUrl());
             if (idTokenString != null) {
-                JWSInput input = new JWSInput(idTokenString);
                 try {
+                    JWSInput input = new JWSInput(idTokenString);
                     idToken = input.readJsonContent(IDToken.class);
-                } catch (IOException e) {
+                } catch (JWSInputException e) {
                     throw new VerificationException();
                 }
             }

diff --git a/integration/adapter-core/src/main/java/org/keycloak/adapters/PreAuthActionsHandler.java b/integration/adapter-core/src/main/java/org/keycloak/adapters/PreAuthActionsHandler.java
@@ -3,6 +3,7 @@
 import org.jboss.logging.Logger;
 import org.keycloak.adapters.spi.HttpFacade;
 import org.keycloak.adapters.spi.UserSessionManagement;
+import org.keycloak.jose.jws.JWSInputException;
 import org.keycloak.representations.VersionRepresentation;
 import org.keycloak.constants.AdapterConstants;
 import org.keycloak.jose.jws.JWSInput;
@@ -178,18 +179,17 @@ protected JWSInput verifyAdminRequest() throws Exception {
             return null;
         }
 
-        JWSInput input = new JWSInput(token);
-        boolean verified = false;
         try {
-            verified = RSAProvider.verify(input, deployment.getRealmKey());
-        } catch (Exception ignore) {
-        }
-        if (!verified) {
-            log.warn("admin request failed, unable to verify token");
-            facade.getResponse().sendError(403, "no token");
-            return null;
+            JWSInput input = new JWSInput(token);
+            if (RSAProvider.verify(input, deployment.getRealmKey())) {
+                return input;
+            }
+        } catch (JWSInputException ignore) {
         }
-        return input;
+
+        log.warn("admin request failed, unable to verify token");
+        facade.getResponse().sendError(403, "no token");
+        return null;
     }