[java-idp-oidc] branch maint-3.0 updated: JOIDC-65 JWT client authentication support is incomplete

Henri Mikkonen henri.mikkonen at iki.fi
Mon Dec 27 14:48:55 UTC 2021


This is an automated email from the git hooks/post-receive script.

hjmikkon pushed a commit to branch maint-3.0
in repository java-idp-oidc.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-oidc.git;a=commit;h=2b7443f7c6a5463f205f70d544efc6c38d9e6862

The following commit(s) were added to refs/heads/maint-3.0 by this push:
     new 2b7443f7 JOIDC-65 JWT client authentication support is incomplete
2b7443f7 is described below

commit 2b7443f7c6a5463f205f70d544efc6c38d9e6862
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Mon Dec 27 16:47:14 2021 +0200

    JOIDC-65 JWT client authentication support is incomplete
    
    https://shibboleth.atlassian.net/browse/JOIDC-65
    
    Refactoring IssuedAtClaimsValidator with the logic taken from
    org.opensaml.saml.common.binding.security.impl.MessageLifetimeSecurityHandler.
---
 .../jwt/claims/impl/IssuedAtClaimsValidator.java   |  78 +++++++----
 .../oauth2/introspection/introspection-beans.xml   |   2 +-
 .../flows/oauth2/revocation/revocation-beans.xml   |   2 +-
 .../idp/flows/oidc/token/token-beans.xml           |   2 +-
 .../claims/impl/IssuedAtClaimsValidatorTest.java   | 156 +++++++++++++++++++++
 5 files changed, 209 insertions(+), 31 deletions(-)

diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/security/jwt/claims/impl/IssuedAtClaimsValidator.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/security/jwt/claims/impl/IssuedAtClaimsValidator.java
index 4645fe6b..ec45b180 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/security/jwt/claims/impl/IssuedAtClaimsValidator.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/security/jwt/claims/impl/IssuedAtClaimsValidator.java
@@ -34,41 +34,46 @@ import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /**
- * Iff the 'iat' claim is present in the ID Token, verifies it is not to far away from 
- * the current time. A configured window/deviation is allowed. See section 3.1.3.7 of OpenID Connect core 1.0.
+ * If the 'iat' claim is present in the JWT, verifies it is not to far away from the current time.
+ * The message lifetime, clock skew and the claim existence requirement can be configured.
  * 
- * Also added clockSkew. This to be moved into commons: temporarily here due to 3.0.3 patch.
+ * TODO: class (net.shibboleth.oidc.security.jwt.claims.impl.IssuedAtClaimsValidator) is temporarily located here due
+ * to OP plugin 3.0.3 patch. Refactored to correspond the logic in
+ * org.opensaml.saml.common.binding.security.impl.MessageLifetimeSecurityHandler.
  */
 @ThreadSafeAfterInit
 public class IssuedAtClaimsValidator extends AbstractClaimsValidator {
     
     /**
-     *  Maximum amount (in either direction from now) of duration for which a token is valid after 
-     *  it is issued (Default value: 60 seconds). 
+     * Clock skew adjustment in both directions to consider still acceptable (Default value: 1 minute).
      */
-    @Nonnull private Duration iatWindow;
-
-    /** The clock skew allowed (Default value: 60 seconds). */
     @Nonnull private Duration clockSkew;
-    
+
+    /** Amount of time for which a message is valid after it is issued (Default value: 1 minute). */
+    @Nonnull private Duration messageLifetime;
+
+    /** Whether this rule is required to be met. */
+    private boolean requiredRule;
+
     /** Constructor.*/
     public IssuedAtClaimsValidator() {
-        iatWindow = Duration.ofSeconds(60);
-        clockSkew = Duration.ofSeconds(60);
+        messageLifetime = Duration.ofMinutes(1);
+        clockSkew = Duration.ofMinutes(1);
+        requiredRule = true;
     }
     
     /**
-     * Sets the amount of time for which a token is valid from when it was issued.
+     * Sets the amount of time for which a message is valid.
      * 
-     * @param window amount of time for which a token is valid
+     * @param lifetime amount of time for which a message is valid
      */
-    public void setIatWindow(@Nonnull final Duration window) {
+    public void setMessageLifetime(@Nonnull final Duration lifetime) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         
-        Constraint.isNotNull(window, "Token issued at window cannot be null");
-        Constraint.isFalse(window.isNegative(), "Token issued at window cannot be negative");
+        Constraint.isNotNull(lifetime, "Token lifetime cannot be null");
+        Constraint.isFalse(lifetime.isNegative(), "Token lifetime cannot be negative");
 
-        iatWindow = window;
+        messageLifetime = lifetime;
     }
     
     /**
@@ -82,6 +87,16 @@ public class IssuedAtClaimsValidator extends AbstractClaimsValidator {
         clockSkew = Constraint.isNotNull(skew, "Clock skew cannot be null");
     }
 
+    /**
+     * Sets whether this rule is required to be met.
+     * 
+     * @param required whether this rule is required to be met
+     */
+    public void setRequiredRule(final boolean required) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        
+        requiredRule = required;
+    }
 
     /** {@inheritDoc} */
     @Override
@@ -91,19 +106,26 @@ public class IssuedAtClaimsValidator extends AbstractClaimsValidator {
         final Date iatDate = claims.getIssueTime();
         if (iatDate != null) {           
             final Instant iat = iatDate.toInstant();        
-            final Instant now = Instant.now();        
-            final Duration iatDifference = Duration.between(now, iat).abs();
-            
-            final Duration window = iatWindow.plus(clockSkew);
-            
-            if (window.compareTo(iatDifference)  < 0) {
-                throw new JWTValidationException("JWT issued-at time is too far away from the current time. "
+            final Instant now = Instant.now();
+            final Instant latestValid = now.plus(clockSkew.abs());
+            final Instant expiration = iat.plus(clockSkew.abs()).plus(messageLifetime);
+
+            // Check message wasn't issued in the future
+            if (iat.isAfter(latestValid)) {
+                throw new JWTValidationException("JWT was rejected because it was issued in the future. "
                         + "Token issued at '"+iat+"' was too far away from the current time '"+now+"' "
-                                + "with acceptable deviation of "
-                                + "'"+window+"', difference is '"+iatDifference+"'");
+                        + "with acceptable lifetime of '" + messageLifetime + "', clockSkew of '" + clockSkew + "'");
             }
+
+            // Check message has not expired
+            if (expiration.isBefore(now)) {
+                throw new JWTValidationException("JWT was rejected due to issue instance expiration. "
+                        + "Token issued at '"+iat+"' was too far away from the current time '"+now+"' "
+                        + "with acceptable lifetime of '" + messageLifetime + "', clockSkew of '" + clockSkew + "'");
+
+            }
+        } else if (requiredRule) {
+            throw new JWTValidationException("JWT was rejected due to missing required 'iat' claim.");
         }
-        
     }
-
 }
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/introspection/introspection-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/introspection/introspection-beans.xml
index b6da4dfd..c919f4d3 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/introspection/introspection-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/introspection/introspection-beans.xml
@@ -42,7 +42,7 @@
 
     <bean id="IssuedAtClaimsValidator"
         class="net.shibboleth.idp.plugin.oidc.op.security.jwt.claims.impl.IssuedAtClaimsValidator"
-        p:iatWindow="%{idp.policy.messageLifetime:PT1M}" p:clockSkew="%{idp.policy.clockSkew:PT1M}" />
+        p:messageLifetime="%{idp.policy.messageLifetime:PT1M}" p:clockSkew="%{idp.policy.clockSkew:PT1M}" />
 
     <bean id="FormOutboundMessage"
         class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.FormOutboundIntrospectionResponseMessage" scope="prototype"
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/revocation/revocation-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/revocation/revocation-beans.xml
index 6a8eb098..064193a9 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/revocation/revocation-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/revocation/revocation-beans.xml
@@ -43,7 +43,7 @@
 
     <bean id="IssuedAtClaimsValidator"
         class="net.shibboleth.idp.plugin.oidc.op.security.jwt.claims.impl.IssuedAtClaimsValidator"
-        p:iatWindow="%{idp.policy.messageLifetime:PT1M}" p:clockSkew="%{idp.policy.clockSkew:PT1M}" />
+        p:messageLifetime="%{idp.policy.messageLifetime:PT1M}" p:clockSkew="%{idp.policy.clockSkew:PT1M}" />
 
     <bean id="RevokeToken" class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.RevokeToken" scope="prototype"
         c:sealer-ref="#{'%{idp.oidc.tokenSealer:shibboleth.oidc.TokenSealer}'.trim()}"
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml
index 6b4b27a0..c6d5e264 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml
@@ -41,7 +41,7 @@
 
     <bean id="IssuedAtClaimsValidator"
         class="net.shibboleth.idp.plugin.oidc.op.security.jwt.claims.impl.IssuedAtClaimsValidator"
-        p:iatWindow="%{idp.policy.messageLifetime:PT1M}" p:clockSkew="%{idp.policy.clockSkew:PT1M}" />
+        p:messageLifetime="%{idp.policy.messageLifetime:PT1M}" p:clockSkew="%{idp.policy.clockSkew:PT1M}" />
 
     <bean id="ValidateGrantType" class="net.shibboleth.idp.plugin.oidc.op.profile.impl.ValidateGrantType"
         scope="prototype" />
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/security/jwt/claims/impl/IssuedAtClaimsValidatorTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/security/jwt/claims/impl/IssuedAtClaimsValidatorTest.java
new file mode 100644
index 00000000..bea5329d
--- /dev/null
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/security/jwt/claims/impl/IssuedAtClaimsValidatorTest.java
@@ -0,0 +1,156 @@
+/* 
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements.  See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID 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 net.shibboleth.idp.plugin.oidc.op.security.jwt.claims.impl;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Date;
+
+import javax.annotation.Nonnull;
+
+import org.opensaml.profile.context.ProfileRequestContext;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.nimbusds.jwt.JWTClaimsSet;
+
+import net.shibboleth.oidc.jwt.claims.JWTValidationException;
+import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+
+/**
+ * Test for the {@link IssuedAtClaimsValidator}. 
+ * 
+ * TODO: temporarily here for the OIDC OP 3.0.3 patch. To be included in commons.
+ */
+public class IssuedAtClaimsValidatorTest {
+    
+    /** The validator to test.*/
+    @Nonnull private IssuedAtClaimsValidator validator;
+
+    @Nonnull private ProfileRequestContext prc;
+    
+    @BeforeMethod
+    public void setup() throws ComponentInitializationException {
+        validator = new IssuedAtClaimsValidator();
+        prc = new ProfileRequestContext();
+    }
+    
+    @Test
+    public void doValidateTest() throws JWTValidationException, ComponentInitializationException {
+        final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().issueTime(Date.from(Instant.now())).build();
+        validator.setId("test-validator");
+        validator.setMessageLifetime(Duration.ofMinutes(1));
+        validator.setClockSkew(Duration.ofMinutes(1));
+        validator.initialize();
+        validator.validate(claimsSet, prc);
+    }
+    
+    @Test
+    public void doValidateTestIssuedInPastButInWindow() 
+            throws JWTValidationException, ComponentInitializationException {
+        final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().issueTime(
+                Date.from(Instant.now().minus(Duration.ofMinutes(10)))).build();
+        validator.setId("test-validator");
+        validator.setMessageLifetime(Duration.ofMinutes(6));
+        validator.setClockSkew(Duration.ofMinutes(6));
+        validator.initialize();
+        validator.validate(claimsSet, prc);
+    }
+
+    @Test
+    public void doValidateTestIssuedInPastButInWindow_longClockSkew() 
+            throws JWTValidationException, ComponentInitializationException {
+        final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().issueTime(
+                Date.from(Instant.now().minus(Duration.ofMinutes(10)))).build();
+        validator.setId("test-validator");
+        validator.setMessageLifetime(Duration.ofMinutes(1));
+        validator.setClockSkew(Duration.ofMinutes(15));
+        validator.initialize();
+        validator.validate(claimsSet, prc);
+    }
+
+    @Test
+    public void doValidateTestIssuedInPastButInWindow_longLifetime() 
+            throws JWTValidationException, ComponentInitializationException {
+        final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().issueTime(
+                Date.from(Instant.now().minus(Duration.ofMinutes(10)))).build();
+        validator.setId("test-validator");
+        validator.setMessageLifetime(Duration.ofMinutes(15));
+        validator.setClockSkew(Duration.ofMinutes(1));
+        validator.initialize();
+        validator.validate(claimsSet, prc);
+    }
+
+    @Test
+    public void doValidateTestIssuedInFutureButInWindow() 
+            throws JWTValidationException, ComponentInitializationException {
+        final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().issueTime(
+                Date.from(Instant.now().plus(Duration.ofMinutes(5)))).build();
+        validator.setId("test-validator");
+        validator.setMessageLifetime(Duration.ofMinutes(1));
+        validator.setClockSkew(Duration.ofMinutes(6));
+        validator.initialize();
+        validator.validate(claimsSet, prc);
+    }
+
+    @Test
+    public void doValidateTestNoOptionalIssuedAtClaim()
+            throws JWTValidationException, ComponentInitializationException {
+        final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().build();
+        validator.setId("test-validator");
+        validator.setMessageLifetime(Duration.ofMinutes(1));
+        validator.setClockSkew(Duration.ofMinutes(1));
+        validator.setRequiredRule(false);
+        validator.initialize();
+        validator.validate(claimsSet, prc);
+    }
+
+    @Test(expectedExceptions = JWTValidationException.class)
+    public void doValidateTestNoRequiredIssuedAtClaim()
+            throws JWTValidationException, ComponentInitializationException {
+        final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().build();
+        validator.setId("test-validator");
+        validator.setMessageLifetime(Duration.ofMinutes(1));
+        validator.setClockSkew(Duration.ofMinutes(1));
+        validator.setRequiredRule(true);
+        validator.initialize();
+        validator.validate(claimsSet, prc);
+    }
+
+    @Test(expectedExceptions = JWTValidationException.class)
+    public void doInValidateTestIssuedTooFarInPast() throws JWTValidationException, ComponentInitializationException {
+        final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().issueTime(
+                Date.from(Instant.now().minus(Duration.ofMinutes(11)))).build();
+        validator.setId("test-validator");
+        validator.setMessageLifetime(Duration.ofMinutes(5));
+        validator.setClockSkew(Duration.ofMinutes(5));
+        validator.initialize();
+        validator.validate(claimsSet, prc);
+    }
+    
+    @Test(expectedExceptions = JWTValidationException.class)
+    public void doInValidateTestIssuedTooFarInFuture() throws JWTValidationException, ComponentInitializationException {
+        final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().issueTime(
+                Date.from(Instant.now().plus(Duration.ofMinutes(6)))).build();
+        validator.setId("test-validator");
+        validator.setMessageLifetime(Duration.ofMinutes(5));
+        validator.setClockSkew(Duration.ofMinutes(5));
+        validator.initialize();
+        validator.validate(claimsSet, prc);
+    }
+}
\ No newline at end of file

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list