383 Commits

Author SHA1 Message Date
Matt Caswell
f55e2fa7b9 Fix the alert sent if no shared sig algs
We were sending illegal parameter. This isn't correct. The parameters are
legal, we just don't have an overlap. A more appropriate alert is
handshake failure.

Fixes #2919

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6010)
2018-04-20 11:40:06 +01:00
Matt Caswell
af2d06d245 Ignore the status_request extension in a resumption handshake
We cannot provide a certificate status on a resumption so we should
ignore this extension in that case.

Fixes #1662

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/5897)
2018-04-17 16:45:02 +01:00
Matt Caswell
e9d26dc852 Tolerate a Certificate using a non-supported group on server side
If a server has been configured to use an ECDSA certificate, we should
allow it regardless of whether the server's own supported groups list
includes the certificate's group.

Fixes #2033

Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/5607)
2018-03-28 15:19:22 +01:00
Matt Caswell
f520f134d1 Update copyright year
Reviewed-by: Richard Levitte <levitte@openssl.org>
2018-03-27 13:43:23 +01:00
Matt Caswell
5a19f9ea7a Sanity check the ticket length before using key name/IV
This could in theory result in an overread - but due to the over allocation
of the underlying buffer does not represent a security issue.

Thanks to Fedor Indutny for reporting this issue.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5415)
2018-02-21 11:22:52 +00:00
Matt Caswell
6e127fdd1c Add the SSL_OP_NO_RENEGOTIATION option to 1.1.0
This is based on a heavily modified version of commit db0f35dda by Todd
Short from the master branch.

We are adding this because it used to be possible to disable reneg using
the flag SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS in 1.0.2. This is no longer
possible because of the opacity work.

A point to note about this is that if an application built against new
1.1.0 headers (that know about the new option SSL_OP_NO_RENEGOTIATION
option) is run using an older version of 1.1.0 (that doesn't know about
the option) then the option will be accepted but nothing will happen, i.e.
renegotiation will not be prevented. There's probably not much we can do
about that.

Fixes #4739

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4901)
2018-01-30 19:31:35 +00:00
David Woodhouse
619c589bdb Add SSL_OP_NO_ENCRYPT_THEN_MAC
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit cde6145ba19a2fce039cf054a89e49f67c623c59)
2017-10-05 09:29:28 +02:00
Todd Short
e7f235768d Reorder extensions to put SigAlgs last
WebSphere application server cannot handle having an empty
extension (e.g. EMS/EtM) as the last extension in a client hello.
This moves the SigAlgs extension last (before any padding) for TLSv1.2
to avoid this issue.

Force the padding extension to a minimum length of 1.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3927)
2017-08-16 11:58:00 -05:00
Bernd Edlinger
2f0af52b42 Fix gcc-7 warnings about missing fall thru comments.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3936)
2017-07-14 18:35:36 +02:00
Matt Caswell
d717edf80e Don't fail the connection in SSLv3 if server selects ECDHE
ECDHE is not properly defined for SSLv3. Commit fe55c4a2 prevented ECDHE
from being selected in that protocol. However, historically, servers do
still select ECDHE anyway so that commit causes interoperability problems.
Clients that previously worked when talking to an SSLv3 server could now
fail.

This commit introduces an exception which enables a client to continue in
SSLv3 if the server selected ECDHE.

(cherry picked from commit 8af91fd9d08487e0dffb6ccac5f42633c964f3f0)

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3734)
2017-06-21 11:17:22 +01:00
Benjamin Kaduk
a504540fcb Allow an ALPN callback to pretend to not exist
RFC 7301 mandates that the server SHALL respond with a fatal
"no_application_protocol" alert when there is no overlap between
the client's supplied list and the server's list of supported protocols.
In commit 062178678f5374b09f00d70796f6e692e8775aca we changed from
ignoring non-success returns from the supplied alpn_select_cb() to
treating such non-success returns as indicative of non-overlap and
sending the fatal alert.

In effect, this is using the presence of an alpn_select_cb() as a proxy
to attempt to determine whether the application has configured a list
of supported protocols.  However, there may be cases in which an
application's architecture leads it to supply an alpn_select_cb() but
have that callback be configured to take no action on connections that
do not have ALPN configured; returning SSL_TLSEXT_ERR_NOACK from
the callback would be the natural way to do so.  Unfortunately, the
aforementioned behavior change also treated SSL_TLSEXT_ERR_NOACK as
indicative of no overlap and terminated the connection; this change
supplies special handling for SSL_TLSEXT_ERR_NOACK returns from the
callback.  In effect, it provides a way for a callback to obtain the
behavior that would have occurred if no callback was registered at
all, which was not possible prior to this change.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3158)
(cherry picked from commit 8313a787d770ac1d7ddafcbc41b13e7fb5841eae)
2017-04-11 13:46:13 +01:00
FdaSilvaYY
a2dcdb57af More typo fixes
Backport of 69687aa829bc8bdcaf5468eb3dd0ada13700b7aa
(Merged from #3069)

Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3079)
2017-03-30 22:13:12 +02:00
Bernd Edlinger
13ed1afa92 Avoid questionable use of the value of a pointer that refers to space
deallocated by a call to the free function in tls_decrypt_ticket.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2897)
2017-03-10 14:07:53 -05:00
Bernd Edlinger
8aa3878933 Add some more consistency checks in tls_decrypt_ticket.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2704)
(cherry picked from commit 79020b27beff060d02830870fdfd821fe8cbd439)
2017-02-22 09:43:21 -05:00
Matt Caswell
4ad93618d2 Don't change the state of the ETM flags until CCS processing
Changing the ciphersuite during a renegotiation can result in a crash
leading to a DoS attack. ETM has not been implemented in 1.1.0 for DTLS
so this is TLS only.

The problem is caused by changing the flag indicating whether to use ETM
or not immediately on negotiation of ETM, rather than at CCS. Therefore,
during a renegotiation, if the ETM state is changing (usually due to a
change of ciphersuite), then an error/crash will occur.

Due to the fact that there are separate CCS messages for read and write
we actually now need two flags to determine whether to use ETM or not.

CVE-2017-3733

Reviewed-by: Richard Levitte <levitte@openssl.org>
2017-02-16 09:39:06 +00:00
Bernd Edlinger
177b4225ba Use TLSEXT_KEYNAME_LENGTH in tls_decrypt_ticket.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2618)
(cherry picked from commit 57b0d651f052ed86528da916397acbcce035fb21)
2017-02-14 14:45:08 -05:00
Todd Short
ac162301b4 Majority rules, use session_ctx vs initial_ctx
session_ctx and initial_ctx are aliases of each other, and with the
opaque data structures, there's no need to keep both around. Since
there were more references of session_ctx, replace all instances of
initial_ctx with session_ctx.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2340)
2017-02-05 00:55:33 +01:00
Dr. Stephen Henson
3f60b8fbdc Use correct signature algorithm list when sending or checking.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2297)
2017-01-26 17:58:21 +00:00
Rich Salz
8accb70ee9 If client doesn't send curves list, don't assume all.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1597)
(cherry picked from commit 3e37351834c203421b7f492dd83d5e5872e17778)
2017-01-18 12:25:16 -05:00
Matt Caswell
5af2ad682e Implement length checks as a macro
Replace the various length checks in the extension code with a macro to
simplify the logic.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-10-28 09:40:55 +01:00
Matt Caswell
3ab5f981ed Ensure we have length checks for all extensions
The previous commit inspired a review of all the length checks for the
extension adding code. This adds more robust checks and adds checks where
some were missing previously. The real solution for this is to use WPACKET
which is currently in master - but that cannot be applied to release
branches.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-10-28 09:40:55 +01:00
Matt Caswell
8c9365a690 Fix length check writing status request extension
The status request extension did not correctly check its length, meaning
that writing the extension could go 2 bytes beyond the buffer size. In
practice this makes little difference because, due to logic in buffer.c the
buffer is actually over allocated by approximately 5k!

Issue reported by Guido Vranken.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-10-28 09:40:55 +01:00
David Woodhouse
8afb9742ae Disable encrypt_then_mac negotiation for DTLS.
I use the word 'negotiation' advisedly. Because that's all we were doing.
We negotiated it, set the TLS1_FLAGS_ENCRYPT_THEN_MAC flag in our data
structure, and then utterly ignored it in both dtls_process_record()
and do_dtls1_write().

Turn it off for 1.1.0; we'll fix it for 1.1.1 and by the time that's
released, hopefully 1.1.0b will be ancient history.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-10-20 09:59:41 +01:00
Matt Caswell
6d32c2ae28 Fix a mem leak in NPN handling
If a server sent multiple NPN extensions in a single ClientHello then a
mem leak can occur. This will only happen where the client has requested
NPN in the first place. It does not occur during renegotiation. Therefore
the maximum that could be leaked in a single connection with a malicious
server is 64k (the maximum size of the ServerHello extensions section). As
this is client side, only occurs if NPN has been requested and does not
occur during renegotiation this is unlikely to be exploitable.

Issue reported by Shi Lei.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-22 09:28:07 +01:00
Matt Caswell
a59ab1c4dd Fix OCSP Status Request extension unbounded memory growth
A malicious client can send an excessively large OCSP Status Request
extension. If that client continually requests renegotiation,
sending a large OCSP Status Request extension each time, then there will
be unbounded memory growth on the server. This will eventually lead to a
Denial Of Service attack through memory exhaustion. Servers with a
default configuration are vulnerable even if they do not support OCSP.
Builds using the "no-ocsp" build time option are not affected.

I have also checked other extensions to see if they suffer from a similar
problem but I could not find any other issues.

CVE-2016-6304

Issue reported by Shi Lei.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-09-22 09:28:07 +01:00
Dr. Stephen Henson
e97763c92c Sanity check ticket length.
If a ticket callback changes the HMAC digest to SHA512 the existing
sanity checks are not sufficient and an attacker could perform a DoS
attack with a malformed ticket. Add additional checks based on
HMAC size.

Thanks to Shi Lei for reporting this bug.

CVE-2016-6302

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
2016-08-23 23:16:24 +01:00
Emilia Kasper
a230b26e09 Indent ssl/
Run util/openssl-format-source on ssl/

Some comments and hand-formatted tables were fixed up
manually by disabling auto-formatting.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-08-18 14:02:29 +02:00
Dr. Stephen Henson
ec24630ae2 Modify TLS support for new X25519 API.
When handling ECDH check to see if the curve is "custom" (X25519 is
currently the only curve of this type) and instead of setting a curve
NID just allocate a key of appropriate type.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-08-13 14:11:05 +01:00
Emilia Kasper
70c22888c1 Fix two bugs in clienthello processing
- Always process ALPN (previously there was an early return in the
  certificate status handling)
- Don't send a duplicate alert. Previously, both
  ssl_check_clienthello_tlsext_late and its caller would send an
  alert. Consolidate alert sending code in the caller.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-07-19 14:18:03 +02:00
FdaSilvaYY
9d22666eb8 Spelling
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)
2016-06-29 09:56:39 -04:00
FdaSilvaYY
f430ba31ac Spelling... and more spelling
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1245)
2016-06-22 00:26:10 +02:00
FdaSilvaYY
823146d65f Useless header include of openssl/rand.h
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1168)
2016-06-18 16:30:24 -04:00
Matt Caswell
e7653f3bab Fix comment
Fix a comment following commit c2c49969e23605.

