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

Added signature length check for RSA PSS signature verification. #11310

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions crypto/err/openssl.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2892,7 +2892,6 @@ RSA_R_BAD_PAD_BYTE_COUNT:103:bad pad byte count
RSA_R_BAD_SIGNATURE:104:bad signature
RSA_R_BLOCK_TYPE_IS_NOT_01:106:block type is not 01
RSA_R_BLOCK_TYPE_IS_NOT_02:107:block type is not 02
RSA_R_DATA_GREATER_THAN_MOD_LEN:108:data greater than mod len
RSA_R_DATA_TOO_LARGE:109:data too large
RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE:110:data too large for key size
RSA_R_DATA_TOO_LARGE_FOR_MODULUS:132:data too large for modulus
Expand Down Expand Up @@ -3273,7 +3272,6 @@ SSL_R_VERSION_TOO_LOW:396:version too low
SSL_R_WRONG_CERTIFICATE_TYPE:383:wrong certificate type
SSL_R_WRONG_CIPHER_RETURNED:261:wrong cipher returned
SSL_R_WRONG_CURVE:378:wrong curve
SSL_R_WRONG_SIGNATURE_LENGTH:264:wrong signature length
SSL_R_WRONG_SIGNATURE_SIZE:265:wrong signature size
SSL_R_WRONG_SIGNATURE_TYPE:370:wrong signature type
SSL_R_WRONG_SSL_VERSION:266:wrong ssl version
Expand Down
2 changes: 0 additions & 2 deletions crypto/rsa/rsa_err.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ static const ERR_STRING_DATA RSA_str_reasons[] = {
"block type is not 01"},
{ERR_PACK(ERR_LIB_RSA, 0, RSA_R_BLOCK_TYPE_IS_NOT_02),
"block type is not 02"},
{ERR_PACK(ERR_LIB_RSA, 0, RSA_R_DATA_GREATER_THAN_MOD_LEN),
"data greater than mod len"},
{ERR_PACK(ERR_LIB_RSA, 0, RSA_R_DATA_TOO_LARGE), "data too large"},
{ERR_PACK(ERR_LIB_RSA, 0, RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE),
"data too large for key size"},
Expand Down
17 changes: 4 additions & 13 deletions crypto/rsa/rsa_ossl.c
Original file line number Diff line number Diff line change
Expand Up @@ -400,13 +400,8 @@ static int rsa_ossl_private_decrypt(int flen, const unsigned char *from,
goto err;
}

