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

Prevent _PassphraseHelper.raise_if_problem() from eating exceptions #581

Merged
merged 3 commits into from
Jan 24, 2017

Conversation

topnotcher
Copy link
Contributor

When _PassphraseHelper.raise_if_problem() does not raise an exception, it still flushes the OpenSSL error queue, possibly causing subsequent code to raise crypto.Error([]) as in bugs #456, #119.

This changeset modifies _PassphraseHelper.raise_if_problem() to flush the error queue only when it raises an exception, thus leaving the OpenSSL errors untouched when it does not raise an exception.

topnotcher added a commit to topnotcher/pyopenssl that referenced this pull request Nov 30, 2016
topnotcher added a commit to topnotcher/pyopenssl that referenced this pull request Nov 30, 2016
topnotcher added a commit to topnotcher/pyopenssl that referenced this pull request Nov 30, 2016
@reaperhulk
Copy link
Member

I have no idea why that test is failing...maybe rebase/merge forward just to make it refresh the environment?

@codecov-io
Copy link

codecov-io commented Jan 24, 2017

Current coverage is 95.69% (diff: 100%)

Merging #581 into master will increase coverage by 2.34%

@@             master       #581   diff @@
==========================================
  Files            16         16          
  Lines          5761       5760     -1   
  Methods           0          0          
  Messages          0          0          
  Branches        405        404     -1   
==========================================
+ Hits           5378       5512   +134   
+ Misses          305        174   -131   
+ Partials         78         74     -4   

Powered by Codecov. Last update 7be83a5...6af209a

topnotcher added a commit to topnotcher/pyopenssl that referenced this pull request Jan 24, 2017
@topnotcher
Copy link
Contributor Author

Yeah... looks like a timing issue. I rebased on current master.

This resolves pyca#119, resolves pyca#456.

`_PassphraseHelper.raise_if_problem()` always flushes the OpenSSL
exception queue, but does not always raise an exception. In some cases,
other code attempts to raise an error from OpenSSL after
`raise_if_problem()` has flushed the queue, thus causing an empty
exception to be raised (i.e. `raise Error([])`).

This commit modifies `_PassphraseHelper.raise_if_problem` to flush the
OpenSSL error queue only if it has en exception to raise. Subsequent
code that detects an error should now be able to raise an non-empty
exception.
@reaperhulk reaperhulk merged commit 36eb2de into pyca:master Jan 24, 2017
aszlig added a commit to NixOS/nixpkgs that referenced this pull request Jun 21, 2017
Upstream changes:

 * Added OpenSSL.X509Store.set_time() to set a custom verification time
   when verifying certificate chains. pyca/pyopenssl#567
 * Added a collection of functions for working with OCSP stapling. None
   of these functions make it possible to validate OCSP assertions, only
   to staple them into the handshake and to retrieve the stapled
   assertion if provided. Users will need to write their own code to
   handle OCSP assertions. We specifically added:
   Context.set_ocsp_server_callback, Context.set_ocsp_client_callback,
   and Connection.request_ocsp. pyca/pyopenssl#580
 * Changed the SSL module's memory allocation policy to avoid zeroing
   memory it allocates when unnecessary. This reduces CPU usage and
   memory allocation time by an amount proportional to the size of the
   allocation. For applications that process a lot of TLS data or that
   use very lage allocations this can provide considerable performance
   improvements. pyca/pyopenssl#578
 * Automatically set SSL_CTX_set_ecdh_auto() on OpenSSL.SSL.Context.
   pyca/pyopenssl#575
 * Fix empty exceptions from OpenSSL.crypto.load_privatekey().
   pyca/pyopenssl#581

The full upstream changelog can be found at:

https://pyopenssl.readthedocs.io/en/17.0.0/changelog.html

I've also added a patch from pyca/pyopenssl#637 in order to fix the
tests, which was the main reason for the version bump because that patch
won't apply for 16.2.0.

According to the upstream changelog there should be no
backwards-incompatible changes, but I've tested building against some of
the packages depending on pyopenssl anyway. Regardless of this, the
build for pyopenssl fails right now anyway, so the worst that could
happen via this commit would be that we break something that's already
broken.

Signed-off-by: aszlig <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants