GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1 by davidben · Pull Request #100373 · python/cpython

Conversation

@davidben

@davidben davidben commented Dec 20, 2022 •
edited by miss-islington
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

In PEM, we need to parse until error and then suppress PEM_R_NO_START_LINE, because PEM allows arbitrary leading and trailing data. DER, however, does not. Parsing until error and suppressing ASN1_R_HEADER_TOO_LONG doesn't quite work because that error also covers some cases that should be rejected.

Instead, check BIO_eof early and stop the loop that way.

  • Issue: SSLContext.load_verify_locations accepts some cases of trailing data in DER #100372

Automerge-Triggered-By: GH:Yhg1s

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@davidben
pythongh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1
863a0a7
In PEM, we need to parse until error and then suppress
PEM_R_NO_START_LINE, because PEM allows arbitrary leading and trailing
data. DER, however, does not. Parsing until error and suppressing
ASN1_R_HEADER_TOO_LONG doesn't quite work because that error also
covers some cases that should be rejected.

Instead, check BIO_eof early and stop the loop that way.
@bedevere-bot bedevere-bot mentioned this pull request Dec 20, 2022
SSLContext.load_verify_locations accepts some cases of trailing data in DER #100372
Closed
@bedevere-bot bedevere-bot added the awaiting review label Dec 20, 2022
@davidben

davidben commented Mar 16, 2023

Copy link
Copy Markdown
Contributor Author

@tiran This PR look reasonable? Anything missing on my end?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Yhg1s
Merge branch 'main' into der-eof
2b80eb0
@Yhg1s
Yhg1s self-requested a review March 24, 2023 12:36
Yhg1s
Yhg1s approved these changes Mar 24, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Mar 24, 2023
@Yhg1s Yhg1s added the 🤖 automerge label Mar 24, 2023
@Yhg1s

Yhg1s commented Mar 24, 2023

Copy link
Copy Markdown
Member

I don't think this is a security issue, or at least not serious enough to backport to security-only releases. Do you disagree, @davidben, or is there a security angle I'm missing?

I'm not sure if this should be backported to 3.11/3.10 either. It's a bug, but it doesn't feel important enough to backport and risk breaking users who rely on the old broken behaviour.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington

miss-islington commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

Status check is done, and it's a success ✅.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington
miss-islington merged commit acfe02f into python:main Mar 24, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Mar 24, 2023
@davidben

davidben commented Mar 24, 2023

Copy link
Copy Markdown
Contributor Author

Do you disagree, @davidben, or is there a security angle I'm missing?

Nah, can't think of any security angle. Just generally improving behavior and reducing dependency on OpenSSL error codes. (Conditioning on OpenSSL error codes can be a bit messy. Sometimes you have to, like the PEM case here, but other times the error codes don't correspond enough to clear, stable conditions to condition on. :-( )

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request Mar 27, 2023
@davidben @Fidget-Spinner
pythongh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1 (pyt…
387cc11
…honGH-100373)

In PEM, we need to parse until error and then suppress `PEM_R_NO_START_LINE`, because PEM allows arbitrary leading and trailing data. DER, however, does not. Parsing until error and suppressing `ASN1_R_HEADER_TOO_LONG` doesn't quite work because that error also covers some cases that should be rejected.

Instead, check `BIO_eof` early and stop the loop that way.

Automerge-Triggered-By: GH:Yhg1s
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@davidben @warsaw
pythongh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1 (pyt…
ef6ca4e
…honGH-100373)

In PEM, we need to parse until error and then suppress `PEM_R_NO_START_LINE`, because PEM allows arbitrary leading and trailing data. DER, however, does not. Parsing until error and suppressing `ASN1_R_HEADER_TOO_LONG` doesn't quite work because that error also covers some cases that should be rejected.

Instead, check `BIO_eof` early and stop the loop that way.

Automerge-Triggered-By: GH:Yhg1s
@bh1428 bh1428 mentioned this pull request Aug 10, 2025
ssl.create_default_context() throws: "ssl.SSLError: [X509] unknown error (_ssl.c:4035)" #108721
Closed
2 tasks done
@h-vetinari h-vetinari mentioned this pull request Jun 11, 2026
openssl 3.6.3 and 3.5.7 regress DER cadata loading, breaking ssl.create_default_context on Windows conda-forge/openssl-feedstock#237
Closed
@h-vetinari

h-vetinari commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

I'm not sure if this should be backported to 3.11/3.10 either. It's a bug, but it doesn't feel important enough to backport and risk breaking users who rely on the old broken behaviour.

FWIW, the fact that this was not backported is now affecting 3.10/3.11 versions together with the newest set of openssl versions (e.g. 3.6.3, 3.5.7): conda-forge/openssl-feedstock#237

