Add support for RSA_R_OAEP_DECODING_ERROR error flag.

[EnTeQuAk]

This pull request adds support for the RSA_R_OAEP_DECODING_ERROR error flag referenced here: https://github.com/openssl/openssl/blob/master/include/openssl/rsa.h#L626

This error code is being raised on error condition (e.g wrong private key) for this code: https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa/#decryption

I actually didn't see any tests for RSA_R_PKCS_DECODING_ERROR flag and don't really know how to test such flags. Please let me know if you need any, I'd be happy for any guidance here 😃

[reaperhulk]

@EnTeQuAk thanks for the PR! In this case I think a good test is whatever you used to produce the problem.

[codecov-io]

Current coverage is 99.97%

Merging #2323 into master will decrease coverage by -0.01% as of 0976585

@@            master   #2323   diff @@
======================================
  Files          115     115       
  Stmts        11950   11957     +7
  Branches      1334    1335     +1
  Methods          0       0       
======================================
+ Hit          11948   11954     +6
- Partial          2       3     +1
  Missed           0       0       

Review entire Coverage Diff as of 0976585

Powered by Codecov. Updated on successful CI builds.

[EnTeQuAk]

@reaperhulk added simple test that would fail on decryption without the support of RSA_R_PKCS_DECODING_ERROR

[reaperhulk]

Your patch appears to be designed to have it properly raise a ValueError on a decryption failure rather than an AssertionError, correct? The test you put up passes without any issues on current trunk without your proposed patch.

[EnTeQuAk]

@reaperhulk that's weird, yeah, if decryption fails (which it does not in the test) a ValueError is expected instead of an AssertionError. Let me check why it's not failing without the patch, I used different rsa-keys here for testing but thought it'd be all the same.

I'll re-check

[reaperhulk]

@EnTeQuAk your test doesn't expect a failure at all (it just encrypts/decrypts and asserts that decryption was successful), so it isn't going to exercise the failure case (which would require a pytest.raises(ValueError)). Did you copy the wrong test?

[EnTeQuAk]

Ah no, it's not expecting a failure. The patch fixes a wrongly raised AssertionError.

[reaperhulk]

I am still a bit confused I think. The patch adds a new error code to the list of expected codes when _handle_rsa_enc_dec_error is called. This is only called when a decryption/encryption has failed though, so the test case you've provided can't exercise that code path.

[EnTeQuAk]

Thanks, I've been blind. Updated the test 😄 Now this fails on master.

[reaperhulk]

Thanks to a new GH feature you'll need to merge the latest master into this to be fully up to date before we can merge as well (sorry!)

[alex]

Can you please move the RSA_KEY_512_ALT.private_key(backend) call outside of the with pytest.raises(ValueError) block.

[EnTeQuAk]

Done.

[reaperhulk]

Also this new test passes for me on master without the patch as well, which suggests this issue is specific to a particular OpenSSL. What OS/distro/OpenSSL version are you seeing this on?

[EnTeQuAk]

@reaperhulk I'm using OpenSSL 1.0.2d 9 Jul 2015 on ArchLinux

[reaperhulk]

Sigh, stupid OpenSSL. I'm using 1.0.2d for my tests as well, but can't replicate. Figures.

[reaperhulk]

jenkins, ok to test

[EnTeQuAk]

Not cool. I'll try to recheck some time tomorrow (CET) what might be going on here. 😞

[alex]

This line is too long, you'll need to format it:

from ..primitives.fixtures_rsa import (
    RSA_KEY_2048, RSA_KEY_512, RSA_KEY_512_ALT
)

[reaperhulk]

@EnTeQuAk have you had a chance to revisit this or would you like us to take it over? It looks like there's just one small PEP8 problem.

[EnTeQuAk]

@reaperhulk Hey, sorry, I did not have the time recently to figure it out. I'll fix the pep8 problem shortly, I'm just curious about the fact that the test works for you on master without the patch but it doesn't for me. I need to test this on various machines. In any case, I'm still on it.