Skip to content

Commit

Permalink
Merge pull request #21 from adorsys/20-account-for-clock-skew
Browse files Browse the repository at this point in the history
Run time comparisons taking account of possible clock skew
  • Loading branch information
Ogenbertrand authored Oct 31, 2024
2 parents 2acdc46 + 03b9b79 commit ea1ec91
Show file tree
Hide file tree
Showing 12 changed files with 361 additions and 218 deletions.
21 changes: 21 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
name: CI

on: [pull_request]

jobs:
build:
name: Build (and Test)
runs-on: ubuntu-latest

steps:
- name: Checkout Repository
uses: actions/checkout@v4

- name: Set up JDK 17
uses: actions/setup-java@v4
with:
distribution: 'temurin'
java-version: '17'

- name: Build with Maven
run: ./mvnw clean package
54 changes: 10 additions & 44 deletions src/main/java/de/adorsys/sdjwt/IssuerSignedJwtVerificationOpts.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,75 +7,41 @@
*
* @author <a href="mailto:[email protected]">Ingrid Kamga</a>
*/
public class IssuerSignedJwtVerificationOpts {
public class IssuerSignedJwtVerificationOpts extends TimeClaimVerificationOpts {

private final JWSVerifier verifier;
private final boolean validateIssuedAtClaim;
private final boolean validateExpirationClaim;
private final boolean validateNotBeforeClaim;

public IssuerSignedJwtVerificationOpts(
JWSVerifier verifier,
boolean validateIssuedAtClaim,
boolean validateExpirationClaim,
boolean validateNotBeforeClaim) {
boolean validateNotBeforeClaim,
int leewaySeconds) {
super(validateExpirationClaim, validateNotBeforeClaim, leewaySeconds);
this.verifier = verifier;
this.validateIssuedAtClaim = validateIssuedAtClaim;
this.validateExpirationClaim = validateExpirationClaim;
this.validateNotBeforeClaim = validateNotBeforeClaim;
}

public JWSVerifier getVerifier() {
return verifier;
}

public boolean mustValidateIssuedAtClaim() {
return validateIssuedAtClaim;
}

public boolean mustValidateExpirationClaim() {
return validateExpirationClaim;
}

public boolean mustValidateNotBeforeClaim() {
return validateNotBeforeClaim;
}

public static IssuerSignedJwtVerificationOpts.Builder builder() {
return new IssuerSignedJwtVerificationOpts.Builder();
public static Builder builder() {
return new Builder();
}

public static class Builder {
public static class Builder extends TimeClaimVerificationOpts.Builder<Builder> {
private JWSVerifier verifier;
private boolean validateIssuedAtClaim;
private boolean validateExpirationClaim = true;
private boolean validateNotBeforeClaim = true;

public Builder withVerifier(JWSVerifier verifier) {
this.verifier = verifier;
return this;
}

public Builder withValidateIssuedAtClaim(boolean validateIssuedAtClaim) {
this.validateIssuedAtClaim = validateIssuedAtClaim;
return this;
}

public Builder withValidateExpirationClaim(boolean validateExpirationClaim) {
this.validateExpirationClaim = validateExpirationClaim;
return this;
}

public Builder withValidateNotBeforeClaim(boolean validateNotBeforeClaim) {
this.validateNotBeforeClaim = validateNotBeforeClaim;
return this;
}

public IssuerSignedJwtVerificationOpts build() {
return new IssuerSignedJwtVerificationOpts(
verifier,
validateIssuedAtClaim,
validateExpirationClaim,
validateNotBeforeClaim
validateNotBeforeClaim,
leewaySeconds
);
}
}
Expand Down
25 changes: 0 additions & 25 deletions src/main/java/de/adorsys/sdjwt/SdJws.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import java.io.IOException;
import java.text.ParseException;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
Expand Down Expand Up @@ -97,30 +96,6 @@ public void verifySignature(JWSVerifier verifier) throws JOSEException {
}
}

public void verifyIssuedAtClaim() throws SdJwtVerificationException {
// The purpose of this method was to check if `iat` is not in the future.
// However, this cannot be achieved at high resolution between times provided
// by different systems. So we removed our unreliable implementation.
}

public void verifyExpClaim() throws SdJwtVerificationException {
long now = Instant.now().getEpochSecond();
long exp = SdJwtUtils.readTimeClaim(payload, "exp");

if (now >= exp) {
throw new SdJwtVerificationException("jwt has expired");
}
}

public void verifyNotBeforeClaim() throws SdJwtVerificationException {
long now = Instant.now().getEpochSecond();
long nbf = SdJwtUtils.readTimeClaim(payload, "nbf");

if (now < nbf) {
throw new SdJwtVerificationException("jwt not valid yet");
}
}

/**
* Verifies that SD-JWT was issued by one of the provided issuers. Verification is case-insensitive
* @param issuers List of trusted issuers
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/de/adorsys/sdjwt/SdJwtUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.util.Base64;
import java.util.Objects;
import java.util.Optional;

/**
Expand Down Expand Up @@ -96,6 +97,8 @@ public static ArrayNode decodeDisclosureString(String disclosure) throws SdJwtVe
}

public static long readTimeClaim(JsonNode payload, String claimName) throws SdJwtVerificationException {
Objects.requireNonNull(payload);

JsonNode claim = payload.get(claimName);
if (claim == null || !claim.isNumber()) {
throw new SdJwtVerificationException("Missing or invalid '" + claimName + "' claim");
Expand Down
48 changes: 19 additions & 29 deletions src/main/java/de/adorsys/sdjwt/SdJwtVerificationContext.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
package de.adorsys.sdjwt;

import de.adorsys.sdjwt.exception.SdJwtVerificationException;
import de.adorsys.sdjwt.vp.KeyBindingJWT;
import de.adorsys.sdjwt.vp.KeyBindingJwtVerificationOpts;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
Expand All @@ -13,9 +10,11 @@
import com.nimbusds.jose.crypto.RSASSAVerifier;
import com.nimbusds.jose.jwk.JWK;
import com.nimbusds.jose.jwk.KeyType;
import de.adorsys.sdjwt.exception.SdJwtVerificationException;
import de.adorsys.sdjwt.vp.KeyBindingJWT;
import de.adorsys.sdjwt.vp.KeyBindingJwtVerificationOpts;

import java.text.ParseException;
import java.time.Instant;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -258,36 +257,31 @@ private JWSVerifier buildHolderVerifier(JsonNode cnf) throws SdJwtVerificationEx
* If a required validity-controlling claim is missing, the SD-JWT MUST be rejected.
* </p>
*
* <p>
* Issuers will typically include claims controlling the validity of the SD-JWT in plaintext in the
* SD-JWT payload, but there is no guarantee they would do so. Therefore, Verifiers cannot reliably
* depend on that and need to operate as though security-critical claims might be selectively disclosable.
* </p>
*
* @throws SdJwtVerificationException if verification failed
*/
private void validateIssuerSignedJwtTimeClaims(
JsonNode payload,
IssuerSignedJwtVerificationOpts issuerSignedJwtVerificationOpts
) throws SdJwtVerificationException {
long now = Instant.now().getEpochSecond();
var timeClaimVerifier = new TimeClaimVerifier(issuerSignedJwtVerificationOpts.getLeewaySeconds());

try {
if (issuerSignedJwtVerificationOpts.mustValidateIssuedAtClaim()
&& now < SdJwtUtils.readTimeClaim(payload, "iat")) {
throw new SdJwtVerificationException("JWT issued in the future");
}
} catch (SdJwtVerificationException e) {
throw new SdJwtVerificationException("Issuer-Signed JWT: Invalid `iat` claim", e);
}

try {
if (issuerSignedJwtVerificationOpts.mustValidateExpirationClaim()
&& now >= SdJwtUtils.readTimeClaim(payload, "exp")) {
throw new SdJwtVerificationException("JWT has expired");
if (issuerSignedJwtVerificationOpts.mustValidateExpirationClaim()) {
timeClaimVerifier.verifyExpClaim(payload);
}
} catch (SdJwtVerificationException e) {
throw new SdJwtVerificationException("Issuer-Signed JWT: Invalid `exp` claim", e);
}

try {
if (issuerSignedJwtVerificationOpts.mustValidateNotBeforeClaim()
&& now < SdJwtUtils.readTimeClaim(payload, "nbf")) {
throw new SdJwtVerificationException("JWT is not yet valid");
if (issuerSignedJwtVerificationOpts.mustValidateNotBeforeClaim()) {
timeClaimVerifier.verifyNotBeforeClaim(payload);
}
} catch (SdJwtVerificationException e) {
throw new SdJwtVerificationException("Issuer-Signed JWT: Invalid `nbf` claim", e);
Expand All @@ -302,16 +296,12 @@ private void validateIssuerSignedJwtTimeClaims(
private void validateKeyBindingJwtTimeClaims(
KeyBindingJwtVerificationOpts keyBindingJwtVerificationOpts
) throws SdJwtVerificationException {
var timeClaimVerifier = new TimeClaimVerifier(keyBindingJwtVerificationOpts.getLeewaySeconds());

// Check that the creation time of the Key Binding JWT, as determined by the iat claim,
// is within an acceptable window

try {
keyBindingJwt.verifyIssuedAtClaim();
} catch (SdJwtVerificationException e) {
throw new SdJwtVerificationException("Key binding JWT: Invalid `iat` claim", e);
}

long now = Instant.now().getEpochSecond();
long now = timeClaimVerifier.currentTimestamp();
long keyBindingJwtIat = SdJwtUtils.readTimeClaim(keyBindingJwt.getPayload(), "iat");

if (now - keyBindingJwtIat > keyBindingJwtVerificationOpts.getAllowedMaxAge()) {
Expand All @@ -322,15 +312,15 @@ private void validateKeyBindingJwtTimeClaims(

try {
if (keyBindingJwtVerificationOpts.mustValidateExpirationClaim()) {
keyBindingJwt.verifyExpClaim();
timeClaimVerifier.verifyExpClaim(keyBindingJwt.getPayload());
}
} catch (SdJwtVerificationException e) {
throw new SdJwtVerificationException("Key binding JWT: Invalid `exp` claim", e);
}

try {
if (keyBindingJwtVerificationOpts.mustValidateNotBeforeClaim()) {
keyBindingJwt.verifyNotBeforeClaim();
timeClaimVerifier.verifyNotBeforeClaim(keyBindingJwt.getPayload());
}
} catch (SdJwtVerificationException e) {
throw new SdJwtVerificationException("Key binding JWT: Invalid `nbf` claim", e);
Expand Down
76 changes: 76 additions & 0 deletions src/main/java/de/adorsys/sdjwt/TimeClaimVerificationOpts.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package de.adorsys.sdjwt;

/**
* Options for validating common time claims during SD-JWT verification.
*
* @author <a href="mailto:[email protected]">Ingrid Kamga</a>
*/
public class TimeClaimVerificationOpts {

/**
* Tolerance window to account for clock skew when checking time claims
*/
public static final int DEFAULT_LEEWAY_SECONDS = 10;

private final boolean validateExpirationClaim;
private final boolean validateNotBeforeClaim;
private final int leewaySeconds;

public TimeClaimVerificationOpts(
boolean validateExpirationClaim,
boolean validateNotBeforeClaim,
int leewaySeconds) {
this.validateExpirationClaim = validateExpirationClaim;
this.validateNotBeforeClaim = validateNotBeforeClaim;
this.leewaySeconds = leewaySeconds;
}

public boolean mustValidateExpirationClaim() {
return validateExpirationClaim;
}

public boolean mustValidateNotBeforeClaim() {
return validateNotBeforeClaim;
}

public int getLeewaySeconds() {
return leewaySeconds;
}

public static <T extends Builder<T>> Builder<T> builder() {
return new Builder<>();
}

public static class Builder<T extends Builder<T>> {

protected boolean validateExpirationClaim = true;
protected boolean validateNotBeforeClaim = true;
protected int leewaySeconds = DEFAULT_LEEWAY_SECONDS;

@SuppressWarnings("unchecked")
public T withValidateExpirationClaim(boolean validateExpirationClaim) {
this.validateExpirationClaim = validateExpirationClaim;
return (T) this;
}

@SuppressWarnings("unchecked")
public T withValidateNotBeforeClaim(boolean validateNotBeforeClaim) {
this.validateNotBeforeClaim = validateNotBeforeClaim;
return (T) this;
}

@SuppressWarnings("unchecked")
public T withLeewaySeconds(int leewaySeconds) {
this.leewaySeconds = leewaySeconds;
return (T) this;
}

public TimeClaimVerificationOpts build() {
return new TimeClaimVerificationOpts(
validateExpirationClaim,
validateNotBeforeClaim,
leewaySeconds
);
}
}
}
Loading

0 comments on commit ea1ec91

Please sign in to comment.