CVE-2021-44878: Improper Verification of Cryptographic Signature (CWE-347)
value: If an OpenID Connect provider supports the "none" algorithm (i.e., tokens with no signature), pac4j v5.3.0 (and prior) does not refuse it without an explicit configuration on its side or for the "idtoken" response type which is not secure and violates the OpenID Core Specification. The "none" algorithm does not require any signature verification when validating the ID tokens, which allows the attacker to bypass the token validation by injecting a malformed ID token using "none" as the value of "alg" key in the header with an empty signature value.
CWE description: The product does not verify, or incorrectly verifies, the cryptographic signature for data.
CVSS v3 base score: 7.5
Published: 2022-01-06T13:15Z
Repository: https://github.com/pac4j/pac4j
Language: Java
Files changed: documentation/docs/clients/openid-connect.md, documentation/docs/release-notes.md, pac4j-oidc/src/main/java/org/pac4j/oidc/config/OidcConfiguration.java, pac4j-oidc/src/main/java/org/pac4j/oidc/credentials/authenticator/UserInfoOidcAuthenticator.java, pac4j-oidc/src/main/java/org/pac4j/oidc/profile/creator/TokenValidator.java, pac4j-oidc/src/test/java/org/pac4j/oidc/profile/creator/TokenValidatorTests.java
Commit date: 2021-12-06 09:09:36 +0100
Commit message: reinforce security on OIDC

Diff: diff --git a/documentation/docs/clients/openid-connect.md b/documentation/docs/clients/openid-connect.md
index 4543ac58..4e2ae1a7 100644
--- a/documentation/docs/clients/openid-connect.md
+++ b/documentation/docs/clients/openid-connect.md
@@ -143,5 +143,11 @@ config.setExpireSessionWithToken(false);
 The additional param `TokenExpirationAdvance` allows to set the time in seconds, previous to the token expiration, in which the expiration is advanced. By default it is `0` seconds.
 
 ```java
 config.setTokenExpirationAdvance(10);
 ```
+
+Since version 5.2 and to reinforce security, the `none` alogithm for ID tokens (meaning no signature validation) must be explictly accepted by using:
+
+```java
+config.setAllowUnsignedIdTokens(true);
+```
diff --git a/documentation/docs/release-notes.md b/documentation/docs/release-notes.md
index 30381648..388fda59 100644
--- a/documentation/docs/release-notes.md
+++ b/documentation/docs/release-notes.md
@@ -4,10 +4,11 @@ title: Release notes&#58;
 ---
 
 **v5.2**:
 
 - The JEE core components are now in the `pac4j-jee` dependency (and no longer in the `pac4j-core` dependency)
+- Reinforce security on the OIDC protocol support: the `none` algorithm must be explicitly accepted on client side (`allowUnsignedIdTokens`)
 
 **v5.1.5**:
 
 - Fixed a NPE on the `SAML2AuthnResponseValidator`
 - Added the `setExpiration(Date)` method on the `OidcProfile`
