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.

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

Jump to bottom

bpo-19180: Updated reference for RFC 1750 and RFC 3280 #148

Merged
merged 1 commit into from Jun 9, 2017
Merged

bpo-19180: Updated reference for RFC 1750 and RFC 3280 #148

merged 1 commit into from Jun 9, 2017

Conversation

chkumar246
Copy link
Contributor

  • RFC 1750 has been been obsoleted by RFC 4086.
  • RFC 3280 has been obsoleted by RFC 5280.
  • RFC 4366 has been obsoleted by RFC 6066.

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:

  1. If you don't have an account on b.p.o, please create one
  2. Make sure your GitHub username is listed in "Your Details" at b.p.o
  3. If you have not already done so, please sign the PSF contributor agreement
  4. If you just signed the CLA, please wait at least one US business day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
  5. Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

@serhiy-storchaka serhiy-storchaka added the docs Documentation in the Doc dir label Feb 18, 2017
Carreau
Carreau approved these changes Feb 22, 2017
View reviewed changes
Copy link
Contributor

@Carreau Carreau left a comment

Choose a reason for hiding this comment

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

For the little effect my review can have, I approve, and comment made by OP are correct.

RFC 4366 has been obsoleted by RFC 6066.

If another reviewer is wondering, this one is already cited a couple of lines after the end the diff.

@chkumar246
Copy link
Contributor Author

Thanks for the review. Any thing more to add to this patch?

@chkumar246
Copy link
Contributor Author

@Carreau @serhiy-storchaka anything more i can improve to this patch?

@orsenthil
Copy link
Member

@serhiy-storchaka - shall we move forward with this? The change looks good to me. These are See Also links which informative guidance ( like introduction to SSL/TLS from Apache site in the same list). Removing the obsolete RFC and mentioning the correct ones seems to be right thing to do.

orsenthil
orsenthil previously approved these changes Apr 4, 2017
View reviewed changes
@serhiy-storchaka
Copy link
Member

RFC references should be updated only if the ssl module actually supports new RFCs and is not sticked to obsolete RFCs. I'm not a SSL expert and don't know if it is. @tiran is an expert.

There are also other references to obsolete RFCs in ssl.rst and _ssl.c. Shouldn't they be updated too?

tiran
tiran requested changes Apr 4, 2017
View reviewed changes
Copy link
Member

@tiran tiran left a comment

Choose a reason for hiding this comment

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

Please update inline references, too.

Also please add RFC 6125 and link it from match_hostname.

Doc/library/ssl.rst
@@ -2306,14 +2306,11 @@ successful call of :func:`~ssl.RAND_add`, :func:`~ssl.RAND_bytes` or
`RFC 1422: Privacy Enhancement for Internet Electronic Mail: Part II: Certificate-Based Key Management <https://www.ietf.org/rfc/rfc1422>`_
Steve Kent

`RFC 1750: Randomness Recommendations for Security <https://www.ietf.org/rfc/rfc1750>`_
Copy link
Member

Choose a reason for hiding this comment

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

This RFC is referenced in the documentation of RAND_add. You need to update both occurrences.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Doc/library/ssl.rst

`RFC 4366: Transport Layer Security (TLS) Extensions <https://www.ietf.org/rfc/rfc4366>`_
Blake-Wilson et. al.
`RFC 5280: Internet X.509 Public Key Infrastructure Certificate and Certificate Revocation List (CRL) Profile <http://datatracker.ietf.org/doc/rfc5280/>`_
Copy link
Member

Choose a reason for hiding this comment

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

5280 is correct. Please update documentation of SSLSocket.getpeercert, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Doc/library/ssl.rst
`RFC 3280: Internet X.509 Public Key Infrastructure Certificate and CRL Profile <https://www.ietf.org/rfc/rfc3280>`_
Housley et. al.

`RFC 4366: Transport Layer Security (TLS) Extensions <https://www.ietf.org/rfc/rfc4366>`_
Copy link
Member

Choose a reason for hiding this comment

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

RFC 6066 is the successor to 4366. Please update documentation of HAS_SNI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@orsenthil orsenthil dismissed their stale review April 4, 2017 14:27

@tiran's comments overrules.

@ncoghlan
Copy link
Contributor

@chkumar246 Thanks for the initial work on this at the PyCon Pune sprints. Do you think you may be able to find time to follow up on @tiran's feedback, or would you prefer that someone else pick up the change and finish up those final details?

@ncoghlan
Copy link
Contributor

Added the sprint label, as this PR was submitted at the PyCon Pune 2017 core development sprint.

@chkumar246
Copy link
Contributor Author

Sorry for the late reply, I will update the PR soon.

@chkumar246
bpo-19180: Updated reference for RFC 1750 and RFC 3280
1da8be5 
* RFC 1750 has been been obsoleted by RFC 4086.
* RFC 3280 has been obsoleted by RFC 5280.
* RFC 4366 has been obsoleted by RFC 6066.
@chkumar246
Copy link
Contributor Author

@ncoghlan @tiran Thanks for the review. I have resolved the comments, please have a look.

@matrixise
Copy link
Member

Just add the url https://bugs.python.org/issue19180

@ncoghlan ncoghlan merged commit 63c2c8a into python:master Jun 9, 2017
@matrixise
Copy link
Member

thanks @ncoghlan

ncoghlan pushed a commit to ncoghlan/cpython that referenced this pull request Jun 9, 2017
@chkumar246 @ncoghlan
[3.6] bpo-19180: Updated references for RFC 1750, RFC 3280 & RFC 4366 (…
fe2d013 
…pythonGH-148)

* RFC 1750 has been been obsoleted by RFC 4086.
* RFC 3280 has been obsoleted by RFC 5280.
* RFC 4366 has been obsoleted by RFC 6066.
(cherry picked from commit 63c2c8a)
@ncoghlan
Copy link
Contributor

ncoghlan commented Jun 9, 2017

Backport: #2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Carreau Carreau Carreau approved these changes

@tiran tiran tiran approved these changes

@Mariatta Mariatta Mariatta approved these changes

@orsenthil orsenthil orsenthil left review comments

@birkenfeld birkenfeld Awaiting requested review from birkenfeld

Assignees
No one assigned
Labels
docs Documentation in the Doc dir sprint
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@chkumar246 @the-knights-who-say-ni @orsenthil @serhiy-storchaka @ncoghlan @matrixise @Carreau @tiran @Mariatta