summaryrefslogtreecommitdiffstats
path: root/ssl (follow)
Commit message (Collapse)AuthorAgeFilesLines
* Fix broken loading of client CAsAndreas Karlsson2016-07-021-2/+1
| | | | | | | | | | | The SSL_load_client_CA_file() failed to load any CAs due to an inccorrect assumption about the return value of lh_*_insert(). The return value when inserting into a hash is the old value of the key. The bug was introduced in 3c82e437bb3af822ea13cd5a24bab0745c556246. Reviewed-by: Kurt Roeckx <kurt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/1279)
* Avoid an overflow in constructing the ServerKeyExchange messageMatt Caswell2016-07-011-0/+5
| | | | | | | | | | | | | We calculate the size required for the ServerKeyExchange message and then call BUF_MEM_grow_clean() on the buffer. However we fail to take account of 2 bytes required for the signature algorithm and 2 bytes for the signature length, i.e. we could overflow by 4 bytes. In reality this won't happen because the buffer is pre-allocated to a large size that means it should be big enough anyway. Addresses an OCAP Audit issue. Reviewed-by: Rich Salz <rsalz@openssl.org>
* Whitespace cleanup in ssl folderFdaSilvaYY2016-06-298-10/+10
| | | | | | Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Kurt Roeckx <kurt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/1264)
* SpellingFdaSilvaYY2016-06-291-1/+1
| | | | | | Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Kurt Roeckx <kurt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/1264)
* Ensure read records are marked as readMatt Caswell2016-06-271-1/+8
| | | | | | | | | | | In some situations (such as when we receive a fragment of an alert) we try to get the next packet but did not mark the current one as read, meaning that we got the same record back again - leading to an infinite loop. Found using the BoringSSL test suite. Reviewed-by: Andy Polyakov <appro@openssl.org>
* Add checks on sk_TYPE_push() returned resultFdaSilvaYY2016-06-234-18/+40
| | | | | Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
* Spelling... and more spellingFdaSilvaYY2016-06-226-9/+9
| | | | | Reviewed-by: Kurt Roeckx <kurt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/1245)
* Make RSA key exchange code actually constant-time.David Benjamin2016-06-211-15/+36
| | | | | | | | | | | | | | | | | | | | | | | Using RSA_PKCS1_PADDING with RSA_private_decrypt is inherently unsafe. The API requires writing output on success and touching the error queue on error. Thus, although the padding check itself is constant-time as of 294d1e36c2495ff00e697c9ff622856d3114f14f, and the logic after the decryption in the SSL code is constant-time as of adb46dbc6dd7347750df2468c93e8c34bcb93a4b, the API boundary in the middle still leaks whether the padding check succeeded, giving us our much-loved Bleichenbacher padding oracle. Instead, PKCS#1 padding must be handled by the caller which uses RSA_NO_PADDING, in timing-sensitive code integrated with the Bleichenbacher mitigation. Removing PKCS#1 padding in constant time is actually much simpler when the expected length is a constant (and if it's not a constant, avoiding a padding oracle seems unlikely), so just do it inline. Signed-off-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Rich Salz <rsalz@openssl.org> GH: #1222
* Useless header include of openssl/rand.hFdaSilvaYY2016-06-184-4/+0
| | | | | Reviewed-by: Kurt Roeckx <kurt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/1168)
* Deal with the consequences of constifying gettersRichard Levitte2016-06-152-2/+2
| | | | | Reviewed-by: Stephen Henson <steve@openssl.org> Reviewed-by: Emilia Käsper <emilia@openssl.org>
* Initialize the session_idKurt Roeckx2016-06-141-0/+2
| | | | | | | | | | | | | ssl_session_hash() always looks at the first 4 bytes, regardless of the length. A client can send a session id that's shorter, and the callback could also generate one that's shorter. So we make sure that the rest of the buffer is initliazed to 0 so that we always calculate the same hash. Found by tis-interpreter, also previously reported as RT #2871 Reviewed-by: Rich Salz <rsalz@openssl.org> MR: #2911
* Fix commentMatt Caswell2016-06-141-2/+1
| | | | | | | | Fix a comment following commit c2c49969e23605. RT2388 Reviewed-by: Rich Salz <rsalz@openssl.org>
* Add some missing return value checksMatt Caswell2016-06-131-1/+4
| | | | | | Some misc return value checks Reviewed-by: Rich Salz <rsalz@openssl.org>
* Ensure SSL_set_session clears the old session from cache if it is badMatt Caswell2016-06-131-19/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | SSL_clear() and SSL_free() will remove a session from the cache if it is considered "bad". However SSL_set_session() does not do this for the session it is replacing. SSL_clear() clears an SSL object ready for reuse. It does not clear the session though. This means that: SSL_clear(s) SSL_set_session(s, sess); and SSL_set_session(s, sess); SSL_clear(s); do not do the same thing, although logically you would expect that they would. The failure of SSL_set_session() to remove bad sessions from the cache should be considered a bug, so this commit fixes it. RT#597 Reviewed-by: Rich Salz <rsalz@openssl.org>
* Don't compare a just free()d pointerKurt Roeckx2016-06-111-1/+1
| | | | | | | | Found by tis-interpreter Reviewed-by: Rich Salz <rsalz@openssl.org> GH: #1173
* RT3720 Increment session miss counter properlyLaszlo Kovacs2016-06-101-0/+1
| | | | | Reviewed-by: Kurt Roeckx <kurt@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
* Fix session ticket and SNITodd Short2016-06-092-1/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | When session tickets are used, it's possible that SNI might swtich the SSL_CTX on an SSL. Normally, this is not a problem, because the initial_ctx/session_ctx are used for all session ticket/id processes. However, when the SNI callback occurs, it's possible that the callback may update the options in the SSL from the SSL_CTX, and this could cause SSL_OP_NO_TICKET to be set. If this occurs, then two bad things can happen: 1. The session ticket TLSEXT may not be written when the ticket expected flag is set. The state machine transistions to writing the ticket, and the client responds with an error as its not expecting a ticket. 2. When creating the session ticket, if the ticket key cb returns 0 the crypto/hmac contexts are not initialized, and the code crashes when trying to encrypt the session ticket. To fix 1, if the ticket TLSEXT is not written out, clear the expected ticket flag. To fix 2, consider a return of 0 from the ticket key cb a recoverable error, and write a 0 length ticket and continue. The client-side code can explicitly handle this case. Fix these two cases, and add unit test code to validate ticket behavior. Reviewed-by: Emilia Käsper <emilia@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/1098)
* Add some accessor API'sRich Salz2016-06-081-0/+5
| | | | | | | | | GH1098: Add X509_get_pathlen() (and a test) GH1097: Add SSL_is_dtls() function. Documented. Reviewed-by: Matt Caswell <matt@openssl.org>
* Always use session_ctx when removing a sessionTodd Short2016-06-085-7/+7
| | | | | | | | Sessions are stored on the session_ctx, which doesn't change after SSL_set_SSL_CTX(). Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
* Reject out of context empty recordsMatt Caswell2016-06-074-13/+29
| | | | | | | | | | | | Previously if we received an empty record we just threw it away and ignored it. Really though if we get an empty record of a different content type to what we are expecting then that should be an error, i.e. we should reject out of context empty records. This commit makes the necessary changes to achieve that. RT#4395 Reviewed-by: Andy Polyakov <appro@openssl.org>
* Fix pipelining bugMatt Caswell2016-06-071-0/+1
| | | | | | | The number of read pipelines should be reset in the event of reuse of an SSL object. Reviewed-by: Andy Polyakov <appro@openssl.org>
* Add SSL_CTX_get_tlsext_status_type()Matt Caswell2016-06-071-0/+3
| | | | Reviewed-by: Rich Salz <rsalz@openssl.org>
* Return the value of tlsext_status_type in the return not argMatt Caswell2016-06-071-2/+1
| | | | Reviewed-by: Rich Salz <rsalz@openssl.org>
* Add SSL_get_tlsext_status_type() methodAlessandro Ghedini2016-06-071-0/+5
| | | | | | | | The tlsext_status_type field in SSL is used by e.g. OpenResty to determine if the client requested the certificate status, but SSL is now opaque. Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
* RT3895: Remove fprintf's from SSL library.Rich Salz2016-06-042-5/+5
| | | | Reviewed-by: Richard Levitte <levitte@openssl.org>
* Handle a memory allocation failure in ssl3_init_finished_mac()Matt Caswell2016-06-036-9/+29
| | | | | | | | | The ssl3_init_finished_mac() function can fail, in which case we need to propagate the error up through the stack. RT#3198 Reviewed-by: Rich Salz <rsalz@openssl.org>
* Remove null check, per review feedback. Note this in the docs.TJ Saunders2016-05-311-2/+0
| | | | | Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/1135)
* Add an SSL_SESSION accessor for obtaining the protocol version number, withTJ Saunders2016-05-311-0/+7
| | | | | | | accompanying documentation. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/1135)
* return error in ct_move_scts()Dr. Stephen Henson2016-05-311-1/+1
| | | | Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
* Fix ssl_cert_set0_chain invalid pointerTodd Short2016-05-271-1/+1
| | | | | | | | When setting the certificate chain, if a certificate doesn't pass security checks, then chain may point to a freed STACK_OF(X509) Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
* Fix some suspect warnings on WindowsMatt Caswell2016-05-263-5/+6
| | | | | | | | Windows was complaining about a unary minus operator being applied to an unsigned type. It did seem to go on and do the right thing anyway, but the code does look a little suspect. This fixes it. Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
* The ssl3_digest_cached_records() function does not handle errors properlyMatt Caswell2016-05-261-5/+4
| | | | | | | | | | The ssl3_digest_cached_records() function was failing to handle errors that might be returned from EVP_DigestSignInit() and EVP_DigestSignUpdate(). RT#4180 Reviewed-by: Stephen Henson <steve@openssl.org>
* Remove unused error/function codes.Rich Salz2016-05-231-135/+45
| | | | | | | | Add script to find unused err/reason codes Remove unused reason codes. Remove entries for unused functions Reviewed-by: Matt Caswell <matt@openssl.org>
* remove encrypt then mac ifdefsDr. Stephen Henson2016-05-232-16/+0
| | | | Reviewed-by: Rich Salz <rsalz@openssl.org>
* Fix some malloc failure crashes on X509_STORE_CTX_set_ex_dataFdaSilvaYY2016-05-231-1/+3
| | | | | | | from BoringSSL 306ece31bcaaed49e0240a2e5555f8901ebb2d45 Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
* Rename lh_xxx,sk_xxx tp OPENSSL_{LH,SK}_xxxRich Salz2016-05-201-3/+3
| | | | | | | | | | | | Rename sk_xxx to OPENSSL_sk_xxx and _STACK to OPENSSL_STACK Rename lh_xxx API to OPENSSL_LH_xxx and LHASH_NODE to OPENSSL_LH_NODE Make lhash stuff opaque. Use typedefs for function pointers; makes the code simpler. Remove CHECKED_xxx macros. Add documentation; remove old X509-oriented doc. Add API-compat names for entire old API Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
* Ensure async IO works with new state machineMatt Caswell2016-05-201-1/+4
| | | | | | | | | | | | In the new state machine if using nbio and we get the header of a handshake message is one record with the body in the next, with an nbio event in the middle, then the connection was failing. This is because s->init_num was getting reset. We should only reset it after we have read the whole message. RT#4394 Reviewed-by: Andy Polyakov <appro@openssl.org>
* Tighten up logic around ChangeCipherSpec.David Benjamin2016-05-201-0/+10
| | | | | | | | | | | | | | | ChangeCipherSpec messages have a defined value. They also may not occur in the middle of a handshake message. The current logic will accept a ChangeCipherSpec with value 2. It also would accept up to three bytes of handshake data before the ChangeCipherSpec which it would discard (because s->init_num gets reset). Instead, require that s->init_num is 0 when a ChangeCipherSpec comes in. RT#4391 Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
* Simplify SSL BIO buffering logicMatt Caswell2016-05-204-40/+22
| | | | | | | | | | | | | | | | | | | | | | | The write BIO for handshake messages is bufferred so that we only write out to the network when we have a complete flight. There was some complexity in the buffering logic so that we switched buffering on and off at various points through out the handshake. The only real reason to do this was historically it complicated the state machine when you wanted to flush because you had to traverse through the "flush" state (in order to cope with NBIO). Where we knew up front that there was only going to be one message in the flight we switched off buffering to avoid that. In the new state machine there is no longer a need for a flush state so it is simpler just to have buffering on for the whole handshake. This also gives us the added benefit that we can simply call flush after every flight even if it only has one message in it. This means that BIO authors can implement their own buffering strategies and not have to be aware of the state of the SSL object (previously they would have to switch off their own buffering during the handshake because they could not rely on a flush being received when they really needed to write data out). This last point addresses GitHub Issue #322. Reviewed-by: Andy Polyakov <appro@openssl.org>
* When strict SCT fails record verification failureViktor Dukhovni2016-05-191-0/+17
| | | | | | | | | | | | | | | Since with SSL_VERIFY_NONE, the connection may continue and the session may even be cached, we should save some evidence that the chain was not sufficiently verified and would have been rejected with SSL_VERIFY_PEER. To that end when a CT callback returs failure we set the verify result to X509_V_ERR_NO_VALID_SCTS. Note: We only run the CT callback in the first place if the verify result is still X509_V_OK prior to start of the callback. RT #4502 Reviewed-by: Tim Hudson <tjh@openssl.org>
* Ensure verify error is set when X509_verify_cert() failsViktor Dukhovni2016-05-181-0/+10
| | | | | | | | | | | Set ctx->error = X509_V_ERR_OUT_OF_MEM when verificaiton cannot continue due to malloc failure. Also, when X509_verify_cert() returns <= 0 make sure that the verification status does not remain X509_V_OK, as a last resort set it it to X509_V_ERR_UNSPECIFIED, just in case some code path returns an error without setting an appropriate value of ctx->error. Reviewed-by: Richard Levitte <levitte@openssl.org>
* Copyright consolidation 01/10Rich Salz2016-05-1744-3693/+265
| | | | | Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Kurt Roeckx <kurt@openssl.org>
* Add a comment to explain the use of |num_recs|Matt Caswell2016-05-171-1/+9
| | | | | | | | In the SSLV2ClientHello processing code in ssl3_get_record, the value of |num_recs| will always be 0. This isn't obvious from the code so a comment is added to explain it. Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
* Use the current record offset in ssl3_get_recordMatt Caswell2016-05-171-1/+2
| | | | | | | | | | | | | | The function ssl3_get_record() can obtain multiple records in one go as long as we are set up for pipelining and all the records are app data records. The logic in the while loop which reads in each record is supposed to only continue looping if the last record we read was app data and we have an app data record waiting in the buffer to be processed. It was actually checking that the first record had app data and we have an app data record waiting. This actually amounts to the same thing so wasn't wrong - but it looks a bit odd because it uses the |rr| array without an offset. Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
* There is only one read bufferMatt Caswell2016-05-171-1/+1
| | | | | | | | | | | | Pipelining introduced the concept of multiple records being read in one go. Therefore we work with an array of SSL3_RECORD objects. The pipelining change erroneously made a change in ssl3_get_record() to apply the current record offset to the SSL3_BUFFER we are using for reading. This is wrong - there is only ever one read buffer. This reverts that change. In practice this should make little difference because the code block in question is only ever used when we are processing a single record. Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
* Fix some out of date commentsMatt Caswell2016-05-173-6/+6
| | | | | | | | | Fix various references to s3_clnt.c and s3_srvr.c which don't exist any more. GitHub Issue #765 Reviewed-by: Richard Levitte <levitte@openssl.org>
* session tickets: use more sizeofKurt Roeckx2016-05-163-15/+23
| | | | | | Reviewed-by: Matt Caswell <matt@openssl.org> MR: #2153
* Use AES256 for the default encryption algoritm for TLS session ticketsTJ Saunders2016-05-163-4/+4
| | | | | | | | | | This involves providing more session ticket key data, for both the cipher and the digest Signed-off-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Matt Caswell <matt@openssl.org> GH: #515, MR: #2153
* session tickets: Use sizeof() for the various fieldsTJ Saunders2016-05-164-17/+33
| | | | | | | Signed-off-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Matt Caswell <matt@openssl.org> GH: #515, MR: #2153
* Fold threads.h into crypto.h making API publicViktor Dukhovni2016-05-163-3/+2
| | | | | | Document thread-safe lock creation Reviewed-by: Richard Levitte <levitte@openssl.org>