Re: Add RSA-OAEP encryption/decryption to Nettle
Hello again, After rethinking it, I refactored the new functions to be consistent with how Nettle functions are designed, now there's one encrypt/decrypt function per hash. https://git.lysator.liu.se/nettle/nettle/-/merge_requests/20 The new functions are the following: Encryption: int rsa_oaep_sha1_encrypt(const struct rsa_public_key *key, void *random_ctx, nettle_random_func *random, size_t label_length, const uint8_t *label, size_t length, const uint8_t *message, mpz_t gibberish); int rsa_oaep_sha256_encrypt(const struct rsa_public_key *key, void *random_ctx, nettle_random_func *random, size_t label_length, const uint8_t *label, size_t length, const uint8_t *message, mpz_t gibberish); int rsa_oaep_sha384_encrypt(const struct rsa_public_key *key, void *random_ctx, nettle_random_func *random, size_t label_length, const uint8_t *label, size_t length, const uint8_t *message, mpz_t gibberish); int rsa_oaep_sha512_encrypt(const struct rsa_public_key *key, void *random_ctx, nettle_random_func *random, size_t label_length, const uint8_t *label, size_t length, const uint8_t *message, mpz_t gibberish); Decryption: int rsa_oaep_sha1_decrypt(const struct rsa_private_key *key, size_t label_length, const uint8_t *label, size_t *length, uint8_t *message, const mpz_t gibberish); int rsa_oaep_sha256_decrypt(const struct rsa_private_key *key, size_t label_length, const uint8_t *label, size_t *length, uint8_t *message, const mpz_t gibberish); int rsa_oaep_sha384_decrypt(const struct rsa_private_key *key, size_t label_length, const uint8_t *label, size_t *length, uint8_t *message, const mpz_t gibberish); int rsa_oaep_sha512_decrypt(const struct rsa_private_key *key, size_t label_length, const uint8_t *label, size_t *length, uint8_t *message, const mpz_t gibberish); The tests are updated too /Nicolas OpenPGP_signature Description: OpenPGP digital signature ___ nettle-bugs mailing list nettle-bugs@lists.lysator.liu.se http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
Re: Add RSA-OAEP encryption/decryption to Nettle
Hello Nicolas, Niels, Now that another attack on RSA encryption with PKCS#1 v1.5 padding has been discovered (though Nettle is not vulnerable)[1], it is recommended to avoid using the v1.5 scheme in new applications[2][3], and thus supporting RSA-OAEP in Nettle is becoming more relevant. I made some modifications to the existing merge request[4], mainly to make it side-channel safe at decryption: https://git.lysator.liu.se/nettle/nettle/-/merge_requests/60 Could you take a look when you have time? Footnotes: [1] https://people.redhat.com/~hkario/marvin/ [2] https://datatracker.ietf.org/doc/draft-kario-rsa-guidance/ [3] https://github.com/checkpoint-restore/criu/pull/2297#discussion_r1420116692 [4] https://git.lysator.liu.se/nettle/nettle/-/merge_requests/20 Regards, -- Daiki Ueno ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Daiki Ueno writes: > Now that another attack on RSA encryption with PKCS#1 v1.5 padding has > been discovered (though Nettle is not vulnerable)[1], it is recommended > to avoid using the v1.5 scheme in new applications[2][3], and thus > supporting RSA-OAEP in Nettle is becoming more relevant. I agree oaep support is desirable. > I made some modifications to the existing merge request[4], mainly to > make it side-channel safe at decryption: > https://git.lysator.liu.se/nettle/nettle/-/merge_requests/60 Thanks for reviving this issue, and looking into side-channel silence. > Could you take a look when you have time? Thanks, I've had a look, and it looks pretty good to me. Some comments and questions: * For tests, would it make some with some test that check that encryption with a given message and randomness gives the expected output? Even better if there are any authoritative testcases for that? * Is it useful to have oaep_decode_mgf1 and oaep_encode_mgf1 advertised as public functions, or would it be better to make them internal? * Do you see any reasonable (i.e., with a net gain in maintainability) way to share more code between _oaep_sec_decrypt_variable and _pkcs1_sec_decrypt_variable? * For oaep_decode_mgf1, oaep_encode_mgf1, maybe one could let the caller allocate and pass in the appropriate hashing context? Would be easy to do, e.g., in rsa_oaep_sha512_decrypt. But it looks like that would be inconsistent with pss_mgf1, though (which looks like it needs a separate hashing context). * I think it was a design mistake to represent RSA ciphertexts as mpz_t rather then octet strings in Nettle's original RSA interfaces. I wonder if it would make sense to let the new functions take octet strings instead? Regards, /Niels -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
On Mon, 2024-01-15 at 21:43 +0100, Niels Möller wrote: > Daiki Ueno writes: > > > Now that another attack on RSA encryption with PKCS#1 v1.5 padding has > > been discovered (though Nettle is not vulnerable)[1], it is recommended > > to avoid using the v1.5 scheme in new applications[2][3], and thus > > supporting RSA-OAEP in Nettle is becoming more relevant. > > I agree oaep support is desirable. > > > I made some modifications to the existing merge request[4], mainly to > > make it side-channel safe at decryption: > > https://git.lysator.liu.se/nettle/nettle/-/merge_requests/60 > > Thanks for reviving this issue, and looking into side-channel silence. > > > Could you take a look when you have time? > > Thanks, I've had a look, and it looks pretty good to me. Some comments > and questions: > > * For tests, would it make some with some test that check that > encryption with a given message and randomness gives the expected > output? Even better if there are any authoritative testcases for that? > > * Is it useful to have oaep_decode_mgf1 and oaep_encode_mgf1 advertised > as public functions, or would it be better to make them internal? > > * Do you see any reasonable (i.e., with a net gain in maintainability) > way to share more code between _oaep_sec_decrypt_variable and > _pkcs1_sec_decrypt_variable? Hi Niels, I did review this part, and to me it seem like it is more maintainable to keep them separate, they already are tricky as it is, adding more variability sounds to me would just make them more complex and difficult to reason about. HTH, Simo. > * For oaep_decode_mgf1, oaep_encode_mgf1, maybe one could let the caller > allocate and pass in the appropriate hashing context? Would be easy to > do, e.g., in rsa_oaep_sha512_decrypt. But it looks like that would be > inconsistent with pss_mgf1, though (which looks like it needs a > separate hashing context). > > * I think it was a design mistake to represent RSA ciphertexts as mpz_t > rather then octet strings in Nettle's original RSA interfaces. I > wonder if it would make sense to let the new functions take > octet strings instead? > > Regards, > /Niels > > -- > Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. > Internet email is subject to wholesale government surveillance. > ___ > nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se > To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se -- Simo Sorce Distinguished Engineer RHEL Crypto Team Red Hat, Inc ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Hello Niels, Simo, Thanks for the comments. I've updated MR addressing part of them (see inline for the details). Simo Sorce writes: > On Mon, 2024-01-15 at 21:43 +0100, Niels Möller wrote: >> Thanks, I've had a look, and it looks pretty good to me. Some comments >> and questions: >> >> * For tests, would it make some with some test that check that >> encryption with a given message and randomness gives the expected >> output? Even better if there are any authoritative testcases for that? I would be happy to add if there are any, even if they are not so authoritative, though I wasn't even able to find ones with compatible license, in particular with SHA-2 being used as an underlying hash algorithm for MGF-1. - Project Wycheproof (Apache 2.0): https://github.com/google/wycheproof/blob/master/testvectors/rsa_oaep_2048_sha256_mgf1sha256_test.json - Python Cryptography (Apache 2.0 and BSD): https://cryptography.io/en/latest/development/custom-vectors/rsa-oaep-sha2/ In any case, I'll try to check against those vectors manually outside the Nettle repository to ensure the correctness. >> * Is it useful to have oaep_decode_mgf1 and oaep_encode_mgf1 advertised >> as public functions, or would it be better to make them internal? Made them internal functions. >> * Do you see any reasonable (i.e., with a net gain in maintainability) >> way to share more code between _oaep_sec_decrypt_variable and >> _pkcs1_sec_decrypt_variable? > > I did review this part, and to me it seem like it is more maintainable > to keep them separate, they already are tricky as it is, adding more > variability sounds to me would just make them more complex and > difficult to reason about. I agree with that, considering potential optimization opportunities by the compiler. >> * For oaep_decode_mgf1, oaep_encode_mgf1, maybe one could let the caller >> allocate and pass in the appropriate hashing context? Would be easy to >> do, e.g., in rsa_oaep_sha512_decrypt. But it looks like that would be >> inconsistent with pss_mgf1, though (which looks like it needs a >> separate hashing context). Done. >> * I think it was a design mistake to represent RSA ciphertexts as mpz_t >> rather then octet strings in Nettle's original RSA interfaces. I >> wonder if it would make sense to let the new functions take >> octet strings instead? Done. Regards, -- Daiki Ueno ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Daiki Ueno writes: >>> * For tests, would it make some with some test that check that >>> encryption with a given message and randomness gives the expected >>> output? Even better if there are any authoritative testcases for that? > > I would be happy to add if there are any, even if they are not so > authoritative, though I wasn't even able to find ones with compatible > license, in particular with SHA-2 being used as an underlying hash > algorithm for MGF-1. > > - Project Wycheproof (Apache 2.0): > > https://github.com/google/wycheproof/blob/master/testvectors/rsa_oaep_2048_sha256_mgf1sha256_test.json > > - Python Cryptography (Apache 2.0 and BSD): > https://cryptography.io/en/latest/development/custom-vectors/rsa-oaep-sha2/ > > In any case, I'll try to check against those vectors manually outside > the Nettle repository to ensure the correctness. To me it looks like those sources provide reasonable test vectors for RSA OAEP decryption. On licensing, it looks like Apache and GPLv2 might be incompatible. I've been a bit sloppy when incorporating test code (e.g., for some time I had some testcode copied from openssl/libcrypto, to test compatibility glue). But in this case, I think a fully correct workaround would be to license the related test file LGPLv3 (no GPLv2 option); odd licensing for some of the test files shouldn't matter much for Nettle applications since the testcode isn't part of the library applications link. Proper attribution is of course important. But my original question was for testing of RSA *en*cryption, if there are some determinstic testvectors with known output, with tests wiring something non-random for the randomness input. >>> * Is it useful to have oaep_decode_mgf1 and oaep_encode_mgf1 advertised >>> as public functions, or would it be better to make them internal? > > Made them internal functions. Thanks. (It was maybe a mistake we didn't do that for the pss_*_mgf1 functions when added years ago). >>> * Do you see any reasonable (i.e., with a net gain in maintainability) >>> way to share more code between _oaep_sec_decrypt_variable and >>> _pkcs1_sec_decrypt_variable? >> >> I did review this part, and to me it seem like it is more maintainable >> to keep them separate, they already are tricky as it is, adding more >> variability sounds to me would just make them more complex and >> difficult to reason about. > > I agree with that, considering potential optimization opportunities by > the compiler. Let's leave this as is, then. >>> * For oaep_decode_mgf1, oaep_encode_mgf1, maybe one could let the caller >>> allocate and pass in the appropriate hashing context? Would be easy to >>> do, e.g., in rsa_oaep_sha512_decrypt. But it looks like that would be >>> inconsistent with pss_mgf1, though (which looks like it needs a >>> separate hashing context). > > Done. Nice. We'll still have another one allocated for each call to pss_mgf1, if I read the code correctly. >>> * I think it was a design mistake to represent RSA ciphertexts as mpz_t >>> rather then octet strings in Nettle's original RSA interfaces. I >>> wonder if it would make sense to let the new functions take >>> octet strings instead? > > Done. What does the OAEP spec say about the ciphertet length? It would make the interface easier if we say that the ciphertext length *always* equals key->size; then one could delete passing and checking of the ciphertext_length argument. In the current MR, it looks like leading zero bytes are trimmed (behavior of nettle_mpz_sizeinbase_256_u), so that ciphertext may sometimes be shorter. Regards, /Niels -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Hello, Niels Möller writes: > Daiki Ueno writes: > * For tests, would it make some with some test that check that encryption with a given message and randomness gives the expected output? Even better if there are any authoritative testcases for that? >> >> I would be happy to add if there are any, even if they are not so >> authoritative, though I wasn't even able to find ones with compatible >> license, in particular with SHA-2 being used as an underlying hash >> algorithm for MGF-1. >> >> - Project Wycheproof (Apache 2.0): >> >> https://github.com/google/wycheproof/blob/master/testvectors/rsa_oaep_2048_sha256_mgf1sha256_test.json >> >> - Python Cryptography (Apache 2.0 and BSD): >> https://cryptography.io/en/latest/development/custom-vectors/rsa-oaep-sha2/ >> >> In any case, I'll try to check against those vectors manually outside >> the Nettle repository to ensure the correctness. > > To me it looks like those sources provide reasonable test vectors for > RSA OAEP decryption. > > On licensing, it looks like Apache and GPLv2 might be incompatible. I've > been a bit sloppy when incorporating test code (e.g., for some time I > had some testcode copied from openssl/libcrypto, to test compatibility > glue). But in this case, I think a fully correct workaround would be to > license the related test file LGPLv3 (no GPLv2 option); odd licensing > for some of the test files shouldn't matter much for Nettle applications > since the testcode isn't part of the library applications link. Proper > attribution is of course important. > > But my original question was for testing of RSA *en*cryption, if there > are some determinstic testvectors with known output, with tests wiring > something non-random for the randomness input. I did a bit of a research and realized that, when we added OAEP in libgcrypt, we apparently picked the test vector from: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1/pkcs-1v2-1d2-vec.zip https://lists.gnupg.org/pipermail/gcrypt-devel/2011-June/001802.html The zip file is no longer accessible, but I still keep a copy and it seems identical to the one at: https://github.com/pyca/cryptography/tree/main/vectors/cryptography_vectors/asymmetric/RSA/pkcs-1v2-1d2-vec Is it OK to use the vector assuming it is public domain? In any case I incorporated one test from the vector. > What does the OAEP spec say about the ciphertet length? It would make > the interface easier if we say that the ciphertext length *always* > equals key->size; then one could delete passing and checking of the > ciphertext_length argument. In the current MR, it looks like leading > zero bytes are trimmed (behavior of nettle_mpz_sizeinbase_256_u), so > that ciphertext may sometimes be shorter. Yes, the length should match key->size; I've omitted the ciphertext_length argument. I'm not sure about the leading zeros though; as far as I read, nettle_mpz_to_octets seems to keep them. Regards, -- Daiki Ueno ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Daiki Ueno writes: > The zip file is no longer accessible, but I still keep a copy and it > seems identical to the one at: > https://github.com/pyca/cryptography/tree/main/vectors/cryptography_vectors/asymmetric/RSA/pkcs-1v2-1d2-vec > > Is it OK to use the vector assuming it is public domain? According to the closest LICENSE file, https://github.com/pyca/cryptography/blob/main/vectors/LICENSE, it's dual licensed apache/BSD (our choice), so I think that is fine. And if we copy just the test vectors and not any surrounding code, it seems questionable if that is even copyrightable. So I think copying from there, with proper attribution, is perfectly fine. Formally, we'll be exercising the BSD option. > Yes, the length should match key->size; I've omitted the > ciphertext_length argument. Thanks. Please remove everywhere, it looks like it's still present in some form in the test code. (You may still want to allocate an extra byte at the end and check that it isn't modified. Alternatively, rely on valgrind for detecting overwrites instead). > I'm not sure about the leading zeros > though; as far as I read, nettle_mpz_to_octets seems to keep them. I think nettle_mpz_to_octets is fine. The problem was when the length passed to this function was computed using nettle_mpz_sizeinbase_256_u, like it was in a previous revision. Regards, /Niels -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Niels Möller writes: > Daiki Ueno writes: > >> The zip file is no longer accessible, but I still keep a copy and it >> seems identical to the one at: >> https://github.com/pyca/cryptography/tree/main/vectors/cryptography_vectors/asymmetric/RSA/pkcs-1v2-1d2-vec >> >> Is it OK to use the vector assuming it is public domain? > > According to the closest LICENSE file, > https://github.com/pyca/cryptography/blob/main/vectors/LICENSE, it's dual > licensed apache/BSD (our choice), so I think that is fine. And if > we copy just the test vectors and not any surrounding code, it seems > questionable if that is even copyrightable. > > So I think copying from there, with proper attribution, is perfectly > fine. Formally, we'll be exercising the BSD option. OK, thanks for the confirmation. I've expanded the KAT tests further using the vector and also added a license notice. >> Yes, the length should match key->size; I've omitted the >> ciphertext_length argument. > > Thanks. Please remove everywhere, it looks like it's still present in > some form in the test code. (You may still want to allocate an extra > byte at the end and check that it isn't modified. Alternatively, rely on > valgrind for detecting overwrites instead). Added `mark_bytes_undefined (1, &ciphertext[key->size]);` to the test cases doing encryption. Regards, -- Daiki Ueno ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Daiki Ueno writes: > Added `mark_bytes_undefined (1, &ciphertext[key->size]);` to the test > cases doing encryption. I'm afraid that isn't right. For one, mark_bytes_undefined is conditioned so it only has any effect when running the sc tests. Second, it will not produce any warnings for writes, which I think is what we'd like to detect here. I think the options are: 1. Just don't allocate any extra byte, and valgrind's should arrange for alerts on out-of-bounds writes without anything special. 2. Allocate an extra byte, write some random value before the call, and check that the value is unchanged after the call (some other tests do that sort of thing, it's simple, old fashioned, and doesn't depend on valgrind). 3. Allocate an extra byte, and mark it using VALGRIND_MAKE_MEM_NOACCESS (wrapped in some macro depending on the memcheck.h configure check). I don't think that gives any real benefit over valgrind's default behavior with (1), but might make sense if done in combination with (2). Regards, /Niels -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Niels Möller writes: > Daiki Ueno writes: > >> Added `mark_bytes_undefined (1, &ciphertext[key->size]);` to the test >> cases doing encryption. > > I'm afraid that isn't right. For one, mark_bytes_undefined is > conditioned so it only has any effect when running the sc tests. Second, > it will not produce any warnings for writes, which I think is what we'd > like to detect here. I think the options are: > > 1. Just don't allocate any extra byte, and valgrind's should arrange for >alerts on out-of-bounds writes without anything special. > > 2. Allocate an extra byte, write some random value before the call, and >check that the value is unchanged after the call (some other tests >do that sort of thing, it's simple, old fashioned, and doesn't depend >on valgrind). > > 3. Allocate an extra byte, and mark it using VALGRIND_MAKE_MEM_NOACCESS >(wrapped in some macro depending on the memcheck.h configure check). >I don't think that gives any real benefit over valgrind's default >behavior with (1), but might make sense if done in combination with >(2). Sorry for the confusion and thank you for the explanation; now I get it. I pushed a change along the of option (2). Could you take a look again? Regards, -- Daiki Ueno ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Daiki Ueno writes: > Sorry for the confusion and thank you for the explanation; now I get it. > I pushed a change along the of option (2). Could you take a look again? Thanks. This is an unusually busy week for me, so I'm afraid I'll not be able to look at this (or any of the other pendning changes recently posted to the list) until next week. Regards, /Niels -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Daiki Ueno writes: > Sorry for the confusion and thank you for the explanation; now I get it. > I pushed a change along the of option (2). Could you take a look again? Thanks, looks good! Two nits, and let me know at which point you'd like to get it merged, and do further improvements as followup MRs. Since the oaep.h header now only declares internal functions, it shouldn't be installed (moved from HEADERS to DISTFILES in Makefile.in). And it would be nice if the manual could give some more detail about the label: As I understnd it, the label is optional, so it's fine to pass 0, NULL if not needed. And if used, the same label must be used for both encrypt and decrypt. Regards, /Niels -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Niels Möller writes: > Daiki Ueno writes: > >> Sorry for the confusion and thank you for the explanation; now I get it. >> I pushed a change along the of option (2). Could you take a look again? > > Thanks, looks good! Two nits, and let me know at which point you'd like > to get it merged, and do further improvements as followup MRs. Thank you; I have addressed those issues. As for the merging, I think it is ready now. I have also created a draft MR in GnuTLS to use the algorithm in PKCS#8, etc., so we can test the implementation in a broader context: https://gitlab.com/gnutls/gnutls/-/merge_requests/1805 > Since the oaep.h header now only declares internal functions, it > shouldn't be installed (moved from HEADERS to DISTFILES in Makefile.in). > > And it would be nice if the manual could give some more detail about the > label: As I understnd it, the label is optional, so it's fine to pass 0, > NULL if not needed. And if used, the same label must be used for both > encrypt and decrypt. Regards, -- Daiki Ueno ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Daiki Ueno writes: > Thank you; I have addressed those issues. As for the merging, I think > it is ready now. Thanks, merged. Thanks to the doc update, I now noticed the possibility of failure from the encryption functions. Failure is propagated from _oaep_encode_mgf1, which does assert (key_size >= 2 * hash->digest_size - 2); if (message_length > key_size - 2 * hash->digest_size - 2) return 0; Why is the first an assert (it could be triggered by using an unusually small RSA key with a large hash function, say rsa_oaep_sha512_encrypt with an old 512-bit RSA key, which from the docs isn't an obviously invalid usage), and the other a return value to indicate failure? One alternative could be to instead check if (message_length > key_size || message_length + 2 + 2*hash->digest_size > key_size) return 0; (with two tests, to not trigger overflow in case message_length is close to the maximum size_t value; maybe that is more defensive than necessary since message_length large enough to trigger overflow can't be the size of properly allocated memory area). The opposite alternative would to be to have a documented way for the application to get the maximum message size, and have an assertion failure for both cases. That would have the advantage that no return value is needed, simplifying the api (at least very locally). Another doc detail: The docs for the decrypt functions don't say explicitly that *length is both an input and output argument. The text for the older function rsa_decrypt could be reused (or possibly improved). Regards, /Niels -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Hello Niels,
Thank you for merging it and the suggestions.
Niels Möller writes:
> Thanks to the doc update, I now noticed the possibility of failure from
> the encryption functions. Failure is propagated from _oaep_encode_mgf1,
> which does
>
> assert (key_size >= 2 * hash->digest_size - 2);
>
> if (message_length > key_size - 2 * hash->digest_size - 2)
> return 0;
>
> Why is the first an assert (it could be triggered by using an unusually
> small RSA key with a large hash function, say rsa_oaep_sha512_encrypt
> with an old 512-bit RSA key, which from the docs isn't an obviously
> invalid usage), and the other a return value to indicate failure?
I think I split these conditions that way, as the first condition is
more about the algorithm setup, i.e., the user should choose RSA key
size and hash digest size appropriately, before using any of the
rsa_oaep_*_encrypt functions; otherwise treat it as a programming error.
That said, I agree that it would be more user friendly to combine them
and treat it as a regular error, as we do in pss_encode_mgf1.
> One alternative could be to instead check
>
> if (message_length > key_size
> || message_length + 2 + 2*hash->digest_size > key_size)
> return 0;
>
> (with two tests, to not trigger overflow in case message_length is close
> to the maximum size_t value; maybe that is more defensive than necessary
> since message_length large enough to trigger overflow can't be the size of
> properly allocated memory area).
I made this change in the attached patch.
> The opposite alternative would to be to have a documented way for the
> application to get the maximum message size, and have an assertion
> failure for both cases. That would have the advantage that no return
> value is needed, simplifying the api (at least very locally).
It is tempting, though for now I just documented the size restriction.
> Another doc detail: The docs for the decrypt functions don't say
> explicitly that *length is both an input and output argument. The text
> for the older function rsa_decrypt could be reused (or possibly
> improved).
Updated the documentation.
Regards,
--
Daiki Ueno
>From 04a7eb300a555617fd6f0503fdd48e8cf389e790 Mon Sep 17 00:00:00 2001
From: Daiki Ueno
Date: Fri, 16 Feb 2024 14:47:18 +0900
Subject: [PATCH] Clarify message length limitation in RSA-OAEP
Signed-off-by: Daiki Ueno
---
nettle.texinfo | 13 ++---
oaep.c | 5 ++---
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/nettle.texinfo b/nettle.texinfo
index 25bce6fb..95e89971 100644
--- a/nettle.texinfo
+++ b/nettle.texinfo
@@ -5202,8 +5202,12 @@ is done with one of the following functions:
@deftypefunx int rsa_oaep_sha384_encrypt (const struct rsa_public_key *@var{key}, void *@var{random_ctx}, nettle_random_func *@var{random}, size_t @var{label_length}, const uint8_t *@var{label}, size_t @var{length}, const uint8_t *@var{message}, uint8_t *@var{ciphertext})
@deftypefunx int rsa_oaep_sha512_encrypt (const struct rsa_public_key *@var{key}, void *@var{random_ctx}, nettle_random_func *@var{random}, size_t @var{label_length}, const uint8_t *@var{label}, size_t @var{length}, const uint8_t *@var{message}, uint8_t *@var{ciphertext})
Returns 1 on success, 0 on failure. The label is optional and if
-omitted, @var{label_length} and @var{label} can be set to 0 and
-@code{NULL} respectively.
+omitted, @var{label_length} and @var{label} is set to 0 and
+@code{NULL} respectively. The maximum size of @var{message} can be
+calculated with @code{k - 2 * hLen - 2}, where @code{k} is the key
+size in octets obtained with @var{key}->size, and @code{hLen} is the
+output size of the underlying hash function (e.g.,
+@code{SHA256_DIGEST_SIZE} for @code{rsa_oaep_sha256_encrypt}).
@end deftypefun
Decrypting a cipher text message using RSA with the OAEP padding
@@ -5213,7 +5217,10 @@ scheme is done with one of the following functions:
@deftypefunx int rsa_oaep_sha384_decrypt (const struct rsa_public_key *@var{pub}, const struct rsa_private_key *@var{key}, void *@var{random_ctx}, nettle_random_func *@var{random}, size_t @var{label_length}, const uint8_t *@var{label}, size_t *@var{length}, uint8_t *@var{message}, const uint8_t *@var{ciphertext})
@deftypefunx int rsa_oaep_sha512_decrypt (const struct rsa_public_key *@var{pub}, const struct rsa_private_key *@var{key}, void *@var{random_ctx}, nettle_random_func *@var{random}, size_t @var{label_length}, const uint8_t *@var{label}, size_t *@var{length}, uint8_t *@var{message}, const uint8_t *@var{ciphertext})
Returns 1 on success, 0 on failure. These function utilize randomized
-RSA blinding similarly to @code{rsa_decrypt_tr}.
+RSA blinding similarly to @code{rsa_decrypt_tr}. The message buffer
+pointed to by @var{cleartext} must be of size *@var{length}. After
+decryption, *@var{length} will be updated with the size of the
+message.
@end deftypefun
diff --git a/oaep.c b/oaep.c
index c504fbb9..5ebf8ba0 100644
--- a/oaep.c
Re: Add RSA-OAEP encryption/decryption to Nettle
Daiki Ueno writes: > That said, I agree that it would be more user friendly to combine them > and treat it as a regular error, as we do in pss_encode_mgf1. Thanks for the update, patch merged. I noticed that there are two failures in the ci builds. See https://gitlab.com/gnutls/nettle/-/pipelines/1178451395. One failure is the new side-channel test failing with mini-gmp. Which is expected, the test should just be skipped in mini-gmp builds (similar to several other sc tests). The other is a complaint from ubsan. I guess it's related to the label == NULL case. I don't know what's the proper place for a fix, maybe it's not in the new code. I think the Nettle APIs should generally allow size == 0, ptr == NULL more or less everywhere, even where libc functions we use formally require ptr != NULL. Can you have a look? Regards, /Niels -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Niels Möller writes:
> I noticed that there are two failures in the ci builds. See
> https://gitlab.com/gnutls/nettle/-/pipelines/1178451395.
>
> One failure is the new side-channel test failing with mini-gmp. Which is
> expected, the test should just be skipped in mini-gmp builds (similar to
> several other sc tests).
Yes, I'm attaching the patch for this.
> The other is a complaint from ubsan. I guess it's related to the label
> == NULL case. I don't know what's the proper place for a fix, maybe it's
> not in the new code. I think the Nettle APIs should generally allow size
> == 0, ptr == NULL more or less everywhere, even where libc functions we
> use formally require ptr != NULL.
This is similar to this issue:
https://gitlab.com/gnutls/gnutls/-/issues/1306
where we passed NULL to sha*_update in the GnuTLS code, though it turned
to be a non-issue.
In the RSA-OAEP case, I'm not exactly sure whether we should be able to
safely special case label == NULL as its hash is part of plaintext data
block. Therefore I'm adding label = "" at the API entry points.
Regards,
--
Daiki Ueno
>From 9ffbac0aa6807231a6842a1ee67f6999c9c2c97a Mon Sep 17 00:00:00 2001
From: Daiki Ueno
Date: Sat, 17 Feb 2024 08:58:47 +0900
Subject: [PATCH] Fix a couple of CI failures in rsa-oaep-encrypt-test
- Skip sc-rsa-oaep-encrypt-test when compiled with mini-gmp
- Pass in "" as label if it was NULL, to pacify __nonnull nature of
memcpy
Signed-off-by: Daiki Ueno
---
rsa-oaep-decrypt.c| 7 +++
rsa-oaep-encrypt.c| 7 +++
testsuite/rsa-oaep-encrypt-test.c | 4
3 files changed, 18 insertions(+)
diff --git a/rsa-oaep-decrypt.c b/rsa-oaep-decrypt.c
index 4006a021..2c00422c 100644
--- a/rsa-oaep-decrypt.c
+++ b/rsa-oaep-decrypt.c
@@ -55,6 +55,13 @@ _rsa_oaep_decrypt (const struct rsa_public_key *pub,
TMP_GMP_DECL (m, mp_limb_t);
TMP_GMP_DECL (em, uint8_t);
int res;
+ const uint8_t empty = 0;
+
+ if (label == NULL)
+{
+ assert (label_length == 0);
+ label = ∅
+}
TMP_GMP_ALLOC (m, mpz_size (pub->n));
TMP_GMP_ALLOC (em, key->size);
diff --git a/rsa-oaep-encrypt.c b/rsa-oaep-encrypt.c
index 488821f0..7e6bb1e5 100644
--- a/rsa-oaep-encrypt.c
+++ b/rsa-oaep-encrypt.c
@@ -51,9 +51,16 @@ _rsa_oaep_encrypt (const struct rsa_public_key *key,
uint8_t *ciphertext)
{
mpz_t gibberish;
+ const uint8_t empty = 0;
mpz_init (gibberish);
+ if (label == NULL)
+{
+ assert (label_length == 0);
+ label = ∅
+}
+
if (_oaep_encode_mgf1 (gibberish, key->size,
random_ctx, random,
hash_ctx, hash,
diff --git a/testsuite/rsa-oaep-encrypt-test.c b/testsuite/rsa-oaep-encrypt-test.c
index 3d9808a5..511c2744 100644
--- a/testsuite/rsa-oaep-encrypt-test.c
+++ b/testsuite/rsa-oaep-encrypt-test.c
@@ -530,6 +530,10 @@ test_encrypt (void)
void
test_main (void)
{
+#if NETTLE_USE_MINI_GMP || WITH_EXTRA_ASSERTS
+ if (test_side_channel)
+SKIP();
+#endif
test_encrypt_decrypt ();
test_encrypt ();
}
--
2.43.0
___
nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se
To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Daiki Ueno writes: > Niels Möller writes: >> One failure is the new side-channel test failing with mini-gmp. Which is >> expected, the test should just be skipped in mini-gmp builds (similar to >> several other sc tests). > > Yes, I'm attaching the patch for this. I've committed and pushed that part of patch. >> The other is a complaint from ubsan. I guess it's related to the label >> == NULL case. I don't know what's the proper place for a fix, maybe it's >> not in the new code. I think the Nettle APIs should generally allow size >> == 0, ptr == NULL more or less everywhere, even where libc functions we >> use formally require ptr != NULL. > > This is similar to this issue: > https://gitlab.com/gnutls/gnutls/-/issues/1306 > where we passed NULL to sha*_update in the GnuTLS code, though it turned > to be a non-issue. I don't remember seeing that issue. I think it should be allowed to call sha*_update with 0, NULL (when size is null, there's no reason to ever attempt to dereference that pointer). I'll see if I can fix that. Regards, /Niels -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Niels Möller writes:
>> This is similar to this issue:
>> https://gitlab.com/gnutls/gnutls/-/issues/1306
>> where we passed NULL to sha*_update in the GnuTLS code, though it turned
>> to be a non-issue.
>
> I don't remember seeing that issue. I think it should be allowed to call
> sha*_update with 0, NULL (when size is null, there's no reason to ever
> attempt to dereference that pointer). I'll see if I can fix that.
Below patch seems to fix this issue, but not entirely sure that's the
way I want to do it. I think I'd rather not touch the MD_* macros
defined in macros.h, and do improved macros in md-internal.h instead.
Since, for historic reasons, the macros.h file is public.
To get this thoroughly fixed, one would need tests where every nettle
function, that accepts a potentially empty buffer, is called with 0,
NULL, and make sure ubsan is happy with that.
Regards,
/Niels
diff --git a/macros.h b/macros.h
index 990d32ee..e67a403f 100644
--- a/macros.h
+++ b/macros.h
@@ -180,6 +180,8 @@ do {\
length and data. */
#define MD_UPDATE(ctx, length, data, f, incr) \
do { \
+if (length == 0) \
+ goto __md_done; \
if ((ctx)->index) \
{
\
/* Try to fill partial block */ \
diff --git a/sha256.c b/sha256.c
index 0c9c21a0..907271bc 100644
--- a/sha256.c
+++ b/sha256.c
@@ -105,6 +105,9 @@ sha256_update(struct sha256_ctx *ctx,
size_t length, const uint8_t *data)
{
size_t blocks;
+ if (length == 0)
+return;
+
if (ctx->index > 0)
{
/* Try to fill partial block */
--
Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677.
Internet email is subject to wholesale government surveillance.
___
nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se
To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
Re: Add RSA-OAEP encryption/decryption to Nettle
Niels Möller writes: > Below patch seems to fix this issue, but not entirely sure that's the > way I want to do it. I've pushed a fix, along the same lines, see https://git.lysator.liu.se/nettle/nettle/-/commit/99e62003c3916fdef04a2d3327281f8f498b609e I believe that should fix all hash update functions (and with proper test coverage). There are probably a few more functions where 0, NULL should be allowed, but currently result in ubsan issues: Corresponding aead update functions, functions accepting optional nonces, empty messages for rsa encryption functions, maybe some of the cipher modes. Regards, /Niels -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se