The patch backports cleanly to 3.10 & 3.11 (and fixes the regression), which is what I'm looking towards doing as a fix for this in conda-forge.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Jun 11, 2026
3.10 openssl rebuild AnacondaRecipes/python-feedstock#221
Merged
3.11 rebuild: fix openssl 3.5.7 AnacondaRecipes/python-feedstock#222
Merged
@encukou encukou added needs backport to 3.10 needs backport to 3.11 only security fixes labels Jul 8, 2026
@miss-islington-app

miss-islington-app Bot commented Jul 8, 2026

Copy link
Copy Markdown

Thanks @davidben for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington-app

miss-islington-app Bot commented Jul 8, 2026

Copy link
Copy Markdown

Thanks @davidben for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app

bedevere-app Bot commented Jul 8, 2026

Copy link
Copy Markdown

GH-153311 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Jul 8, 2026
@bedevere-app

bedevere-app Bot commented Jul 8, 2026

Copy link
Copy Markdown

GH-153312 is a backport of this pull request to the 3.10 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.10 label Jul 8, 2026
@encukou

encukou commented Jul 8, 2026

Copy link
Copy Markdown
Member

It now fails on macOS on GitHub Actions; I made backports to run the CI.

It's up to the RMs. Not a security issue; it can make an OpenSSL upgrade break working code.
The alternative is of course to skip (or ignore) the relevant tests.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou encukou mentioned this pull request Jul 8, 2026
[3.10] gh-151987: Pass filter_function to TarFile._extract_one() during .extract() (GH-151988) #152613
Merged
@nerijus

nerijus commented Jul 8, 2026

Copy link
Copy Markdown

Should be backported to 3.9 too, as it fails on CentOS 9 after openssl update.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Jul 11, 2026

Copy link
Copy Markdown
Member

Should be backported to 3.9 too, as it fails on CentOS 9 after openssl update.

Sadly, Python 3.9 no longer get bugfixes nor security fixes:
https://devguide.python.org/versions/#supported-versions

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou encukou mentioned this pull request Jul 12, 2026
[3.11] gh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1 (GH-100373) #153311
Merged
pablogsal pushed a commit that referenced this pull request Aug 10, 2026
@miss-islington @davidben @hugovk
[3.10] gh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1 (GH…
129667d
…-100373) (#153312)

gh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1 (GH-100373)

In PEM, we need to parse until error and then suppress `PEM_R_NO_START_LINE`, because PEM allows arbitrary leading and trailing data. DER, however, does not. Parsing until error and suppressing `ASN1_R_HEADER_TOO_LONG` doesn't quite work because that error also covers some cases that should be rejected.

Instead, check `BIO_eof` early and stop the loop that way.
(cherry picked from commit acfe02f)


Automerge-Triggered-By: GH:Yhg1s

Co-authored-by: David Benjamin <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
pablogsal pushed a commit that referenced this pull request Aug 10, 2026
@miss-islington @davidben @encukou
[3.11] gh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1 (GH…
5f13018
…-100373) (#153311)

gh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1 (GH-100373)

In PEM, we need to parse until error and then suppress `PEM_R_NO_START_LINE`, because PEM allows arbitrary leading and trailing data. DER, however, does not. Parsing until error and suppressing `ASN1_R_HEADER_TOO_LONG` doesn't quite work because that error also covers some cases that should be rejected.

Instead, check `BIO_eof` early and stop the loop that way.
(cherry picked from commit acfe02f)


Automerge-Triggered-By: GH:Yhg1s

Co-authored-by: David Benjamin <[email protected]>
Co-authored-by: Petr Viktorin <[email protected]>
icanhasmath pushed a commit to ActiveState/cpython that referenced this pull request Oct 28, 2026
@miss-islington @davidben
@hugovk @icanhasmath
[3.10] pythongh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_AS…
3eb4181
…N1 (pythonGH-100373) (python#153312)

pythongh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1 (pythonGH-100373)

In PEM, we need to parse until error and then suppress `PEM_R_NO_START_LINE`, because PEM allows arbitrary leading and trailing data. DER, however, does not. Parsing until error and suppressing `ASN1_R_HEADER_TOO_LONG` doesn't quite work because that error also covers some cases that should be rejected.

Instead, check `BIO_eof` early and stop the loop that way.
(cherry picked from commit acfe02f)

Automerge-Triggered-By: GH:Yhg1s

Co-authored-by: David Benjamin <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
(cherry picked from commit 129667d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Yhg1s Yhg1s Yhg1s approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@davidben @Yhg1s @miss-islington @h-vetinari @encukou @nerijus @vstinner @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.