Attempt side-channel free RSA decryption #438
Conversation
with M2Crypto we can process byte strings, and for padding/depadding we need bytes, so don't convert back and forth between the formats also put all the integer to bytes conversion in a single place for the implementations that can't handle bytes as input
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
|
This pull request fixes 1 alert when merging 982cfe2 into 0812ed6 - view on LGTM.com fixed alerts:
|
tomato42
force-pushed
the
bleichenbacher-fixes
branch
2 times, most recently
from
79b609a to
77e8b06
Compare
|
This pull request fixes 1 alert when merging 77e8b06 into 0812ed6 - view on LGTM.com fixed alerts:
|
tomato42
force-pushed
the
bleichenbacher-fixes
branch
from
77e8b06 to
477d681
Compare
| @@ -33,9 +33,39 @@ def parsePEM(s, passwordCallback=None): | |||
| elif pemSniff(s, "EC PRIVATE KEY"): | |||
| bytes = dePem(s, "EC PRIVATE KEY") | |||
| return Python_Key._parse_ecc_ssleay(bytes) | |||
| elif pemSniff(s, "PUBLIC KEY"): | |||
| bytes = dePem(s, "PUBLIC KEY") | |||
| return Python_Key._parse_public_key(bytes) | |||
There was a problem hiding this comment.
Avoid too many return statements within this function.
|
This pull request fixes 1 alert when merging 477d681 into 0812ed6 - view on LGTM.com fixed alerts:
|
| if cryptomath.m2cryptoLoaded: | ||
| if type(key) == Python_RSAKey: | ||
| return _createPrivateKey(key) | ||
| assert type(key) in (OpenSSL_RSAKey, Python_ECDSAKey), type(key) |
There was a problem hiding this comment.
I don't understand the syntax. Why is type(key) at the end of the line?
There was a problem hiding this comment.
assert will print the second element in the tuple on error, it's so that if the assert fails, it will print result of type(key)
tlslite/utils/rsakey.py
Outdated
| raise ValueError("only multiples of 8 supported as output size") | ||
|
|
||
| iterator = 0 | ||
| while len(out) <= out_len // 8: |
There was a problem hiding this comment.
yes, it would be, will fix
| @@ -67,13 +68,21 @@ def _rawPrivateKeyOp(self, message): | |||
| ciphertext = bytesToNumber(bytearray(string)) | |||
| return ciphertext | |||
|
|
|||
| def _raw_private_key_op_bytes(self, message): | |||
| return bytearray(m2.rsa_private_encrypt(self.rsa, bytes(message), | |||
There was a problem hiding this comment.
What about using _rawPrivateKeyOp instead of m2 internally? It would be nice if the pattern is the same as in tlslite/utils/rsakey.py. (same for public key operations)
There was a problem hiding this comment.
- this is the class for wrapping m2crypto specifically
- we do direct call to m2crypto here instead of reusing the code from rsakey because we don't want to do the
int()->bytes()conversion in python, as it can't be done in side-channel safe manner
tomato42
force-pushed
the
bleichenbacher-fixes
branch
from
477d681 to
44d6fd4
Compare
|
This pull request fixes 1 alert when merging 44d6fd4 into 0812ed6 - view on LGTM.com fixed alerts:
|
currently we leak severely where the issue is in the padding check of RSA decryption, so attempt depadding in side-channel free way (at least to the degree that Python allows)
This change is