diff --git a/pac4j-oidc/src/main/java/org/pac4j/oidc/config/OidcConfiguration.java b/pac4j-oidc/src/main/java/org/pac4j/oidc/config/OidcConfiguration.java
index 1a4318b5..ef2c80f3 100644
--- a/pac4j-oidc/src/main/java/org/pac4j/oidc/config/OidcConfiguration.java
+++ b/pac4j-oidc/src/main/java/org/pac4j/oidc/config/OidcConfiguration.java
@@ -138,10 +138,12 @@ public class OidcConfiguration extends BaseClientConfiguration {
 
     private LogoutHandler logoutHandler;
 
     private TokenValidator tokenValidator;
 
+    private boolean allowUnsignedIdTokens;
+
     @Override
     protected void internalInit() {
         // checks
         assertNotBlank("clientId", getClientId());
         if (!AUTHORIZATION_CODE_FLOWS.contains(responseType) && !IMPLICIT_FLOWS.contains(responseType)
@@ -484,17 +486,25 @@ public class OidcConfiguration extends BaseClientConfiguration {
 
     public void setMappedClaims(Map<String, String> mappedClaims) {
         this.mappedClaims = mappedClaims;
     }
 
+    public boolean isAllowUnsignedIdTokens() {
+        return allowUnsignedIdTokens;
+    }
+
+    public void setAllowUnsignedIdTokens(final boolean allowUnsignedIdTokens) {
+        this.allowUnsignedIdTokens = allowUnsignedIdTokens;
+    }
+
     @Override
     public String toString() {
         return toNiceString(this.getClass(), "clientId", clientId, "secret", "[protected]",
             "discoveryURI", discoveryURI, "scope", scope, "customParams", customParams,
             "clientAuthenticationMethod", clientAuthenticationMethod, "useNonce", useNonce,
             "preferredJwsAlgorithm", preferredJwsAlgorithm, "maxAge", maxAge, "maxClockSkew", maxClockSkew,
             "connectTimeout", connectTimeout, "readTimeout", readTimeout, "resourceRetriever", resourceRetriever,
             "responseType", responseType, "responseMode", responseMode, "logoutUrl", logoutUrl,
             "withState", withState, "stateGenerator", stateGenerator, "logoutHandler", logoutHandler,
-            "tokenValidator", tokenValidator, "mappedClaims", mappedClaims);
+            "tokenValidator", tokenValidator, "mappedClaims", mappedClaims, "allowUnsignedIdTokens", allowUnsignedIdTokens);
     }
 }
diff --git a/pac4j-oidc/src/main/java/org/pac4j/oidc/credentials/authenticator/UserInfoOidcAuthenticator.java b/pac4j-oidc/src/main/java/org/pac4j/oidc/credentials/authenticator/UserInfoOidcAuthenticator.java
index 5a5fee08..686b54a3 100644
--- a/pac4j-oidc/src/main/java/org/pac4j/oidc/credentials/authenticator/UserInfoOidcAuthenticator.java
+++ b/pac4j-oidc/src/main/java/org/pac4j/oidc/credentials/authenticator/UserInfoOidcAuthenticator.java
@@ -4,12 +4,10 @@ import static java.util.Optional.ofNullable;
 import static org.pac4j.core.profile.AttributeLocation.PROFILE_ATTRIBUTE;
 
 import java.io.IOException;
 import java.util.Map;
 
-import javax.naming.AuthenticationException;
-
 import org.pac4j.core.context.WebContext;
 import org.pac4j.core.context.session.SessionStore;
 import org.pac4j.core.credentials.Credentials;
 import org.pac4j.core.credentials.TokenCredentials;
 import org.pac4j.core.credentials.authenticator.Authenticator;
@@ -97,24 +95,23 @@ public class UserInfoOidcAuthenticator extends InitializableObject implements Au
             final var httpResponse = userInfoHttpRequest.send();
             logger.debug("Token response: status={}, content={}", httpResponse.getStatusCode(),
                 httpResponse.getContent());
             final var userInfoResponse = UserInfoResponse.parse(httpResponse);
             if (userInfoResponse instanceof UserInfoErrorResponse) {
-                logger.error("Bad User Info response, error={}",
-                    ((UserInfoErrorResponse) userInfoResponse).getErrorObject().toJSONObject());
-                throw new AuthenticationException();
+                throw new TechnicalException("Bad User Info response, error="
+                    + ((UserInfoErrorResponse) userInfoResponse).getErrorObject().toJSONObject());
             } else {
                 final var userInfoSuccessResponse = (UserInfoSuccessResponse) userInfoResponse;
                 final JWTClaimsSet userInfoClaimsSet;
                 if (userInfoSuccessResponse.getUserInfo() != null) {
                     userInfoClaimsSet = userInfoSuccessResponse.getUserInfo().toJWTClaimsSet();
                 } else {
                     userInfoClaimsSet = userInfoSuccessResponse.getUserInfoJWT().getJWTClaimsSet();
                 }
                 return userInfoClaimsSet;
             }
-        } catch (IOException | ParseException | java.text.ParseException | AuthenticationException e) {
+        } catch (IOException | ParseException | java.text.ParseException e) {
             throw new TechnicalException(e);
         }
     }
 
     public OidcConfiguration getConfiguration() {
diff --git a/pac4j-oidc/src/main/java/org/pac4j/oidc/profile/creator/TokenValidator.java b/pac4j-oidc/src/main/java/org/pac4j/oidc/profile/creator/TokenValidator.java
index c72fd220..15c1771f 100644
--- a/pac4j-oidc/src/main/java/org/pac4j/oidc/profile/creator/TokenValidator.java
+++ b/pac4j-oidc/src/main/java/org/pac4j/oidc/profile/creator/TokenValidator.java
@@ -49,17 +49,17 @@ public class TokenValidator {
 
         idTokenValidators = new ArrayList<>();
         final var _clientID = new ClientID(configuration.getClientId());
 
         for (var jwsAlgorithm : jwsAlgorithms) {
-            if ("none".equals(jwsAlgorithm.getName())) {
-                jwsAlgorithm = null;
-            }
-
             // build validator
             final IDTokenValidator idTokenValidator;
-            if (jwsAlgorithm == null) {
+            if ("none".equals(jwsAlgorithm.getName())) {
+                if (!configuration.isAllowUnsignedIdTokens()) {
+                    throw new TechnicalException("Unsigned ID tokens are not allowed");
+                }
+                logger.warn("Allowing unsigned ID tokens");
                 idTokenValidator = new IDTokenValidator(configuration.findProviderMetadata().getIssuer(), _clientID);
             } else if (CommonHelper.isNotBlank(configuration.getSecret()) && (JWSAlgorithm.HS256.equals(jwsAlgorithm) ||
                 JWSAlgorithm.HS384.equals(jwsAlgorithm) || JWSAlgorithm.HS512.equals(jwsAlgorithm))) {
                 final var _secret = new Secret(configuration.getSecret());
                 idTokenValidator = createHMACTokenValidator(configuration, jwsAlgorithm, _clientID, _secret);
@@ -110,6 +110,11 @@ public class TokenValidator {
             throw joseException;
         } else {
             throw new TechnicalException("Unable to validate the ID token");
         }
     }
+
+    // for tests
+    List<IDTokenValidator> getIdTokenValidators() {
+        return idTokenValidators;
+    }
 }
diff --git a/pac4j-oidc/src/test/java/org/pac4j/oidc/profile/creator/TokenValidatorTests.java b/pac4j-oidc/src/test/java/org/pac4j/oidc/profile/creator/TokenValidatorTests.java
new file mode 100644
index 00000000..8de571a3
--- /dev/null
+++ b/pac4j-oidc/src/test/java/org/pac4j/oidc/profile/creator/TokenValidatorTests.java
@@ -0,0 +1,88 @@
+package org.pac4j.oidc.profile.creator;
+
+import com.nimbusds.jose.JWSAlgorithm;
+import com.nimbusds.oauth2.sdk.id.Issuer;
+import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata;
+import com.nimbusds.openid.connect.sdk.validators.IDTokenValidator;
+import org.junit.Before;
+import org.junit.Test;
+import org.pac4j.core.exception.TechnicalException;
+import org.pac4j.core.util.TestsConstants;
+import org.pac4j.core.util.TestsHelper;
+import org.pac4j.oidc.config.OidcConfiguration;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.mockito.Mockito.*;
+import static org.junit.Assert.*;
+
+/**
+ * Tests {@link TokenValidator}.
+ *
+ * @author Jerome LELEU
+ * @since 5.2.0
+ */
+public final class TokenValidatorTests implements TestsConstants {
+
+    private OidcConfiguration configuration;
+
+    private List<JWSAlgorithm> algorithms;
+
+    @Before
+    public void setUp() throws URISyntaxException {
+        configuration = mock(OidcConfiguration.class);
+        final OIDCProviderMetadata metadata = mock(OIDCProviderMetadata.class);
+        when(metadata.getIssuer()).thenReturn(new Issuer(PAC4J_URL));
+        when(metadata.getJWKSetURI()).thenReturn(new URI(PAC4J_BASE_URL));
+        when(configuration.findProviderMetadata()).thenReturn(metadata);
+        when(configuration.getClientId()).thenReturn(ID);
+        when(configuration.getSecret()).thenReturn(SECRET);
+        algorithms = new ArrayList<>();
+        when(metadata.getIDTokenJWSAlgs()).thenReturn(algorithms);
+    }
+
+    @Test
+    public void testNoAlgoDefinedAtProvider() {
+        TestsHelper.expectException(() -> new TokenValidator(configuration), TechnicalException.class,
+            "There must at least one JWS algorithm supported on the OpenID Connect provider side");
+    }
+
+    @Test
+    public void testNoneAlgoNotAllowed() {
+        algorithms.add(JWSAlgorithm.parse("none"));
+        TestsHelper.expectException(() -> new TokenValidator(configuration), TechnicalException.class,
+            "Unsigned ID tokens are not allowed");
+    }
+
+    @Test
+    public void testNoneAlgoAllowed() {
+        algorithms.add(JWSAlgorithm.parse("none"));
+        when(configuration.isAllowUnsignedIdTokens()).thenReturn(true);
+        final TokenValidator validator = new TokenValidator(configuration);
+        final List<IDTokenValidator> validators = validator.getIdTokenValidators();
+        assertEquals(1, validators.size());
+        assertTrue(validators.get(0) instanceof IDTokenValidator);
+    }
+
+    @Test
+    public void testTwoAlgorithms() {
+        algorithms.add(JWSAlgorithm.HS256);
+        algorithms.add(JWSAlgorithm.RS256);
+        final TokenValidator validator = new TokenValidator(configuration);
+        final List<IDTokenValidator> validators = validator.getIdTokenValidators();
+        assertEquals(2, validators.size());
+    }
+
+    @Test
+    public void testTwoAlgorithmsOnePreferred() {
+        algorithms.add(JWSAlgorithm.HS256);
+        algorithms.add(JWSAlgorithm.RS256);
+        when(configuration.getPreferredJwsAlgorithm()).thenReturn(JWSAlgorithm.HS256);
+        final TokenValidator validator = new TokenValidator(configuration);
+        final List<IDTokenValidator> validators = validator.getIdTokenValidators();
+        assertEquals(1, validators.size());
+    }
+}

Vulnerable code:             "tokenValidator", tokenValidator, "mappedClaims", mappedClaims);
import javax.naming.AuthenticationException;

                logger.error("Bad User Info response, error={}",
                    ((UserInfoErrorResponse) userInfoResponse).getErrorObject().toJSONObject());
                throw new AuthenticationException();
        } catch (IOException | ParseException | java.text.ParseException | AuthenticationException e) {
            if ("none".equals(jwsAlgorithm.getName())) {
                jwsAlgorithm = null;
            }

            if (jwsAlgorithm == null) {
Fixed code: 
Since version 5.2 and to reinforce security, the `none` alogithm for ID tokens (meaning no signature validation) must be explictly accepted by using:

```java
config.setAllowUnsignedIdTokens(true);
```
- Reinforce security on the OIDC protocol support: the `none` algorithm must be explicitly accepted on client side (`allowUnsignedIdTokens`)
    private boolean allowUnsignedIdTokens;

    public boolean isAllowUnsignedIdTokens() {
        return allowUnsignedIdTokens;
    }

    public void setAllowUnsignedIdTokens(final boolean allowUnsignedIdTokens) {
        this.allowUnsignedIdTokens = allowUnsignedIdTokens;
    }

            "tokenValidator", tokenValidator, "mappedClaims", mappedClaims, "allowUnsignedIdTokens", allowUnsignedIdTokens);
                throw new TechnicalException("Bad User Info response, error="
                    + ((UserInfoErrorResponse) userInfoResponse).getErrorObject().toJSONObject());
        } catch (IOException | ParseException | java.text.ParseException e) {
            if ("none".equals(jwsAlgorithm.getName())) {
                if (!configuration.isAllowUnsignedIdTokens()) {
                    throw new TechnicalException("Unsigned ID tokens are not allowed");
                }
                logger.warn("Allowing unsigned ID tokens");

    // for tests
    List<IDTokenValidator> getIdTokenValidators() {
        return idTokenValidators;
    }
package org.pac4j.oidc.profile.creator;

import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.oauth2.sdk.id.Issuer;
import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata;
import com.nimbusds.openid.connect.sdk.validators.IDTokenValidator;
import org.junit.Before;
import org.junit.Test;
import org.pac4j.core.exception.TechnicalException;
import org.pac4j.core.util.TestsConstants;
import org.pac4j.core.util.TestsHelper;
import org.pac4j.oidc.config.OidcConfiguration;

import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.List;

import static org.mockito.Mockito.*;
import static org.junit.Assert.*;

/**
 * Tests {@link TokenValidator}.
 *
 * @author Jerome LELEU
 * @since 5.2.0
 */
public final class TokenValidatorTests implements TestsConstants {

    private OidcConfiguration configuration;

    private List<JWSAlgorithm> algorithms;

    @Before
    public void setUp() throws URISyntaxException {
        configuration = mock(OidcConfiguration.class);
        final OIDCProviderMetadata metadata = mock(OIDCProviderMetadata.class);
        when(metadata.getIssuer()).thenReturn(new Issuer(PAC4J_URL));
        when(metadata.getJWKSetURI()).thenReturn(new URI(PAC4J_BASE_URL));
        when(configuration.findProviderMetadata()).thenReturn(metadata);
        when(configuration.getClientId()).thenReturn(ID);
        when(configuration.getSecret()).thenReturn(SECRET);
        algorithms = new ArrayList<>();
        when(metadata.getIDTokenJWSAlgs()).thenReturn(algorithms);
    }

    @Test
    public void testNoAlgoDefinedAtProvider() {
        TestsHelper.expectException(() -> new TokenValidator(configuration), TechnicalException.class,
            "There must at least one JWS algorithm supported on the OpenID Connect provider side");
    }

    @Test
    public void testNoneAlgoNotAllowed() {
        algorithms.add(JWSAlgorithm.parse("none"));
        TestsHelper.expectException(() -> new TokenValidator(configuration), TechnicalException.class,
            "Unsigned ID tokens are not allowed");
    }

    @Test
    public void testNoneAlgoAllowed() {
        algorithms.add(JWSAlgorithm.parse("none"));
        when(configuration.isAllowUnsignedIdTokens()).thenReturn(true);
        final TokenValidator validator = new TokenValidator(configuration);
        final List<IDTokenValidator> validators = validator.getIdTokenValidators();
        assertEquals(1, validators.size());
        assertTrue(validators.get(0) instanceof IDTokenValidator);
    }

    @Test
    public void testTwoAlgorithms() {
        algorithms.add(JWSAlgorithm.HS256);
        algorithms.add(JWSAlgorithm.RS256);
        final TokenValidator validator = new TokenValidator(configuration);
        final List<IDTokenValidator> validators = validator.getIdTokenValidators();
        assertEquals(2, validators.size());
    }

    @Test
    public void testTwoAlgorithmsOnePreferred() {
        algorithms.add(JWSAlgorithm.HS256);
        algorithms.add(JWSAlgorithm.RS256);
        when(configuration.getPreferredJwsAlgorithm()).thenReturn(JWSAlgorithm.HS256);
        final TokenValidator validator = new TokenValidator(configuration);
        final List<IDTokenValidator> validators = validator.getIdTokenValidators();
        assertEquals(1, validators.size());
    }
}