Cryptography: Support OAEP labels/alternate digests in 1.0.2

[reaperhulk]

As of 1.0.2 OpenSSL supports:

• RSA OAEP labeling (EVP_PKEY_CTX_set0_rsa_oaep_label and EVP_PKEY_CTX_get0_rsa_oaep_label)
• alternate MGF1 digests when using OAEP (EVP_PKEY_CTX_get_rsa_mgf1_md and EVP_PKEY_CTX_set_rsa_mgf1_md)
• alternate OAEP digests (EVP_PKEY_CTX_set_rsa_oaep_md and EVP_PKEY_CTX_get_rsa_oaep_md)

These macros are all exported in rsa.h.

Supporting this will require installing 1.0.2 on a travis builder to monitor coverage (or else finally solving our coverage woes).

[alex]

On OS X homebrew has 1.0.2

[reaperhulk]

Progress on this depends on finding/creating vectors and also adding new backend interfaces to query for support during tests. As this is a low priority item I'm going to remove the ninth release target for this and we can get to it later.

[collisdigital]

@reaperhulk Can we get a quick bit of advice on this, our Information Assurance team are telling us that we fundamentally can't use SHA-1 in _any_ algorithm, including RSA-OAEP because of the known weaknesses in SHA-1 and instead we must use SHA-2 family (in this instance SHA-256) with RSA-OAEP. Are there any plans to add this support any time soon, I was under the impression that using SHA-1 within RSA-OAEP doesn't suffer from the same weaknesses but I'm no expert?

[reaperhulk]

Here's a paper about what properties a hash function needs to be secure within OAEP (https://eprint.iacr.org/2006/223.pdf). (SHA1 is fine for now)

I'm happy to add this as soon as we find a source for vectors to confirm spec conformance. Looking around a bit it looks like we may need to generate vectors using OpenSSL and another implementation (probably golang) to confirm them.

[collisdigital]

Thanks for the response, we'll also see if we can find some test vectors to support this work.

[Badg]

I'm also blocking on this, so I would be happy to help generate some test vectors using pycryptodome, which does not wrap openssl. However, that raises (for me) two questions:

• Is pyCA comfortable with that as a source for test vectors? It's also unaudited, and this would hang primarily on:
  
  
  
  1. Me correctly understanding pycryptodome's API (not entirely trivial, but I've read through a bunch of the relevant source code, so I think I have a good handle on it)
  
  
  2. pycryptodome correctly implementing
  
  
• What format would pyCA like the test vectors? For example, should MGF1 have its own independent vectors for unit tests, or would combinations (as in full OAEP signatures using given combinations of MGF1/OAEP hash algorithms) be sufficient? It would be easier for me to do the latter.

Since we're both blocking on it, maybe @collisdigital could generate vectors with openSSL and I could confirm them? Thoughts, @reaperhulk?

[reaperhulk]

You're both in luck. This was mildly interesting so I wrote a script that can generate these vectors (in a form loadable by the load_pkcs1_vectors function) for all the permutations of SHA224,256,384,512 for both mgf1 and primary hash alg. The next step is writing some code to validate them against another implementation. I'd prefer that to be golang because we've used that in the past for other vector validations and the ability to compile and validate the vectors without significant dependencies is useful. I'll put up a WIP PR shortly with the vector generator and the vectors themselves.

[collisdigital]

Wow, that's pretty amazing and a massive 👍 from us, thank you! We should be able to test this against our system which is interfacing with a Java service so that would give a level of validation that it works in a real world scenario against a different implementation too. Is #2829 usable for such a test, we could try it?

[reaperhulk]

@collisdigital I've removed the code that prevents SHA2 from being allowed by RSA OAEP in that PR (temporarily -- it doesn't belong in that PR really) so you can use it to validate that it works if you want. Be sure you're compiled against 1.0.2+ though of course!

[reaperhulk]

