- May 13, 2015
-
-
Hanno Böck authored
The function obj_cmp() (file crypto/objects/obj_dat.c) can in some situations call memcmp() with a null pointer and a zero length. This is invalid behaviour. When compiling openssl with undefined behaviour sanitizer (add -fsanitize=undefined to compile flags) this can be seen. One example that triggers this behaviour is the pkcs7 command (but there are others, e.g. I've seen it with the timestamp function): apps/openssl pkcs7 -in test/testp7.pem What happens is that obj_cmp takes objects of the type ASN1_OBJECT and passes their ->data pointer to memcmp. Zero-sized ASN1_OBJECT structures can have a null pointer as data. RT#3816 Signed-off-by: Matt Caswell <matt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit 2b8dc08b)
-
Matt Caswell authored
Currently we set change_cipher_spec_ok to 1 before calling ssl3_get_cert_verify(). This is because this message is optional and if it is not sent then the next thing we would expect to get is the CCS. However, although it is optional, we do actually know whether we should be receiving one in advance. If we have received a client cert then we should expect a CertificateVerify message. By the time we get to this point we will already have bombed out if we didn't get a Certificate when we should have done, so it is safe just to check whether |peer| is NULL or not. If it is we won't get a CertificateVerify, otherwise we will. Therefore we should change the logic so that we only attempt to get the CertificateVerify if we are expecting one, and not allow a CCS in this scenario. Whilst this is good practice for TLS it is even more important for DTLS. In DTLS messages can be lost. Therefore we may be in a situation where a CertificateVerify message does not arrive even though one was sent. In that case the next message the server will receive will be the CCS. This could also happen if messages get re-ordered in-flight. In DTLS if |change_cipher_spec_ok| is not set and a CCS is received it is ignored. However if |change_cipher_spec_ok| *is* set then a CCS arrival will immediately move the server into the next epoch. Any messages arriving for the previous epoch will be ignored. This means that, in this scenario, the handshake can never complete. The client will attempt to retransmit missing messages, but the server will ignore them because they are the wrong epoch. The server meanwhile will still be waiting for the CertificateVerify which is never going to arrive. RT#2958 Reviewed-by: Emilia Käsper <emilia@openssl.org> (cherry picked from commit a0bd6493)
-
- May 11, 2015
-
-
Kurt Cancemi authored
Matt's note: I added a call to X509V3err to Kurt's original patch. RT#3840 Signed-off-by: Matt Caswell <matt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit 344c271e)
-
Bjoern D. Rasmussen authored
clang says: "s_cb.c:958:9: error: implicitly declaring library function 'memcpy'" Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (cherry picked from commit 8f744cce) Conflicts: apps/s_cb.c
-
Matt Caswell authored
If sk_SSL_CIPHER_new_null() returns NULL then ssl_bytes_to_cipher_list() should also return NULL. Based on an original patch by mrpre <mrpre@163.com>. Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit 14def5f5)
-
- May 07, 2015
-
-
Viktor Dukhovni authored
Reviewed-by: Rich Salz <rsalz@akamai.com>
-
- May 05, 2015
-
-
Matt Caswell authored
Ensure all fatal errors transition into the new error state for DTLS. Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit cefc9391) Conflicts: ssl/d1_srvr.c
-
Matt Caswell authored
Ensure all fatal errors transition into the new error state on the client side. Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit cc273a93) Conflicts: ssl/s3_clnt.c
-
Matt Caswell authored
Ensure all fatal errors transition into the new error state on the server side. Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit cf9b0b6f) Conflicts: ssl/s3_srvr.c
-
Matt Caswell authored
Reusing an SSL object when it has encountered a fatal error can have bad consequences. This is a bug in application code not libssl but libssl should be more forgiving and not crash. Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit a89db885) Conflicts: ssl/s3_srvr.c ssl/ssl_stat.c
-
Matt Caswell authored
Remove dependency on ssl_locl.h from v3_scts.c, and incidentally fix a build problem with kerberos (the dependency meant v3_scts.c was trying to include krb5.h, but without having been passed the relevanant -I flags to the compiler) Reviewed-by: Dr. Stephen Henson <steve@openssl.org> (cherry picked from commit d13bd613) Conflicts: crypto/x509v3/v3_scts.c
-
- May 04, 2015
-
-
Richard Levitte authored
RT2943 only complains about the incorrect check of -K argument size, we might as well do the same thing with the -iv argument. Before this, we only checked that the given argument wouldn't give a bitstring larger than EVP_MAX_KEY_LENGTH. we can be more precise and check against the size of the actual cipher used. (cherry picked from commit 8920a7cd ) Reviewed-by: Rich Salz <rsalz@openssl.org>
-
Rich Salz authored
Was memset with wrong sizeof. Reviewed-by: Richard Levitte <levitte@openssl.org> (cherry picked from commit 23b0fa5a)
-
- May 02, 2015
-
-
Gilles Khouzam authored
Signed-off-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (cherry picked from commit bed2edf1)
-
Rich Salz authored
Use sizeof *foo parameter, to avoid these errors. Reviewed-by: Viktor Dukhovni <viktor@openssl.org> (cherry picked from commit 53ba0a9e)
-
Hanno Böck authored
Signed-off-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Viktor Dukhovni <viktor@openssl.org> (cherry picked from commit 539ed89f)
-
- Apr 30, 2015
-
-
Matt Caswell authored
The problem occurs in EVP_PKEY_sign() when using RSA with X931 padding. It is only triggered if the RSA key size is smaller than the digest length. So with SHA512 you can trigger the overflow with anything less than an RSA 512 bit key. I managed to trigger a 62 byte overflow when using a 16 bit RSA key. This wasn't sufficient to cause a crash, although your mileage may vary. In practice RSA keys of this length are never used and X931 padding is very rare. Even if someone did use an excessively short RSA key, the chances of them combining that with a longer digest and X931 padding is very small. For these reasons I do not believe there is a security implication to this. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov <appro@openssl.org> (cherry picked from commit 34166d41)
-
Matt Caswell authored
Add a sanity check to the print_bin function to ensure that the |off| argument is positive. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov <appro@openssl.org> (cherry picked from commit 3deeeeb6)
-
Matt Caswell authored
Sanity check the |len| parameter to ensure it is positive. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov <appro@openssl.org> (cherry picked from commit cb0f400b)
-
Matt Caswell authored
The return value is checked for 0. This is currently safe but we should really check for <= 0 since -1 is frequently used for error conditions. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov <appro@openssl.org> (cherry picked from commit c427570e) Conflicts: ssl/ssl_locl.h
-
Matt Caswell authored
For SSLv3 the code assumes that |header_length| > |md_block_size|. Whilst this is true for all SSLv3 ciphersuites, this fact is far from obvious by looking at the code. If this were not the case then an integer overflow would occur, leading to a subsequent buffer overflow. Therefore I have added an explicit sanity check to ensure header_length is always valid. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov <appro@openssl.org> (cherry picked from commit 29b0a15a)
-
Matt Caswell authored
The static function dynamically allocates an output buffer if the output grows larger than the static buffer that is normally used. The original logic implied that |currlen| could be greater than |maxlen| which is incorrect (and if so would cause a buffer overrun). Also the original logic would call OPENSSL_malloc to create a dynamic buffer equal to the size of the static buffer, and then immediately call OPENSSL_realloc to make it bigger, rather than just creating a buffer than was big enough in the first place. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov <appro@openssl.org> (cherry picked from commit 9d9e3774)
-
Matt Caswell authored
There was already a sanity check to ensure the passed buffer length is not zero. Extend this to ensure that it also not negative. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov <appro@openssl.org> (cherry picked from commit b86d7dca)
-
Matt Caswell authored
The various implementations of EVP_CTRL_AEAD_TLS_AAD expect a buffer of at least 13 bytes long. Add sanity checks to ensure that the length is at least that. Also add a new constant (EVP_AEAD_TLS1_AAD_LEN) to evp.h to represent this length. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov <appro@openssl.org> (cherry picked from commit c8269881) Conflicts: ssl/record/ssl3_record.c
-
Matt Caswell authored
Add a sanity check to DES_enc_write to ensure the buffer length provided is not negative. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov <appro@openssl.org> (cherry picked from commit 873fb39f)
-
- Apr 29, 2015
-
-
Matt Caswell authored
Fortify flagged up a problem in n_do_ssl_write() in SSLv2. Analysing the code I do not believe there is a real problem here. However the logic flows are complicated enough that a sanity check of |len| is probably worthwhile. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Rich Salz <rsalz@openssl.org>
-
Matt Caswell authored
This reverts commit 47daa155 . The above commit was backported to the 1.0.2 branch as part of backporting the alternative chain verify algorithm changes. However it has been pointed out (credit to Shigeki Ohtsu) that this is unnecessary in 1.0.2 as this commit is a work around for loop checking that only exists in master. Reviewed-by: Richard Levitte <levitte@openssl.org>
-
- Apr 27, 2015
-
-
Emilia Kasper authored
Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit 6038354c)
-
Emilia Kasper authored
Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit a4d5269e)
-
Emilia Kasper authored
Cosmetic, no real effect. Reviewed-by: Richard Levitte <levitte@openssl.org> (cherry picked from commit 4446044a)
-
Emilia Kasper authored
Thanks to Brian Smith for reporting these issues. Reviewed-by: Richard Levitte <levitte@openssl.org>
-
- Apr 24, 2015
-
-
Emilia Kasper authored
Thanks to Brian Smith for reporting these issues. Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit 53dd4ddf)
-
Emilia Kasper authored
Also add a few comments about constant-timeness. Thanks to Brian Smith for reporting this issue. Reviewed-by: Rich Salz <rsalz@openssl.org>
-
- Apr 22, 2015
-
-
Loganaden Velvindron authored
The function CRYPTO_strdup (aka OPENSSL_strdup) fails to check the return value from CRYPTO_malloc to see if it is NULL before attempting to use it. This patch adds a NULL check. RT3786 Signed-off-by: Matt Caswell <matt@openssl.org> (cherry picked from commit 37b0cf936744d9edb99b5dd82cae78a7eac6ad60) Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit 20d21389c8b6f5b754573ffb6a4dc4f3986f2ca4)
-
- Apr 21, 2015
-
-
Emilia Kasper authored
EAP-FAST session resumption relies on handshake message lookahead to determine server intentions. Commits 980bc1ec and 7b3ba508 removed the lookahead so broke session resumption. This change partially reverts the commits and brings the lookahead back in reduced capacity for TLS + EAP-FAST only. Since EAP-FAST does not support regular session tickets, the lookahead now only checks for a Finished message. Regular handshakes are unaffected by this change. Reviewed-by: David Benjamin <davidben@chromium.org> Reviewed-by: Matt Caswell <matt@openssl.org> (cherry picked from commit 6e3d0153)
-
Emilia Kasper authored
newsig may be used (freed) uninitialized on a malloc error. Reviewed-by: Rich Salz <rsalz@openssl.org> (cherry picked from commit 68249414)
-
Emilia Kasper authored
Reviewed-by: Dr. Stephen Henson <steve@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org>
-
- Apr 20, 2015
-
-
Richard Levitte authored
The logic with how 'ok' was calculated didn't quite convey what's "ok", so the logic is slightly redone to make it less confusing. Reviewed-by: Andy Polyakov <appro@openssl.org> (cherry picked from commit 06affe3d)
-
Matt Caswell authored
Filled in lots of return value checks that were missing the GOST engine, and added appropriate error handling. Reviewed-by: Richard Levitte <levitte@openssl.org> (cherry picked from commit 8817e2e0)
-
Matt Caswell authored
Fix miscellaneous NULL pointer derefs in the sureware engine. Reviewed-by: Richard Levitte <levitte@openssl.org> (cherry picked from commit 7b611e5f)
-