This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-jdkim.git
commit eb72d80d67bf38a45a45358c067e8bc7dff4c9c8 Author: Benoit TELLIER <[email protected]> AuthorDate: Fri Oct 30 13:46:08 2026 +0100 [FIX] SignatureRecord: reject an x= that is not greater than t= RFC 6376 section 3.5 requires x= to be greater than t= when both are present, but validate() only compared x= with the current time: a signature with x <= t in the near future was accepted and verified. Both record classes now reject it; the template still accepts an empty t=, filled in when signing. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../apache/james/jdkim/tagvalue/SignatureRecordImpl.java | 5 +++++ .../james/jdkim/tagvalue/SignatureRecordTemplate.java | 5 +++++ .../org/apache/james/jdkim/SignatureRecordImplTest.java | 13 +++++++++++++ .../org/apache/james/jdkim/SignatureRecordTemplateTest.java | 10 ++++++++++ 4 files changed, 33 insertions(+) diff --git a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java index 5f42991..d72e4d0 100644 --- a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java +++ b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java @@ -86,6 +86,11 @@ public class SignatureRecordImpl extends TagValue implements SignatureRecord { if (!ALL.equals(getValue("l")) && !bodyLengthPattern.matcher(getValue("l")).matches()) throw new IllegalStateException("Invalid body length (l=): " + getValue("l")); + // RFC 6376 section 3.5: x= MUST be greater than t= if both are present + if (getValue("x") != null && timestampPattern.matcher(getValue("x")).matches() && getValue("t") != null + && Long.parseLong(getValue("x").toString()) <= Long.parseLong(getValue("t").toString())) + throw new IllegalStateException("Signature expiration (x=) must be greater than its timestamp (t=)"); + // when "x=" exists and signature expired then return PERMFAIL // (signature expired) if (getValue("x") != null) { diff --git a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java index e4c2594..96438db 100644 --- a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java +++ b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java @@ -95,6 +95,11 @@ public class SignatureRecordTemplate extends TagValue implements SignatureRecord if (!ALL.equals(getValue("l")) && !bodyLengthPattern.matcher(getValue("l")).matches()) throw new IllegalStateException("Invalid body length (l=): " + getValue("l")); + // RFC 6376 section 3.5: x= MUST be greater than t= if both are present + if (getValue("x") != null && timestampPattern.matcher(getValue("x")).matches() && getValue("t") != null && getValue("t").length() > 0 + && Long.parseLong(getValue("x").toString()) <= Long.parseLong(getValue("t").toString())) + throw new IllegalStateException("Signature expiration (x=) must be greater than its timestamp (t=)"); + // when "x=" exists and signature expired then return PERMFAIL // (signature expired) if (getValue("x") != null) { diff --git a/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java b/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java index 4312957..e9212a9 100644 --- a/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java +++ b/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java @@ -287,4 +287,17 @@ public class SignatureRecordImplTest { assertThat(new SignatureRecordImpl("v=1; b = AA\r\n BB ; a=rsa-sha256; d=example.com; s=s; h=from:b=:x; bh=AA==").toUnsignedString()) .isEqualTo("v=1; b =; a=rsa-sha256; d=example.com; s=s; h=from:b=:x; bh=AA=="); } + + @Test + public void expirationNotAfterTimestampShouldBeRejected() { + long now = System.currentTimeMillis() / 1000; + String base = "v=1; a=rsa-sha256; d=example.com; s=sel; h=from; bh=AA==; b=AA==; "; + + assertThatThrownBy(() -> new SignatureRecordImpl(base + "t=" + (now + 1000) + "; x=" + (now + 500))) + .isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> new SignatureRecordImpl(base + "t=" + (now + 1000) + "; x=" + (now + 1000))) + .isInstanceOf(IllegalStateException.class); + // x= after t= stays valid + new SignatureRecordImpl(base + "t=" + now + "; x=" + (now + 1000)); + } } diff --git a/main/src/test/java/org/apache/james/jdkim/SignatureRecordTemplateTest.java b/main/src/test/java/org/apache/james/jdkim/SignatureRecordTemplateTest.java index 009eba2..4fb500a 100644 --- a/main/src/test/java/org/apache/james/jdkim/SignatureRecordTemplateTest.java +++ b/main/src/test/java/org/apache/james/jdkim/SignatureRecordTemplateTest.java @@ -188,4 +188,14 @@ public class SignatureRecordTemplateTest { assertThatThrownBy(() -> new SignatureRecordTemplate("v=1; a=rsa-sha256; d=example.com; s=s; h=from; i=.example.com")) .isInstanceOf(IllegalStateException.class); } + + @Test + public void expirationNotAfterTimestampShouldBeRejected() { + long now = System.currentTimeMillis() / 1000; + + assertThatThrownBy(() -> new SignatureRecordTemplate("v=1; a=rsa-sha256; d=example.com; s=s; h=from; t=" + (now + 1000) + "; x=" + (now + 1000))) + .isInstanceOf(IllegalStateException.class); + // an empty t= is filled in when signing + new SignatureRecordTemplate("v=1; a=rsa-sha256; d=example.com; s=s; h=from; t=; x=" + (now + 1000)); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