/*
* This check was for equality but PGP does evil things and chops off the
* top '0' bytes
*/
if (flen > num) {
RSAerr(RSA_F_RSA_OSSL_PRIVATE_DECRYPT,
RSA_R_DATA_GREATER_THAN_MOD_LEN);
if (flen != num) {
RSAerr(RSA_F_RSA_OSSL_PRIVATE_DECRYPT, RSA_R_WRONG_SIGNATURE_LENGTH);
goto err;
}

Expand Down Expand Up @@ -556,12 +551,8 @@ static int rsa_ossl_public_decrypt(int flen, const unsigned char *from,
goto err;
}

/*
* This check was for equality but PGP does evil things and chops off the
* top '0' bytes
*/
if (flen > num) {
RSAerr(RSA_F_RSA_OSSL_PUBLIC_DECRYPT, RSA_R_DATA_GREATER_THAN_MOD_LEN);
if (flen != num) {
RSAerr(RSA_F_RSA_OSSL_PUBLIC_DECRYPT, RSA_R_WRONG_SIGNATURE_LENGTH);
goto err;
}

Expand Down
6 changes: 0 additions & 6 deletions crypto/rsa/rsa_saos.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,6 @@ int RSA_verify_ASN1_OCTET_STRING(int dtype,
const unsigned char *p;
ASN1_OCTET_STRING *sig = NULL;

if (siglen != (unsigned int)RSA_size(rsa)) {
RSAerr(RSA_F_RSA_VERIFY_ASN1_OCTET_STRING,
RSA_R_WRONG_SIGNATURE_LENGTH);
return 0;
}

s = OPENSSL_malloc((unsigned int)siglen);
if (s == NULL) {
RSAerr(RSA_F_RSA_VERIFY_ASN1_OCTET_STRING, ERR_R_MALLOC_FAILURE);
Expand Down
5 changes: 0 additions & 5 deletions crypto/rsa/rsa_sign.c
Original file line number Diff line number Diff line change
Expand Up @@ -293,11 +293,6 @@ int int_rsa_verify(int type, const unsigned char *m, unsigned int m_len,
size_t decrypt_len, encoded_len = 0;
unsigned char *decrypt_buf = NULL, *encoded = NULL;

if (siglen != (size_t)RSA_size(rsa)) {
RSAerr(RSA_F_INT_RSA_VERIFY, RSA_R_WRONG_SIGNATURE_LENGTH);
return 0;
}

/* Recover the encoded digest. */
decrypt_buf = OPENSSL_malloc(siglen);
if (decrypt_buf == NULL) {
Expand Down
17 changes: 7 additions & 10 deletions doc/man3/RSA_public_encrypt.pod
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,13 @@ RSA_size(B<rsa>) bytes.
B<to> and B<from> may overlap.

RSA_private_decrypt() decrypts the B<flen> bytes at B<from> using the
private key B<rsa> and stores the plaintext in B<to>. B<flen> should
be equal to RSA_size(B<rsa>) but may be smaller, when leading zero
bytes are in the ciphertext. Those are not important and may be removed,
but RSA_public_encrypt() does not do that. B<to> must point
to a memory section large enough to hold the maximal possible decrypted
data (which is equal to RSA_size(B<rsa>) for RSA_NO_PADDING,
RSA_size(B<rsa>) - 11 for the PKCS #1 v1.5 based padding modes and
RSA_size(B<rsa>) - 42 for RSA_PKCS1_OAEP_PADDING).
B<padding> is the padding mode that was used to encrypt the data.
B<to> and B<from> may overlap.
private key B<rsa> and stores the plaintext in B<to>. B<flen> must be
equal to RSA_size(B<rsa>). B<to> must point to a memory section large
enough to hold the maximal possible decrypted data (which is equal to
RSA_size(B<rsa>) for RSA_NO_PADDING, RSA_size(B<rsa>) - 11 for the
PKCS #1 v1.5 based padding modes and RSA_size(B<rsa>) - 42 for
RSA_PKCS1_OAEP_PADDING). B<padding> is the padding mode that was used
to encrypt the data. B<to> and B<from> may overlap.

=head1 RETURN VALUES

Expand Down
1 change: 0 additions & 1 deletion include/openssl/rsaerr.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ int ERR_load_RSA_strings(void);
# define RSA_R_BAD_SIGNATURE 104
# define RSA_R_BLOCK_TYPE_IS_NOT_01 106
# define RSA_R_BLOCK_TYPE_IS_NOT_02 107
# define RSA_R_DATA_GREATER_THAN_MOD_LEN 108
# define RSA_R_DATA_TOO_LARGE 109
# define RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE 110
# define RSA_R_DATA_TOO_LARGE_FOR_MODULUS 132
Expand Down
1 change: 0 additions & 1 deletion include/openssl/sslerr.h
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,6 @@ int ERR_load_SSL_strings(void);
# define SSL_R_WRONG_CERTIFICATE_TYPE 383
# define SSL_R_WRONG_CIPHER_RETURNED 261
# define SSL_R_WRONG_CURVE 378
# define SSL_R_WRONG_SIGNATURE_LENGTH 264
# define SSL_R_WRONG_SIGNATURE_SIZE 265
# define SSL_R_WRONG_SIGNATURE_TYPE 370
# define SSL_R_WRONG_SSL_VERSION 266
Expand Down
2 changes: 0 additions & 2 deletions ssl/ssl_err.c
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,6 @@ static const ERR_STRING_DATA SSL_str_reasons[] = {
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_WRONG_CIPHER_RETURNED),
"wrong cipher returned"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_WRONG_CURVE), "wrong curve"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_WRONG_SIGNATURE_LENGTH),
"wrong signature length"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_WRONG_SIGNATURE_SIZE),
"wrong signature size"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_WRONG_SIGNATURE_TYPE),
Expand Down
25 changes: 25 additions & 0 deletions test/certs/cert-wrong-sig-len.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
-----BEGIN CERTIFICATE-----
MIIEHTCCAtKgAwIBAgIUJvpzQbf5lboq/NuxbTBAi0HVKsgwQQYJKoZIhvcNAQEK
MDSgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQMEAgEF
AKIDAgEgMBIxEDAOBgNVBAMMB1Jvb3QgQ0EwHhcNMjAwMzEyMTEwMjA2WhcNMzkw
NTEyMTEwMjA2WjAYMRYwFAYDVQQDDA1XUk9ORy1TSUctTEVOMIICIjANBgkqhkiG
9w0BAQEFAAOCAg8AMIICCgKCAgEArqgYa80c/QshKgXWgpm3N1plrHnqO7kodCdW
wKEDq0MSTL8Rvt6mdcmRT1ijx76QMlfSg83bc7m2QU0hpypBSzTfsFxXHMsfaAqE
HlYhKODrT+L1GpDKMsnbo4U83sjKyIgkspOR0Mtth/IXsjzWiJTtwoowko1KHIK5
iDx6s8hG1v8kupEmeQ+vLiVQm8WhV14iNsGKb3u5izwxs4bYLmMN0pYEDpRwcBXt
v+ilrs+jbQ/l9zLEMbyLNYHtcVA+QYv3tyJKBMF960LDA8VJaCXFyAYfVgfaiBrK
z9QBfLoE+5IfrOyRJxk2AtSfO4EuWcXU1p58LEOge3oiRcTl1z5CnaFjQR8ecv3Z
709N9MOxSt5sRHeWfE1nGcP8A9R7lbOv7V7RlNe1LM2AbLgYnTpe0qxwTF7lLqnw
JhP5RaWqBCL8Mpxn6r58qt+INFoXQspt+0/kwh3msM9mEljl1fOgqwm7XxEZ0hRu
UgAvUjcrOVlf6MMjXe65sUKH9H1/VumHqwLldlo356h33TwADvuHsoufVaizPkjp
YzGWmEyzgQX31I5g08i3q1xk49sB8RztnIioye9GUV+tY7RLR0xP1/kruuE7dtiO
XdmdKxHCrot1XW9KFlcUo0iGGPrzY+Rgr1Po7Tj2IAKoUpXe6p9Ud6pAkPc8L7Nj
piiHy/MCAwEAATBBBgkqhkiG9w0BAQowNKAPMA0GCWCGSAFlAwQCAQUAoRwwGgYJ
KoZIhvcNAQEIMA0GCWCGSAFlAwQCAQUAogMCASADggEAACkV0y/mMLqqEb1xlvzH
MD3tkeFbErkrV/l5Kod90n0nra1f/beS86d2w9L47AoBmMyTgrj+k2BXieZWZ8C2
vefdmfwynCtHtgK+ExeGN9Qnmb5rjWzH/HCwruHSSBnTs9/xZzSoSk5gTenL37ga
+G3KmtJHguCtOo3taWTirOTZ5F3hcX6QKBdk0BrZwNrBTGouRDx+CDkymiBjVM9P
idxIL7tcIDsuHr/83Z95L5J7QjKGeEUjYhXuY2/L3fVfqBe4pQev5K1cbGg1MeSF
vrp46StOJCvdo5qzZNHolAw7uHmh2FCjYDrnxuw9XMoAdI51Hvq2RiQxaDOUAk4O
kA==
-----END CERTIFICATE-----
5 changes: 4 additions & 1 deletion test/recipes/25-test_verify.t
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ sub verify {
run(app([@args]));
}

plan tests => 137;
plan tests => 138;

# Canonical success
ok(verify("ee-cert", "sslserver", ["root-cert"], ["ca-cert"]),
Expand Down Expand Up @@ -364,6 +364,9 @@ ok(verify("some-names2", "sslserver", ["many-constraints"], ["many-constraints"]
ok(verify("root-cert-rsa2", "sslserver", ["root-cert-rsa2"], [], "-check_ss_sig"),
"Public Key Algorithm rsa instead of rsaEncryption");

ok(!verify("cert-wrong-sig-len", "sslserver", ["root-cert"], [], ),
"Wrong signature length");

SKIP: {
skip "Ed25519 is not supported by this OpenSSL build", 1
if disabled("ec");
Expand Down