Skip to content

RFC - Proposal for an updated JDKIM API #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

RFC - Proposal for an updated JDKIM API #27

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
197 changes: 185 additions & 12 deletions main/src/test/java/org/apache/james/jdkim/DKIMTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.james.jdkim;

import static java.util.stream.Collectors.joining;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;

Expand All @@ -34,13 +35,17 @@
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Stream;

import org.apache.commons.codec.binary.Base64;
import org.apache.james.jdkim.MockPublicKeyRecordRetriever.Record;
import org.apache.james.jdkim.api.Headers;
import org.apache.james.jdkim.api.Result;
import org.apache.james.jdkim.api.SignatureRecord;
import org.apache.james.jdkim.impl.Message;
import org.apache.james.jdkim.tagvalue.SignatureRecordTemplate;
import org.junit.Test;

public class DKIMTest {
Expand All @@ -62,33 +67,37 @@ public class DKIMTest {
* - "bh=" and "b=" placeholder are required for now because the same implementation is used for
* signing and verifying. The fields are mandatory for verifying.
*/
private static final String SIGNATURE_TEMPLATE = "v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=date:from:subject; q=dns/txt; s=selector2;";
private static final String SIGNATURE_TEMPLATE = "v=1; c=simple; d=messiah.edu; h=date:from:subject; q=dns/txt; s=selector2;";
private static final String SIGNATURE_TEMPLATE_2 = "v=1; a=rsa-sha1; c=simple; d=messiah.edu; h=date:from:subject; q=dns/txt; s=selector2;";
private static final String SIGNATURE_TEMPLATE_3 = "v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=date:from:subject; q=dns/txt; s=selector3;";

private final DKIMSigner dkimSigner = new DKIMSigner(SIGNATURE_TEMPLATE, TestKeys.privateKey);
private final DKIMSigner dkimSigner = new DKIMSigner();
private final DKIMVerifier verifier = new DKIMVerifier(keyRecordRetriever);


@Test
public void should_verify_generated_signature_single_key() throws Exception {
String expectedSignature = "DKIM-Signature: a=rsa-sha256; q=dns/txt; b=Axa8s/gTnnJ8em45KV/AQw33hQ4uYtBKiQp3dLq7oRFt+WmDZ5ZErPq4lBVXfP+IAvP+Au91J8270ivn1J/6E0YqKntn4s1hjcNBPPRVohvmlcQ1mEMd6DuYDtWjDFwG2GWZwtilaPY2afhlTuAbHkn8nHm7MVtAGETO8QQ2zfD1NSGzKbNYP9I+hrDJq5ajka6PZn1d+mDhUH5Px8yScYqo5i8Z8GXaejSIu7RsLDuxtOO2cuClRi8MKGxc7MiGndMufXB8xbS1L80IFlyunOVY5eBaqnnhF2YrDDQfZ6DTqorzX6D5dNjpjOG6AbsqkW83Drx0TTV/5M0raU1SIw==; c=simple; s=selector2; d=messiah.edu; v=1; bh=6pQY5V6Dw8mCYWq017gfbpv+x2X4GvOhIIZtKw6iU6g=; h=date:from:subject;";
DkimSignatureHeader expectedSignature = new DkimSignatureHeader("a=rsa-sha256; q=dns/txt; b=Axa8s/gTnnJ8em45KV/AQw33hQ4uYtBKiQp3dLq7oRFt+WmDZ5ZErPq4lBVXfP+IAvP+Au91J8270ivn1J/6E0YqKntn4s1hjcNBPPRVohvmlcQ1mEMd6DuYDtWjDFwG2GWZwtilaPY2afhlTuAbHkn8nHm7MVtAGETO8QQ2zfD1NSGzKbNYP9I+hrDJq5ajka6PZn1d+mDhUH5Px8yScYqo5i8Z8GXaejSIu7RsLDuxtOO2cuClRi8MKGxc7MiGndMufXB8xbS1L80IFlyunOVY5eBaqnnhF2YrDDQfZ6DTqorzX6D5dNjpjOG6AbsqkW83Drx0TTV/5M0raU1SIw==; c=simple; s=selector2; d=messiah.edu; v=1; bh=6pQY5V6Dw8mCYWq017gfbpv+x2X4GvOhIIZtKw6iU6g=; h=date:from:subject;");

SignatureRecordTemplate recordTemplate = new SignatureRecordTemplate(SIGNATURE_TEMPLATE);
ByteArrayInputStream inputStream = readFileToByteArrayInputStream("/org/apache/james/jdkim/Mail-DKIM/corpus/multiple_2.txt");
String actualSignature = dkimSigner.sign(inputStream);

DkimSignatureHeader actualSignature = dkimSigner.sign(inputStream, recordTemplate, TestKeys.privateKey); // optional HashMethod parameter defaults to SHA-256

// sanity check as it is very easy to change the bodyhash behaviour and thus break the signature
assertEquals(expectedSignature, actualSignature);

InputStream originalInputStream = readFileToByteArrayInputStream("/org/apache/james/jdkim/Mail-DKIM/corpus/multiple_2.txt");
InputStream signatureInputStream = new ByteArrayInputStream((actualSignature + "\r\n").getBytes(StandardCharsets.UTF_8));
InputStream signatureInputStream = new ByteArrayInputStream((actualSignature.asMimeHeader() + "\r\n").getBytes(StandardCharsets.UTF_8));

SequenceInputStream verifyInputStream = new SequenceInputStream(signatureInputStream, originalInputStream);
List<SignatureRecord> verifiedSignatures = verifier.verify(verifyInputStream);
DkimVerification dkimVerification = verifier.verify(verifyInputStream);

assertThat(verifiedSignatures)
assertThat(dkimVerification.isSuccess).isTrue();
assertThat(dkimVerification.signatureVerifications)
.hasSize(1)
.allSatisfy(it ->
assertThat("DKIM-Signature:" + it.toString()).isEqualTo(expectedSignature)
assertThat("DKIM-Signature:" + it.asHeaderString()).isEqualTo(expectedSignature)
);

}
Expand All @@ -98,9 +107,9 @@ public void should_verify_generated_signature_multiple_keys() throws Exception {
// You might want to sign with multiple keys to support several hashmethods on the same selector
// or several signing algorithm on the same selector ( ECDSA is a proposed alternative to rsa )
// or when upgrading keys for any reason to add several signatures using different selectors.
String expectedSignature = "DKIM-Signature: a=rsa-sha256; q=dns/txt; b=Axa8s/gTnnJ8em45KV/AQw33hQ4uYtBKiQp3dLq7oRFt+WmDZ5ZErPq4lBVXfP+IAvP+Au91J8270ivn1J/6E0YqKntn4s1hjcNBPPRVohvmlcQ1mEMd6DuYDtWjDFwG2GWZwtilaPY2afhlTuAbHkn8nHm7MVtAGETO8QQ2zfD1NSGzKbNYP9I+hrDJq5ajka6PZn1d+mDhUH5Px8yScYqo5i8Z8GXaejSIu7RsLDuxtOO2cuClRi8MKGxc7MiGndMufXB8xbS1L80IFlyunOVY5eBaqnnhF2YrDDQfZ6DTqorzX6D5dNjpjOG6AbsqkW83Drx0TTV/5M0raU1SIw==; c=simple; s=selector2; d=messiah.edu; v=1; bh=6pQY5V6Dw8mCYWq017gfbpv+x2X4GvOhIIZtKw6iU6g=; h=date:from:subject;";
String expectedSignature2 = "DKIM-Signature: a=rsa-sha1; q=dns/txt; b=YiwcfjqM7myZ/OENExlyGVzy+rg/779R6pF7bPl79aL6e7yGYeN0XdLRcJEqhg+/uNFwcC7zrbWUwPBVFpFN8pKdQT7TgTr+ydoN65QiBa/rXH4m8Ga+oKx8652dXAHm9oMvG166VdMRsEKTJq2bFpM9RR4mW0KtHPte2JWiOtCYO6MPTlWA2JnIgQp+3+03rnOcKdQ+sn/bi9OwanwE4jgIBcPeHkHVr1fVsV53nvDlbk1DiDX+uOXvuk6bjPVBN4srZiSIvFKsmco0tGZx8cgs5OKyjmtIWVOvjxgupXWvEJJ1nMi1UQ1AXh6jDqWrMDCioRCMG9TeGy8fjcjcfw==; c=simple; s=selector2; d=messiah.edu; v=1; bh=q6DWKdHUzNbVPt6YBbD1KOai/b8=; h=date:from:subject;";
String expectedSignature3 = "DKIM-Signature: a=rsa-sha256; q=dns/txt; b=crkBsqVuTJJmjZyNuJtmGXBsHIT7tJq0ONWLvNfO29sl1kNm9UzTZ4mOYR+akNJqonkaFFaVM9MZ/6QUd5NbGaIytxXnxv+NPNu6ZzUunlcRyOPhEQ/znemg3WjibRs24gWubBZZXApkqQ9kFh/atatoaJhTls/lnbP8ZV3XlWVN12UuESU3qdieRvrhKWX5/Od7LqZS04ZTeToAabOtDmm6hYl2R6wxizdHrOkGiNERfbB8Iaws5f3Qnt0S94wQ5FVTPzgRiO9OW8hYbAijS4Bh8/NXV5xauMXjCETfxX3pQYUuxc4QVhnoMbmuqgEulzuJUzapjotLLFxQRaSsjw==; c=simple; s=selector3; d=messiah.edu; v=1; bh=6pQY5V6Dw8mCYWq017gfbpv+x2X4GvOhIIZtKw6iU6g=; h=date:from:subject;";
DkimSignatureHeader expectedSignature = new DkimSignatureHeader("a=rsa-sha256; q=dns/txt; b=Axa8s/gTnnJ8em45KV/AQw33hQ4uYtBKiQp3dLq7oRFt+WmDZ5ZErPq4lBVXfP+IAvP+Au91J8270ivn1J/6E0YqKntn4s1hjcNBPPRVohvmlcQ1mEMd6DuYDtWjDFwG2GWZwtilaPY2afhlTuAbHkn8nHm7MVtAGETO8QQ2zfD1NSGzKbNYP9I+hrDJq5ajka6PZn1d+mDhUH5Px8yScYqo5i8Z8GXaejSIu7RsLDuxtOO2cuClRi8MKGxc7MiGndMufXB8xbS1L80IFlyunOVY5eBaqnnhF2YrDDQfZ6DTqorzX6D5dNjpjOG6AbsqkW83Drx0TTV/5M0raU1SIw==; c=simple; s=selector2; d=messiah.edu; v=1; bh=6pQY5V6Dw8mCYWq017gfbpv+x2X4GvOhIIZtKw6iU6g=; h=date:from:subject;");
DkimSignatureHeader expectedSignature2 = new DkimSignatureHeader("a=rsa-sha1; q=dns/txt; b=YiwcfjqM7myZ/OENExlyGVzy+rg/779R6pF7bPl79aL6e7yGYeN0XdLRcJEqhg+/uNFwcC7zrbWUwPBVFpFN8pKdQT7TgTr+ydoN65QiBa/rXH4m8Ga+oKx8652dXAHm9oMvG166VdMRsEKTJq2bFpM9RR4mW0KtHPte2JWiOtCYO6MPTlWA2JnIgQp+3+03rnOcKdQ+sn/bi9OwanwE4jgIBcPeHkHVr1fVsV53nvDlbk1DiDX+uOXvuk6bjPVBN4srZiSIvFKsmco0tGZx8cgs5OKyjmtIWVOvjxgupXWvEJJ1nMi1UQ1AXh6jDqWrMDCioRCMG9TeGy8fjcjcfw==; c=simple; s=selector2; d=messiah.edu; v=1; bh=q6DWKdHUzNbVPt6YBbD1KOai/b8=; h=date:from:subject;");
DkimSignatureHeader expectedSignature3 = new DkimSignatureHeader("DKIM-Signature: a=rsa-sha256; q=dns/txt; b=crkBsqVuTJJmjZyNuJtmGXBsHIT7tJq0ONWLvNfO29sl1kNm9UzTZ4mOYR+akNJqonkaFFaVM9MZ/6QUd5NbGaIytxXnxv+NPNu6ZzUunlcRyOPhEQ/znemg3WjibRs24gWubBZZXApkqQ9kFh/atatoaJhTls/lnbP8ZV3XlWVN12UuESU3qdieRvrhKWX5/Od7LqZS04ZTeToAabOtDmm6hYl2R6wxizdHrOkGiNERfbB8Iaws5f3Qnt0S94wQ5FVTPzgRiO9OW8hYbAijS4Bh8/NXV5xauMXjCETfxX3pQYUuxc4QVhnoMbmuqgEulzuJUzapjotLLFxQRaSsjw==; c=simple; s=selector3; d=messiah.edu; v=1; bh=6pQY5V6Dw8mCYWq017gfbpv+x2X4GvOhIIZtKw6iU6g=; h=date:from:subject;");

ByteArrayInputStream inputStream1 = readFileToByteArrayInputStream("/org/apache/james/jdkim/Mail-DKIM/corpus/multiple_2.txt");
String actualSignature1 = dkimSigner.sign(inputStream1);
Expand All @@ -119,7 +128,7 @@ public void should_verify_generated_signature_multiple_keys() throws Exception {
// prepend signatures to the message as per https://datatracker.ietf.org/doc/html/rfc6376#section-3.5
InputStream originalInputStream = readFileToByteArrayInputStream("/org/apache/james/jdkim/Mail-DKIM/corpus/multiple_2.txt");
String signatures = String.join("\r\n", Arrays.asList(actualSignature1, actualSignature2, actualSignature3));
InputStream signatureInputStream = new ByteArrayInputStream((signatures+"\r\n").getBytes(StandardCharsets.UTF_8));
InputStream signatureInputStream = new ByteArrayInputStream((signatures + "\r\n").getBytes(StandardCharsets.UTF_8));

SequenceInputStream verifyInputStream = new SequenceInputStream(signatureInputStream, originalInputStream);

Expand Down Expand Up @@ -183,4 +192,168 @@ public InputStream getBodyInputStream() {
return headers.getBodyInputStream();
}
}

// ======== IGNORE CODE BELOW THIS FOR STARTERS ====== //

private static class DkimSignatureHeader {
private final String name = "DKIM-Signature";
private final String value;

private DkimSignatureHeader(String value) {
this.value = value;
}

public String asMimeHeader() {
return name + ": " + value;
}

@Override
public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) return false;
DkimSignatureHeader that = (DkimSignatureHeader) o;
return Objects.equals(name, that.name) && Objects.equals(value, that.value);
}

@Override
public int hashCode() {
return Objects.hash(name, value);
}
}

/**
* source https://datatracker.ietf.org/doc/html/rfc8601#section-2.7.1
*/
private static enum SignatureVerificationResult {
// NONE means The message was not signed so it cannot be a signature result.
/**
* The message was signed, the signature or signatures were acceptable
* to the ADMD, and the signature(s) passed verification tests.
*/
PASS,
/**
* The message was signed and the signature or signatures were
* acceptable to the ADMD, but they failed the verification test(s).
*/
FAIL,
/**
* The message was signed, but some aspect of the signature or
* signatures was not acceptable to the ADMD.
Copy link
Contributor

@chibenwa chibenwa Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is an ADMD ?

Copy link
Contributor Author

@jeantil jeantil Jun 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ADministrative Management Domain (ADMD)

The descriptions are copy pasted straight from the [Message Header Field for Indicating Message Authentication Status RFC] (https://datatracker.ietf.org/doc/html/rfc7001#section-2.6.)
The full definition for an administrative management domain is given this section

A final version of the API should include these references in the comments of the class

*/
POLICY,
/**
* The message was signed, but the signature or signatures contained
* syntax errors or were not otherwise able to be processed.
* This result is also used for other failures not covered elsewhere
* in this list.
*/
NEUTRAL,
/**
* The message could not be verified due to some error th at is likely
* transient in nature, such as a temporary inability to retrieve a
* public key.
* A later attempt may produce a final result.
*/
TEMPFAIL,
/**
* The message could not be verified due to some error that is
* unrecoverable, such as a required header field being absent.
* A later attempt is unlikely to produce a final result.
*/
PERMFAIL
}

private enum FailedHeaderTagName {
SIGNING_DOMAIN("header.d"),
IDENTITY("header.i"),
ALGORITHM("header.a"),
SELECTOR("header.s");

private final String value;

FailedHeaderTagName(String value) {
this.value = value;
}
}

private static class FailedHeaderTag {
private final FailedHeaderTagName tagName;
private final String value;

private FailedHeaderTag(FailedHeaderTagName tagName, String value) {
this.tagName = tagName;
this.value = value;
}

public String asHeaderString() {
return tagName.value + "=" + value + ";";
}
}

private static class SignatureVerification {
private final SignatureVerificationResult result;
private final Optional<String> reason;
private final Optional<FailedHeaderTag> tag;

private SignatureVerification(
SignatureVerificationResult result,
Optional<String> reason,
Optional<FailedHeaderTag> tag
) {
this.result = result;
this.reason = reason;
this.tag = tag;
}

String asHeaderString() {
String reason = this.reason.map(it -> "reason=" + it).orElse("");
String tag = this.tag.map(FailedHeaderTag::asHeaderString).orElse("");

switch (result) {
case PASS:
return "dkim=pass (good signature);";
case FAIL:
return Stream.of("dkim=fail", reason, tag)
.filter(it->!it.isEmpty())
.collect(joining("", " ", ";"));
case POLICY:
return Stream.of("dkim=policy", reason, tag)
.filter(it->!it.isEmpty())
.collect(joining("", " ", ";"));
case NEUTRAL:
return Stream.of("dkim=neutral", reason, tag)
.filter(it->!it.isEmpty())
.collect(joining("", " ", ";"));
case TEMPFAIL:
return Stream.of("dkim=temperror", reason, tag)
.filter(it->!it.isEmpty())
.collect(joining("", " ", ";"));
case PERMFAIL:
return Stream.of("dkim=permerror", reason, tag)
.filter(it->!it.isEmpty())
.collect(joining("", " ", ";"));
Comment on lines +314 to +333
Copy link
Contributor

@chibenwa chibenwa Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor remark: we could get rid of a bit of duplication with method extraction as all those do the same?

Copy link
Contributor Author

@jeantil jeantil Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't even tried to polish the implementation. this PR focuses on the shape of the API, supporting implementation is only here to help understand the shape of the API and by no means definitive or mergeable :D

default:
throw new IllegalStateException("unreachable code, which will disappear in later java versions with exhaustivity checking");
}
}
}

private static class DkimVerification {
private final List<SignatureVerification> signatureVerifications;
boolean isSuccess;
Copy link
Contributor

@chibenwa chibenwa Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final?


private DkimVerification(List<SignatureVerification> signatureVerifications) {
this.signatureVerifications = signatureVerifications;
isSuccess = signatureVerifications.stream()
.anyMatch(it ->
it.result == SignatureVerificationResult.PASS
);
}

String asHeaderString() {
return signatureVerifications.stream()
.map(it ->
it.asHeaderString()).collect(joining(";\r\n")
);
}
}
}