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

Explain why #2356 "works". #2358

Merged
merged 1 commit into from
Feb 10, 2024
Merged

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Feb 9, 2024

The explanation was hard won and a bit embarrasing in outcome. The
original behavior was correct afaict and PBS use on a RedHat system with
custom RedHat OpenSSL config keys should fail, obviously with a much
better error message, and prompt OpenSSL configuration evaluation on the
machine. That said, the behavior is released now and Pex stands behind
it.

It may make sense to add a --strict-ssl option or something similar to
restore the old behavior and let the (confusing) error bubble, perhaps
with a pointer to what may be wrong.

This should also serve to close
indygreg/python-build-standalone#207 or at
least give Gregory enough information to decide what to do over in PBS.

Closes the loose ends in #2355.

The explanation was hard won and a bit embarrasing in outcome. The
original behavior was correct afaict and PBS use on a RedHat system with
custom RedHat OpenSSL config keys should fail, obviously with a much
better error message, and prompt OpenSSL configuration evalutaion on the
machine. That said, the behavior is released now and Pex stands behind
it.

It may make sense to add a `--strict-ssl` option or something similar to
restore the old behavior and let the (confusing) error bubble, perhaps
with a pointer to what may be wrong.

This should also serve to close
indygreg/python-build-standalone#207 or at
least give Gregory enough information to decide what to do over in PBS.
@jsirois
Copy link
Member Author

jsirois commented Feb 9, 2024

@mjimlittle and @xlevus, I'd appreciate your eyes on the big comment added in this PR. In short, it explains that your RedHat systems use a RedHat proprietary OpenSSL config option that only works with the patched OpenSSL that ships with the OSes you use. There is not much I can see that PBS Python's can do about this! That said, you should know that PBS Pythons are choking on your SSL Config on those machines and may only be partially configured / ignoring some of your settings.

Copy link
Collaborator

@benjyw benjyw left a comment

Choose a reason for hiding this comment

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

Thanks, that's painful.

@jsirois
Copy link
Member Author

jsirois commented Feb 10, 2024

Thanks, that's painful.

Yeah, a bit. Always fun to learn more gdb though.

I'm not sure what Pants / scie-pants want to do about this, but I'll be adding a warning over in the science PBS provider docs about this potential security hole in PBS ssl. If you'd like to provide security-conscious Pants users the option to see these errors I'm happy to field a feature request / bug report about this silent bad SSL config dropping (i.e. add the --strict-ssl option I mentioned in the OP). I'm even happier to review a PR if any of you want to contribute that.

@jsirois jsirois merged commit e70d5d6 into pex-tool:main Feb 10, 2024
26 checks passed
@jsirois jsirois deleted the pr/2356/follow-up branch February 10, 2024 17:04
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.

3 participants