Skip to content
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.

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

Backport: 5x -> 2x ARH checking and code structure #496

Open
wants to merge 7 commits into
base: 2.x
Choose a base branch
from

Conversation

dodmi
Copy link
Contributor

@dodmi dodmi commented Feb 24, 2025

Hi,

I followed your changes to separate the code for "cached results" and "ARH checking" to different modules.
Additionally, I backported the change to not check the AUID anymore, after reading the RFC.

But, I don't understand your changes to setting SDID and AUID for ARHs:
Instead of always complementing SDID and AUID, it's only done, if the ARH result replaces DKIM verification.

  • Why did you move the code to do the complementation only in this case?
  • As AUID isn't checked anymore, why is it complemented with an existing SDID, anyway?
  • If neither AUID nor SDID is given, SDID stays undefined
  • If there's no SDID and ARH result doesn't replace the DKIM verification, SDID stays undefined
  • But the SUCCESS string contains a field for the SDID

At the moment, my code follows the old behavior to complement SDID/AUID and I've copied your check method, which will do the same. Depending on your reply, I'll remove the code in one place.

As SDID may be undefined in the case, that neither SDID nor AUID is defined, in any case, we'll probably need to change the SUCCESS message. (I didn't have any real world mail and just realized this, checking your new test cases)

Other topic: I didn't backport the changes to the libunbound files. I couldn't get it working, trying to backport the changes.

And I hope, I didn't miss anything

@dodmi dodmi closed this Feb 24, 2025
@dodmi
Copy link
Contributor Author

dodmi commented Feb 24, 2025

Sorry, wrong branch...

@dodmi dodmi reopened this Feb 24, 2025
@dodmi dodmi changed the base branch from master to 2.x February 24, 2025 19:35
- Removed additional check for from alignment (is now done with policy check)
- Removed check for auid alignment
@dodmi dodmi force-pushed the backport-5x-2x-arh-checking-and-code-structure branch from 55b9727 to 077a256 Compare February 25, 2025 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant