[pulsar] 09/27: Disallow parsing of token with none signature in authenticateToken (#9172)

[pe...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

penghui pushed a commit to branch branch-2.7
in repository https://gitbox.apache.org/repos/asf/pulsar.git

commit bdd062b4d822ab4487472af4a1561a51ce42900c
Author: Zixuan Liu <no...@gmail.com>
AuthorDate: Tue Feb 9 16:27:09 2021 +0800

    Disallow parsing of token with none signature in authenticateToken (#9172)
    
    Signed-off-by: Zixuan Liu <no...@gmail.com>
    
    ### Motivation
    
    If Apache Pulsar is configured to authenticate clients using tokens based on JSON Web Tokens (JWT), the signature of the token is not validated if the algorithm of the presented token is set to "none". This allows an attacker to connect to Pulsar instances as any user (incl. admins).
    
    ### Modifications
    
    - using `parseClaimsJws` instead of `parse`
    
    `parseClaimsJws` can guarantees the correct security model for parsing signed JWTs.
    
    
    (cherry picked from commit 71bc84164610ff15ff7c5747f923d09b57787e0a)
---
 .../AuthenticationProviderToken.java               |  4 +---
 .../AuthenticationProviderTokenTest.java           | 28 ++++++++++++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
index d847548..5071298 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java
@@ -186,9 +186,7 @@ public class AuthenticationProviderToken implements AuthenticationProvider {
     @SuppressWarnings("unchecked")
     private Jwt<?, Claims> authenticateToken(final String token) throws AuthenticationException {
         try {
-            Jwt<?, Claims> jwt = Jwts.parser()
-                    .setSigningKey(validationKey)
-                    .parse(token);
+            Jwt<?, Claims> jwt = Jwts.parserBuilder().setSigningKey(validationKey).build().parseClaimsJws(token);
 
             if (audienceClaim != null) {
                 Object object = jwt.getBody().get(audienceClaim);
diff --git a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java
index c2610aa..fe4d6a7 100644
--- a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java
+++ b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java
@@ -173,6 +173,34 @@ public class AuthenticationProviderTokenTest {
         });
         assertEquals(subject, SUBJECT);
 
+        /**
+         * HEADER:ALGORITHM & TOKEN TYPE
+         * {
+         *   "alg": "none"
+         * }
+         * PAYLOAD:DATA
+         * {
+         *   "sub": "test-user"
+         * }
+         */
+        String tokenWithNoneAlg = "eyJhbGciOiJub25lIn0.eyJzdWIiOiJ0ZXN0LXVzZXIifQ.";
+        try {
+            provider.authenticate(new AuthenticationDataSource() {
+                @Override
+                public boolean hasDataFromCommand() {
+                    return true;
+                }
+
+                @Override
+                public String getCommandData() {
+                    return tokenWithNoneAlg;
+                }
+            });
+            fail("Should have failed");
+        } catch (AuthenticationException e) {
+            // expected, Unsigned Claims JWTs are not supported.
+        }
+
         // Expired token. This should be rejected by the authentication provider
         String expiredToken = AuthTokenUtils.createToken(secretKey, SUBJECT,
                 Optional.of(new Date(System.currentTimeMillis() - TimeUnit.HOURS.toMillis(1))));