It looks like golang may not allow independent combination of MGF1 hash function and OAEP hash function. Instead it appears to only let you pass one hash and it uses it for both. One possible solution would be to restrict cryptography in the same way for now and revisit this if and when we run into a use case that requires them to be mismatched. Alternately, we may need to validate using something else (perhaps crypto++ or botan's python bindings or some Java?).

[warrenbailey]

I'm no crypto expert, but I tried the your change against a Java implementation (using JDK8) and the decryption failed when using SHA256. It works perfectly with SHA1, I can encrypt with Java and decrypt with Python no problem.

However when I move to the SHA256 hash function, then the decryption fails in Python with the following:

Traceback (most recent call last):
  File "rsa_test.py", line 48, in <module>
    decoder.decrypt_helloworld(encrypted, sha)
  File "rsa_test.py", line 34, in decrypt_helloworld
    decrypted = self.sr_private_key.decrypt(decoded_text, padding.OAEP(mgf=padding.MGF1(algorithm=hashes_sha), algorithm=hashes_sha, label=None))
  File "/Users/warrenbailey/Env/survey-runner/lib/python3.5/site-packages/cryptography/hazmat/backends/openssl/rsa.py", line 534, in decrypt
    return _enc_dec_rsa(self._backend, self, ciphertext, padding)
  File "/Users/warrenbailey/Env/survey-runner/lib/python3.5/site-packages/cryptography/hazmat/backends/openssl/rsa.py", line 76, in _enc_dec_rsa
    return _enc_dec_rsa_pkey_ctx(backend, key, data, padding_enum)
  File "/Users/warrenbailey/Env/survey-runner/lib/python3.5/site-packages/cryptography/hazmat/backends/openssl/rsa.py", line 105, in _enc_dec_rsa_pkey_ctx
    _handle_rsa_enc_dec_error(backend, key)
  File "/Users/warrenbailey/Env/survey-runner/lib/python3.5/site-packages/cryptography/hazmat/backends/openssl/rsa.py", line 147, in _handle_rsa_enc_dec_error
    raise ValueError("Decryption failed.")
ValueError: Decryption failed.

There's a chance I have something wrong in the implementation of either the python code or the Java code, but I thought I'd share it with you in case it helps.

The Java code I'm using to encrypted the string "Helloworld"

import java.io.DataInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.security.AlgorithmParameters;
import java.security.KeyFactory;
import java.security.NoSuchAlgorithmException;
import java.security.PublicKey;
import java.security.spec.AlgorithmParameterSpec;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.MGF1ParameterSpec;
import java.security.spec.X509EncodedKeySpec;
import java.util.Base64;

import javax.crypto.Cipher;
import javax.crypto.NoSuchPaddingException;
import javax.crypto.spec.OAEPParameterSpec;
import javax.crypto.spec.PSource;

public class RSATest {

    public enum Padding {
        SHA1,
        SHA256
    }

    public static void main(String[] args) {
        try {

            Padding padding = Padding.SHA256;
            String test = new String("Helloworld");
            String key = new String("./src/main/resources/public.der");

            AlgorithmParameters algp = AlgorithmParameters.getInstance("OAEP");
            AlgorithmParameterSpec paramSpec = getAlgorithmParameterSpec(padding);
            algp.init(paramSpec);
            Cipher cipher = getCipherInstance(padding);
            cipher.init(Cipher.ENCRYPT_MODE, loadPublicKey(key), algp);
            byte[] result = cipher.doFinal(test.getBytes());
            String encrypted = Base64.getEncoder().encodeToString(result);
            System.out.println(encrypted);

        } catch (Exception e) {
            e.printStackTrace();
        }
    }

    private static AlgorithmParameterSpec getAlgorithmParameterSpec(Padding sha) {
        if (Padding.SHA1 == sha) {
            return  new OAEPParameterSpec("SHA-1", "MGF1", MGF1ParameterSpec.SHA1, PSource.PSpecified.DEFAULT);
        } else {
            return  new OAEPParameterSpec("SHA-256", "MGF1", MGF1ParameterSpec.SHA256, PSource.PSpecified.DEFAULT);
        }
    }

    public static Cipher getCipherInstance(Padding sha) throws NoSuchAlgorithmException, NoSuchPaddingException {
        if (Padding.SHA1 == sha) {
            return Cipher.getInstance("RSA/ECB/OAEPWithSHA-1AndMGF1Padding");
        } else {
            return Cipher.getInstance("RSA/ECB/OAEPWithSHA-256AndMGF1Padding");
        }
    }

    private static PublicKey loadPublicKey(String keyLocation) {
        try {
            File f = new File(keyLocation);
            FileInputStream fis = new FileInputStream(f);
            DataInputStream dis = new DataInputStream(fis);
            byte[] keyBytes = new byte[(int) f.length()];
            dis.readFully(keyBytes);
            dis.close();

            X509EncodedKeySpec spec = new X509EncodedKeySpec(keyBytes);
            KeyFactory kf = KeyFactory.getInstance("RSA");
            return kf.generatePublic(spec);
        } catch (IOException | InvalidKeySpecException | NoSuchAlgorithmException e) {
            e.printStackTrace();
            return null;
        }
    }
}

And the python code I'm using to decrypt

from cryptography.hazmat.backends.openssl.backend import backend
from cryptography.hazmat.primitives import serialization
from cryptography.hazmat.primitives import hashes
from cryptography.hazmat.primitives.asymmetric import padding

import base64


PRIVATE_KEY = "./test-keys/private.pem"

class Decoder (object):
    def __init__(self):
        sr_private_key_as_bytes = self.__to_bytes(self.get_key(PRIVATE_KEY))
        self.sr_private_key = serialization.load_pem_private_key(
            sr_private_key_as_bytes,
            password="digitaleq".encode(),
            backend=backend
        )

    def get_key(self, key_name):
        key = open(key_name, 'r')
        contents = key.read()
        return contents

    def __to_bytes(self, bytes_or_str):
        if isinstance(bytes_or_str, str):
            value = bytes_or_str.encode()
        else:
            value = bytes_or_str
        return value

    def decrypt_helloworld(self, encrypted_text, hashes_sha):
        decoded_text = self._base64_decode(encrypted_text)
        decrypted = self.sr_private_key.decrypt(decoded_text, padding.OAEP(mgf=padding.MGF1(algorithm=hashes_sha), algorithm=hashes_sha, label=None))
        print(decrypted)

    @staticmethod
    def _base64_decode(text):
        if len(text) % 4 != 0:
            while len(text) % 4 != 0:
                text += "="
        return base64.urlsafe_b64decode(text)

if __name__ == '__main__':
        encrypted = "encrypted_token_goes_herer"
        sha = hashes.SHA256()
        decoder = Decoder()
        decoder.decrypt_helloworld(encrypted, sha)

[reaperhulk]

I'm seeing the same behavior in some go test code, so it's quite possible the generated vectors are invalid in some fashion. I haven't had a chance to look into it much yet, but it's good to know more than one person is seeing this.

[natedub]

I can't really speak to whether the test vectors are correct but I have worked a little with Python/Java crypto interop; it turns out JDK crypto has some unintuitive behaviour when it comes to OAEP padding.

Cipher.getInstance("RSA/NONE/OAEPWithSHA256AndMGF1Padding");

is equivalent to

ciphertext = public_key.encrypt(
    message,
    padding.OAEP(
        mgf=padding.MGF1(algorithm=hashes.SHA1()),
        algorithm=hashes.SHA256(),
        label=None
    )
)

When you use BouncyCastle crypto, the behaviour changes. Note the added "BC" argument below.

Cipher.getInstance("RSA/NONE/OAEPWithSHA256AndMGF1Padding", "BC");

is equivalent to

ciphertext = public_key.encrypt(
    message,
    padding.OAEP(
        mgf=padding.MGF1(algorithm=hashes.SHA256()),
        algorithm=hashes.SHA256(),
        label=None
    )
)

Hope that helps, looking forward to this functionality being added!

[collisdigital]

@natedub ah, could be the problem! We can test this tomorrow @warrenbailey. Bit of googling what you wrote above confirms the confusion. Interesting the point previously above about supporting different mgf1 vs. hash algorithm usage at the same time, it seems then that Java does this even if it isn't intentional!

[warrenbailey]

I tried using the same code with the bouncy castle provider rather than the JDK one and unfortunately I get the same issue.

I've also tried using the two sha algorithms in the python code with the Java 8 JDK provider but that didn't work either:

decrypted = self.sr_private_key.decrypt(decoded_text, padding.OAEP(mgf=padding.MGF1(algorithm=hashes.SHA1()), algorithm=hashes.SHA256(), label=None))

I notice you've got "RSA/NONE/OAEPWithSHA256AndMGF1Padding" rather than "RSA/ECB/OAEPWithSHA-256AndMGF1Padding". Any reason for that?

[warrenbailey]

I tried RSA/NONE/OAEPWithSHA-256AndMGF1Padding with BouncyCastle and that doesn't work either.

[Badg]

I did this pretty hastily so it's possible I may have misinterpreted the vectors files. However: I played around some with the vectors using aforementioned pycryptodome. I only looked at message1/CT1 and message2/CT2 with sha256/sha256. At least for them, it looks like the actual algo being used is SHA1/SHA1:

>>> import Crypto
>>> from Crypto.Cipher import PKCS1_OAEP as OAEP
>>> from Crypto.PublicKey import RSA
>>> from Crypto.Hash import SHA256
>>> from Crypto.Hash import SHA1
>>> from Crypto.Signature.pss import MGF1
>>> key = RSA.construct((0xa8b3b284af8eb50b387034a860f146c4919f318763cd6c5598c8ae4811a1e0abc4c7e0b082d693a5e7fced675cf4668512772c0cbc64a742c6c630f533c8cc72f62ae833c40bf25842e984bb78bdbf97c0107d55bdb662f5c4e0fab9845cb5148ef7392dd3aaff93ae1e6b667bb3d4247616d4f5ba10d4cfd226de88d39f16fb, 0x10001, 0x53339cfdb79fc8466a655c7316aca85c55fd8f6dd898fdaf119517ef4f52e8fd8e258df93fee180fa0e4ab29693cd83b152a553d4ac4d1812b8b9fa5af0e7f55fe7304df41570926f3311f15c4d65a732c483116ee3d3d2d0af3549ad9bf7cbfb78ad884f84d5beb04724dc7369b31def37d0cf539e9cfcdd3de653729ead5d1))
>>> message1 = 0x6628194e12073db03ba94cda9ef9532397d50dba79b987004afefe34
>>> message1 = int.to_bytes(message1, length=28, byteorder='big')
>>> ct1 = 0x28d94f5b6b6b9c53d1869251dfebb80d01a16ed1a8e782ccc2c0bb546466b7537ab2004fd3a8a7e411372ea4904700a20bf85937f2631dcffd30acae2d08ea7dae8e38c16ad89e7bb00fa44282813aaf894bec636efb79bd5be93df53240d20acae0cf61fea899e889f7ea78bfd6a63dbbf12945e4fb296e76fcc04c73ba230c
>>> ct1 = int.to_bytes(ct1, length=128, byteorder='big')
>>> message2 = 0x750c4047f547e8e41411856523298ac9bae245efaf1397fbe56f9dd5
>>> message2 = int.to_bytes(message2, length=28, byteorder='big')
>>> ct2 = 0x52927673760f9cc5b681b8c93f63c19940f12b26e16b9c0856857dbf4b805aa57d9e6be66183d760d1f7e3ef9b4f9543814259ea1466573ca203ccb369aa7f58f6ac9a153a4be94546e0ceec525c3dc72686f58dc15ceb2d6902f5f3cea9a8f0cc223fbbd872c27c27cb5b690eaef4eff543a000f87f20524af11bb42637e274
>>> ct2 = int.to_bytes(ct2, length=128, byteorder='big')
>>> mgf_256 = lambda x, y: MGF1(x, y, SHA256)
>>> cipher1 = OAEP.new(key, hashAlgo=SHA256, mgfunc=mgf_256)
>>> cipher1.decrypt(ct1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python34\lib\site-packages\Crypto\Cipher\PKCS1_OAEP.py", line 227, in decrypt
    raise ValueError("Incorrect decryption.")
ValueError: Incorrect decryption.
>>> mgf_1 = lambda x, y: MGF1(x, y, SHA1)
>>> cipher2 = OAEP.new(key, hashAlgo=SHA1, mgfunc=mgfsha1, label=b'')
>>> cipher2.decrypt(ct1)
b'f(\x19N\x12\x07=\xb0;\xa9L\xda\x9e\xf9S#\x97\xd5\r\xbay\xb9\x87\x00J\xfe\xfe4'
>>> cipher2.decrypt(ct1) == message1
True
>>> cipher2.decrypt(ct2) == message2
True

[warrenbailey]

That's interesting, and ties with what I'm seeing. If I encrypt with SHA256 using my Java code, I then can decrypt with SHA1 in my Python code.

[collisdigital]

Something I spotted on this: https://tools.ietf.org/html/rfc3447 page 49:

PKCS1MGFAlgorithms (see Appendix A.2.1). The default mask
generation function is MGF1 with SHA-1. For MGF1 (and more
generally, for other mask generation functions based on a hash
function), it is recommended that the underlying hash function be
the same as the one identified by hashAlgorithm; see Note 2 in
Section 9.1 for further comments.

So the standard recommends the same hash functions are used, although doesn't 'prevent' different ones being allowed. Given the Java quirks listed above between Bouncy Castle and the normal libs it might be useful to support different combinations to allow compatibility with other implementations, then again.... ?

[collisdigital]

Hey @reaperhulk; I did some research and found this mostly unrelated SE question that contains an example of how to use SHA-256 with RSA-OAEP in OpenSSL (http://stackoverflow.com/questions/23143485/c-library-for-cms-x-509-manipulation); the magic incantation appears to be:

EVP_PKEY_CTX_set_rsa_padding(wrap_ctx, RSA_PKCS1_OAEP_PADDING);
EVP_PKEY_CTX_set_rsa_oaep_md(wrap_ctx, EVP_sha256());
EVP_PKEY_CTX_set_rsa_mgf1_md(wrap_ctx, EVP_sha256());
EVP_PKEY_CTX_set0_rsa_oaep_label(wrap_ctx, oaep_label, oaep_label_l);
/* NB: oaep_label must be heap-allocated, and will be freed by OSSL */

Where EVP_sha256() can be replaced with the desired hash algo.

A quick look at https://github.com/pyca/cryptography/blob/master/src/cryptography/hazmat/backends/openssl/rsa.py doesn't show this or similar code being called currently (I guess it isn't?); the padding object passed to _enc_dec_rsa which specifies the hash algorithms is not used after being checked (the checks which you commented out in #2829) and only padding_enum is used which is set to backend._lib.RSA_PKCS1_OAEP_PADDING. The same goes for the label value.

I'm assuming that these extra steps are needed to map the padding object to the OpenSSL calls to set the hash (and label if desired), which would explain why whatever you set the hash to it is always using SHA-1 today regardless.

[reaperhulk]

My apologies for not getting back to this yet -- you are correct about the functions required. I have a branch that adds them as conditional bindings but haven't submitted the PR yet. I'll try to get to that today.

[collisdigital]

No problem! Thanks for the update 👍

[reaperhulk]

I've updated the vector PR with vectors that are much more likely to be correct. I've done a bit of preliminary verification, but nothing comprehensive yet. That PR also contains (for now) the changes required to use alternate MGF1 MD and OAEP MD.

[reaperhulk]

(Note that those changes do not check whether your openssl is capable of doing this, so it will blow up if you try to use it on < 1.0.2)

[collisdigital]

Is the main blocker on this getting some independent test vectors to verify the implementation? If we produced a set using Java bouncy castle would that help?

[reaperhulk]

We need another implementation (including code that we can run to verify) before we can land. If you're willing to write some Java that loads and verifies the vectors present in that PR and contribute it that would give us what we need. A colleague wrote some Go code that can do some of the verification, but Go doesn't allow independent setting of MGF1 MD and OAEP MD, which limits its utility for this purpose.

https://cryptography.io/en/latest/development/custom-vectors/arc4/ is an example of how we document the creation/verification of these vectors.

[collisdigital]

Ok, I've put something together to verify the vectors using Java and Bouncy Castle. I've got the format of vectors from https://github.com/pyca/cryptography/pull/2829 loading correctly and tried these vectors/hash combinations initially to prove the code:

• oaep-sha224-sha224.txt
• oaep-sha256-sha512.txt
• oaep-sha512-sha512.txt
• oaep-sha256-sha256.txt
• oaep-sha512-sha256.txt

They all decrypt successfully! Setting the wrong hashes correctly causes the decryption to fail with a "javax.crypto.BadPaddingException: data hash wrong". So far so good. I'll automate the file/hash combination loading next and try the other combos and write up some instructions on how to compile/run it.

I've pushed it here (https://github.com/collisdigital/cryptography/tree/oaep-sha2-vectors) while it's being worked on; I'll update this issue as I progress.

[collisdigital]

Ok I've raised a pull request here: https://github.com/reaperhulk/cryptography/pull/4 which is into your fork that the work is happening on, all vectors you generated verified successfully. If you can fix the OpenSSL version problem you mentioned (i.e. bindings not supported on older version) it feels like this is now close to ready...

[collisdigital]

I was thinking since the test vectors and Java/pyca code only verify SHA2 (224, 256, 384, 512) the supported hashes should be white listed like in the current implementation but extended to include the new algos? Do you have another PR with the actual changes to pyca now they've been removed from the #2829 ?

[collisdigital]

I'm happy to work on this one if you have a branch with the basic changes in still somewhere that I can contribute to?

[reaperhulk]

@collisdigital https://github.com/reaperhulk/cryptography/tree/oaep-sha2-support (only truly missing bit is that it doesn't restrict to sha1/sha2, which it should)

[reaperhulk]

Oh and it needs an encrypt/decrypt round trip test. Right now it only tests the decrypt vectors.

[collisdigital]

Do you mean generate some encrypted vectors from Java to decrypt in pyca?

[reaperhulk]

No I mean a unit test in Python that encrypts and then decrypts the encrypted data to confirm it round trips.

[collisdigital]

Got it, will take a look at what I can do.
On Fri, 22 Apr 2016 at 21:25, Paul Kehrer [email protected] wrote:

No I mean a unit test in Python that encrypts and then decrypts the
encrypted data to confirm it round trips.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/pyca/cryptography/issues/1663#issuecomment-213577015

[collisdigital]

Ok, SHA reatrictions in place, encrypt decrypt round trip next, do we need all combinations of hash round tripped? Is there a standard approach to this kind of test like you had for the vectors?

[collisdigital]

@reaperhulk I'll look at adding the round trip tests now, will follow what's already existing in tests unless there's anything specific to do...

[reaperhulk]

That's the last thing we need before that branch should be ready for a PR, yes. Although we do also need #2829 to land in order for it to merge.

[collisdigital]

Raised PR for round trip tests on @reaperhulk repo.

[alex]

Alternate digests are now supported.

[collisdigital]

👍🍾

[reaperhulk]

I'm actually going to go ahead and close this for now. I don't believe supporting OAEP labels is something we should do without an explicit use case. When/if someone finds one they can open a new issue.