apache
First commit
Thanks all for taking time to review and providing your feedback! Keep it coming! I will work on improving the code in the coming days.
I hope I can convince you not to follow the existing DKIM api which is really not so great. please have a look at the proposed API in https://github.com/apache/james-jdkim/pull/27 if we are going to introduce a completely new feature set we should strive to improve the design
I'm not saying my proposed API design is perfect (by no means) but it should be better than what we have today
This work is wonderful, thanks for putting it together.
I have mostly nits and checkstyles.
However I do believe we need to invest in significant testing with at least a dozen messages and ensure a good overall test coverage,
also while dmarc makes sense as part of arc, but it also makes sense of its own. How about extracting DMARC tooling (record parsing & evaluation) in a standalong
dmarcmaven module we could depend on?
Thanks for the kind comments, Benoit!
100% agree on need in broad testing covering all possible edge cases that I might have missed. I wish Appendix B in RFC 8617 had more examples as well as signing/public key pairs used in them.
Let me think on making a module out of dmarc. My first instinct says it will probably be a very small module. But if it makes sense to keep is separate, we can make it happen.
I hope I can convince you not to follow the existing DKIM api which is really not so great. please have a look at the proposed API in #27 if we are going to introduce a completely new feature set we should strive to improve the design
I'm not saying my proposed API design is perfect (by no means) but it should be better than what we have today
Sure, I will take a look at the proposed API and see how we can change it.
I checked it a new commit with updates. Outstanding on my list:
- look into making a module out of
dmarc - check proposed API in https://github.com/apache/james-jdkim/pull/27 and see how we can align with it
- investigate DMARC strict vs relaxed. Make sure both are supported. Let me know if I missed anything.
Let me think on making a module out of dmarc. My first instinct says it will probably be a very small module. But if it makes sense to keep is separate, we can make it happen.
I'd see a point: discoverability.
Just like OpenDmarc and OpenArc are 2 distict projects.
I wish Appendix B in RFC 8617 had more examples as well as signing/public key pairs used in them.
Maybe we could reach the DMARC work group that published the spec and ask if there is a TCK (Technology COmpliance Kit): [email protected] ?
I had a look at OpenARC tests but they are thin.
What method can be used to know if the sealer can be trusted ? I didn't see any in the ARCVerifier.
What method can be used to know if the sealer can be trusted ? I didn't see any in the ARCVerifier.
The logic to validate previous ARC sealers is included in the ARCChainValidator class. This is the entry point for it
public ArcValidationResult validateArcChain(Message message). If our instance is between 1 and 50 we go into validatePreviousArcHops where we recompute the hashes on previous AMS and AS headers and check them using sealers' public keys pulled from DNS.
What method can be used to know if the sealer can be trusted ? I didn't see any in the ARCVerifier.
The logic to validate previous ARC sealers is included in the
ARCChainValidatorclass. This is the entry point for itpublic ArcValidationResult validateArcChain(Message message). If our instance is between 1 and 50 we go intovalidatePreviousArcHopswhere we recompute the hashes on previous AMS and AS headers and check them using sealers' public keys pulled from DNS.
My usage of ARC was limited, but as far as I know, no ARC sealer can be trusted. I think it was designed to be an opt-in feature. Only accept ARC headers from previously trusted (and configured) servers. Like Microsoft does, you need to configure the sealer there. A sealer can be a malicious actor with a valid signature modifying messages, and your server will deliver an authenticated phishing or spam. These things must be treated with caution. The RFC 8617 is experimental.
From rfc8617#section-7.1: "If ARC-enabled ADMDs are trusted, Authenticated Received Chains can be used to bridge administrative boundaries."
The section rfc8617#section-9.4 (Message Sealer Suspicion), says sealers must be treated with suspicion.
Let me think on making a module out of dmarc. My first instinct says it will probably be a very small module. But if it makes sense to keep is separate, we can make it happen.
I'd see a point: discoverability.
Just like OpenDmarc and OpenArc are 2 distict projects.
I wish Appendix B in RFC 8617 had more examples as well as signing/public key pairs used in them.
Maybe we could reach the DMARC work group that published the spec and ask if there is a TCK (Technology COmpliance Kit): [email protected] ?
I had a look at OpenARC tests but they are thin.
Yes, discoverability makes sense. I can move it into it's own module under james-jdkim. A few additional things to consider before we do that:
- what about SPF, does it make sense to put it in its own module too? I am not opposed to it and think logically it might make sense if we decide to separate DMARC.
- sounds like @epinter is working on DMARC library, so may be it would make more sense to integrate with it?
what about SPF, does it make sense to put it in its own module too? I am not opposed to it and think logically it might make sense if we decide to separate DMARC.
SPF is already a standalong library of its own. I'll scope it as "let's keep this a dependency of DMARC"
CF what about SPF, does it make sense to put it in its own module too? I am not opposed to it and think logically it might make sense if we decide to separate DMARC.
sounds like @epinter is working on DMARC library, so may be it would make more sense to integrate with it?
If @epinter agrees I think a dmarc library in Java fully deserve to be in the Apache James umbrella.
No objections to depend on it though.
what about SPF, does it make sense to put it in its own module too? I am not opposed to it and think logically it might make sense if we decide to separate DMARC.
SPF is already a standalong library of its own. I'll scope it as "let's keep this a dependency of DMARC"
CF what about SPF, does it make sense to put it in its own module too? I am not opposed to it and think logically it might make sense if we decide to separate DMARC.
sounds like @epinter is working on DMARC library, so may be it would make more sense to integrate with it?
If @epinter agrees I think a dmarc library in Java fully deserve to be in the Apache James umbrella.
I agree.
No objections to depend on it though.
I think the development of this PR doesn't need to stop because of dmarc. Now it is just a method call to check alignment, it will be easy to replace with a call to a future library.
I plan a very simple API, something like public DmarcResult verify(String rfc5322domain, Identifier... identifiers).
Hey guys!
I committed changes with DMARC in a separate module and also implemented both strict and relaxed header checking against PSL list.
The Jenkins build fails though because MockPublicKeyRecordRetrieverDmarc test class that is in DMARC module is being used by test classes in the ARC module. It needs to build apache-dmarc-library-0.6-SNAPSHOT-tests.jar first so that ARC tests can find it. Tried changing a few things in POM, and they did not help. Will appreciate any suggestions. Works fine on my local.
I have limited bandwith because of over-committing on another project.
I will take time for a complete review of this work this weekend.