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

Added a new PEP as draft: Enabling certificate verification by default for stdlib mail modules #3537

Closed
wants to merge 8 commits into from

Conversation

nitram2342
Copy link

@nitram2342 nitram2342 commented Nov 17, 2023

Basic requirements (all PEP Types)

  • Read and followed PEP 1 & PEP 12
  • File created from the latest PEP template
  • PEP has next available number, & set in filename (pep-NNNN.rst), PR title (PEP 123: <Title of PEP>) and PEP header
  • Title clearly, accurately and concisely describes the content in 79 characters or less
  • Core dev/PEP editor listed as Author or Sponsor, and formally confirmed their approval
  • Author, Status (Draft), Type and Created headers filled out correctly
  • PEP-Delegate, Topic, Requires and Replaces headers completed if appropriate
  • Required sections included
    • Abstract (first section)
    • Copyright (last section; exact wording from template required)
  • Code is well-formatted (PEP 7/PEP 8) and is in code blocks, with the right lexer names if non-Python
  • PEP builds with no warnings, pre-commit checks pass and content displays as intended in the rendered HTML
  • Authors/sponsor added to .github/CODEOWNERS for the PEP

Standards Track requirements

  • PEP topic discussed in a suitable venue with general agreement that a PEP is appropriate
  • Suggested sections included (unless not applicable)
    • Motivation
    • Rationale
    • Specification
    • Backwards Compatibility
    • Security Implications
    • How to Teach This
    • Reference Implementation
    • Rejected Ideas
    • Open Issues
  • Python-Version set to valid (pre-beta) future Python version, if relevant
  • Any project stated in the PEP as supporting/endorsing/benefiting from the PEP formally confirmed such
  • Right before or after initial merging, PEP discussion thread created and linked to in Discussions-To and Post-History

📚 Documentation preview 📚: https://pep-previews--3537.org.readthedocs.build/

@nitram2342 nitram2342 requested a review from a team as a code owner November 17, 2023 18:56
Copy link

cpython-cla-bot bot commented Nov 17, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

peps/pep-8106.rst Outdated Show resolved Hide resolved
peps/pep-8106.rst Outdated Show resolved Hide resolved
peps/pep-8106.rst Outdated Show resolved Hide resolved
peps/pep-8106.rst Outdated Show resolved Hide resolved
peps/pep-8106.rst Outdated Show resolved Hide resolved
.github/CODEOWNERS Outdated Show resolved Hide resolved
@hugovk
Copy link
Member

hugovk commented Nov 17, 2023

Please check this again, it looks like this file has been created from an old PEP rather than the PEP 12 PEP-NNNN.rst template, there's quite a few old headers and missing new headers in this PR.

Comment on lines +3 to +4
Author: Martin Schobert <[email protected]>
Sponsor: Victor Stinner <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer that you ask me to be a sponsor, and ask me to review the PEP before you go ahead and propose it directly.

Suggested change
Author: Martin Schobert <[email protected]>
Sponsor: Victor Stinner <[email protected]>
Author: Martin Schobert <[email protected]>

Copy link
Member

Choose a reason for hiding this comment

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

Context: I offered to sponsor a PEP if someone wants to write it: python/cpython#91826 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the background.

It's great to be proactive and write the PEP, but it's also very important to have a sponsor beforehand who's happy with the text, and also to help with the process. PEP 1 says:

Once the sponsor or the core developer(s) co-authoring the PEP deem the PEP ready for submission, the proposal should be submitted as a draft PEP via a GitHub pull request.

I suggest we close this PR for now, and @nitram2342 contacts @vstinner privately and they decide how to proceed. If they can agree on a text together, then update and re-open this PR, or a fresh one, and we can take it from there.

How does that sound?

Copy link
Member

Choose a reason for hiding this comment

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

If someone wants me to sponsor a PEP, I would prefer to read it and approve it ahead, right.

@nitram2342: If you would like to sponsor your PEP, I suggest you closing this PR, as @hugovk suggests.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, that is fine. I will do it this way.

@nitram2342 nitram2342 closed this Nov 18, 2023
Smattr added a commit to Smattr/needtoknow that referenced this pull request Feb 2, 2024
The Python docs say:¹

  _ssl_context_ is a `ssl.SSLContext` object which allows bundling SSL
  configuration options, certificates and private keys into a single
  (potentially long-lived) structure. Please read Security considerations
  for best practices.
  …
  For client use, if you don’t have any special requirements for your security
  policy, it is highly recommended that you use the `create_default_context()`
  function to create your SSL context. It will load the system’s trusted CA
  certificates, enable certificate validation and hostname checking, and try to
  choose reasonably secure protocol and cipher settings.
  …
  By contrast, if you create the SSL context by calling the `SSLContext`
  constructor yourself, it will not have certificate validation nor hostname
  checking enabled by default.

While this is clear, it is counter-intuitive behaviour of which I was unaware.
I only learned of this through an oss-sec posting.² This issue seems to have a
long history and we are not the only software affected by it.³

¹ https://docs.python.org/3/library/imaplib.html#imaplib.IMAP4_SSL
² https://www.openwall.com/lists/oss-security/2024/02/01/4
³ python/cpython#91826,
  https://peps.python.org/pep-0476/,
  python/cpython#91875,
  https://www.pentagrid.ch/en/blog/python-mail-libraries-certificate-verification/,
  python/peps#3537
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.

4 participants