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

updated RFC 4408 citation to RFC 7208 and added link in README #137

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

chrislandis
Copy link

@chrislandis chrislandis commented Jun 12, 2023

🗣 Description

This pull request updates the README's RFC 4408 citation to RFC 7208 and adds a link thereto.

💭 Motivation and context

We do not want trustymail documentation (or code) to cite superseded references. RFC 7208 superseded RFC 4408. Resolves #136.

🧪 Testing

After making the change to the README, I pushed the change to my forked repo and could proof-read the text and test the link from there.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated to reflect the changes in this PR.
  • All new and existing tests pass.

@jsf9k jsf9k self-assigned this Jun 13, 2023
@jsf9k jsf9k added the documentation This issue or pull request improves or adds to documentation label Jun 13, 2023
@jsf9k jsf9k added this to In progress in BOD 18-01 via automation Jun 13, 2023
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution @chrislandis!

@jsf9k jsf9k requested a review from a team June 13, 2023 12:48
@jsf9k jsf9k enabled auto-merge June 13, 2023 12:48
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Before we change the documentation has the code been verified to conform to 7208? RFC 4408 was experimental and there are likely to have been changes in the proposed standard (RFC 7208). If this was originally written against 4408 then before we claim it checks records in accordance with 7208 we should make sure that is the case.

BOD 18-01 automation moved this from In progress to Review in progress Jun 13, 2023
@chrislandis
Copy link
Author

Before we change the documentation has the code been verified to conform to 7208? RFC 4408 was experimental and there are likely to have been changes in the proposed standard (RFC 7208). If this was originally written against 4408 then before we claim it checks records in accordance with 7208 we should make sure that is the case.

Great point, @mcdonnnj! In my look at the code (which was not a thorough verification), I did not happen to see any way in which trustymail acted contrary to RFC 7208; however, I did not conduct a full analysis to ensure that there were no missing actions. In other words, to achieve a formal verification, we would need to ensure both soundness (that trustymail acts strictly within the SPF specification, not doing extra stuff related to SPF) and completeness (that trustymail does everything in the SPF specification).

There are various courses of action that we could take from here in support of this verification.

  • We could pause this pull request to complete the verification. If we find problems with soundness or completeness, each of those would become its own new issue. Once all (if any) issues are identified and created (but not necessarily resolved), we could advance this pull request.
  • Alternatively, we could proceed with this pull request now and create a new issue that tasks trustymail's SPF verification with RFC 7208.
  • There may be a different way that is preferable to the CISA Team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This issue or pull request improves or adds to documentation
Projects
Status: Review in progress
Status: No status
BOD 18-01
  
Review in progress
Development

Successfully merging this pull request may close these issues.

Citing Obsolete SPF RFC 4408 vice Current RFC 7208
3 participants