RT2388

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-06-14 10:41:00 +01:00
Todd Short
5c753de668 Fix session ticket and SNI
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)
2016-06-09 13:07:51 -04:00
Dr. Stephen Henson
77ab2b0193 remove encrypt then mac ifdefs
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-05-23 19:15:20 +01:00
Rich Salz
846e33c729 Copyright consolidation 01/10
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
2016-05-17 14:19:19 -04:00
TJ Saunders
05df5c2036 Use AES256 for the default encryption algoritm for TLS session tickets
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
2016-05-16 20:43:06 +02:00
TJ Saunders
4e2e1ec9d5 session tickets: Use sizeof() for the various fields
Signed-off-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matt Caswell <matt@openssl.org>

GH: #515, MR: #2153
2016-05-16 20:42:21 +02:00
Kazuki Yamaguchi
b04f947941 Fix NPN protocol name list validation
Since 50932c4 "PACKETise ServerHello processing",
ssl_next_proto_validate() incorrectly allows empty protocol name.
draft-agl-tls-nextprotoneg-04[1] says "Implementations MUST ensure that
the empty string is not included and that no byte strings are
truncated."
This patch restores the old correct behavior.

[1] https://tools.ietf.org/html/draft-agl-tls-nextprotoneg-04

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
2016-05-16 11:45:25 +01:00
David Benjamin
f7aa318552 Don't send signature algorithms when client_version is below TLS 1.2.
Per RFC 5246,

    Note: this extension is not meaningful for TLS versions prior to 1.2.
    Clients MUST NOT offer it if they are offering prior versions.
    However, even if clients do offer it, the rules specified in [TLSEXT]
    require servers to ignore extensions they do not understand.

Although second sentence would suggest that there would be no interop
problems in always offering the extension, WebRTC has reported issues
with Bouncy Castle on < TLS 1.2 ClientHellos that still include
signature_algorithms. See also
https://bugs.chromium.org/p/webrtc/issues/detail?id=4223

RT#4390

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Stephen Henson <steve@openssl.org>
2016-05-09 17:46:23 +01:00
FdaSilvaYY
8483a003bf various spelling fixes
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/952)
2016-04-28 14:22:26 -04:00
Matt Caswell
5951e840d9 Fix no-ocsp on Windows (and probably VMS)
The ocsp.h file did not have appropriate guards causing link failures on
Windows.

GH Issue 900

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-04-21 17:03:02 +01:00
Rich Salz
9021a5dfb3 Rename some lowercase API's
Make OBJ_name_cmp internal
Rename idea_xxx to IDEA_xxx
Rename get_rfc_xxx to BN_get_rfc_xxx
Rename v3_addr and v3_asid functions to X509v3_...

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-04-18 08:22:00 -04:00
Matt Caswell
0aeddcfa61 Make DH opaque
Move the dh_st structure into an internal header file and provide
relevant accessors for the internal fields.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-04-09 10:10:55 +01:00
Matt Caswell
3e41ac3528 Fix no-ocsp
Misc fixes for no-ocsp

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-04-06 14:57:45 +01:00
Rich Salz
e771eea6d8 Revert "various spelling fixes"
This reverts commit 620d540bd47a96fb6905fbbdd8ea5167a8841a3e.
It wasn't reviewed.

Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-04-04 16:11:43 -04:00
FdaSilvaYY
620d540bd4 various spelling fixes
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-04-04 15:06:32 -04:00
Todd Short
0351baae36 Fix ALPN - more fixes
* Clear proposed, along with selected, before looking at ClientHello
* Add test case for above
* Clear NPN seen after selecting ALPN on server
* Minor documentation updates

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
2016-03-20 21:09:32 -04:00
Rich Salz
3c27208fab Remove #error from include files.
Don't have #error statements in header files, but instead wrap
the contents of that file in #ifndef OPENSSL_NO_xxx
This means it is now always safe to include the header file.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-03-20 19:48:36 -04:00
Matt Caswell
d1247df266 Ensure that memory allocated for the ticket is freed
If a call to EVP_DecryptUpdate fails then a memory leak could occur.
Ensure that the memory is freed appropriately.

Issue reported by Guido Vranken.

Reviewed-by: Richard Levitte <levitte@openssl.org>
2016-03-18 11:52:22 +00:00