The siglen value needs to be initialised prior to it being read in the
call to EVP_DigestSignFinal later in this function.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
TLSv1.3 introduces PSS based sigalgs. Offering these in a TLSv1.3 client
implies that the client is prepared to accept these sigalgs even in
TLSv1.2.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
TLSv1.3 freezes the record layer version and ensures that it is always set
to TLSv1.0. Some implementations check this.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
In TLSv1.3 we must use PSS based sig algs for RSA signing. Ignore any
shared sig algs which are PKCS1 based.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
A misreading of the TLS1.3 spec meant we were using the handshake hashes
up to and including the Client Finished to calculate the client
application traffic secret. We should be only use up until the Server
Finished.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
We need to use the length of the handshake hash for the length of the
finished key.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
We were not incrementing the sequence number every time we sent/received
a record.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
The sigalgs work has made some old lookup tables and functions redundant
so remove them.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
We had an extra layer of indirection in looking up hashes and sigs based
on sigalgs which is now no longer necessary. This removes it.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
In TLSv1.2 an individual sig alg is represented by 1 byte for the hash
and 1 byte for the signature. In TLSv1.3 each sig alg is represented by
two bytes, where the two bytes together represent a single hash and
signature combination. This converts the internal representation of sigalgs
to use a single int for the pair, rather than a pair of bytes.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
Otherwise the client will try to process it again. The second time around
it will try and move the record data into handshake fragment storage and
realise that there is no data left. At that point it marks it as read
anyway. However, it is a bug that we go around the loop a second time, so
we prevent that.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2200)
This variable represents the index of the cert within the chain, so give it
a name that better represents that.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2020)
We remove the separate CertificateStatus message for TLSv1.3, and instead
send back the response in the appropriate Certificate message extension.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2020)
Also updates TLSProxy to be able to understand the format and parse the
contained extensions.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2020)
Continuing from the previous commit we also need to extend the extensions
framework to supply the Certificate we just read during parsing.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2020)
The Certificate message in TLS1.3 has an extensions block for each
Certificate. Therefore we need to extend tls_construct_extensions() to pass
in the certificate we are working on. We also pass in the position in the
chain (with 0 being the first certificate).
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2020)
Previously we were omitting the extension information from ext_defs if
the association no- option was defined. This doesn't work because the
indexes into the table are no longer valid.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2153)
This should be sent in the ServerHello if a EC based ciphersuite is
negotiated. The relevant flag to do this was missed off in the recent
extensions refactor.
Fixes GitHub Issue #2133
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2153)
SSL_clear() was resetting numwpipes to 0, but not freeing any allocated
memory for existing write buffers.
Fixes#2026
Reviewed-by: Rich Salz <rsalz@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Travis was indicating a bogus uninit var warning. This fixes it.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Various functions got renamed. We need to rename the error codes too.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
The existing code used the magic number -1 to represent the absence of
a status_type in the extension. This commit replaces it with a macro.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
When we call tls_collect_extensions() we need to free up the raw
extensions data later.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
In TLS1.3 some ServerHello extensions remain in the ServerHello, while
others move to the EncryptedExtensions message. This commit performs that
move.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Because extensions were keyed by type which is sparse, we were continually
scanning the list to find the one we wanted. The way we stored them also
had the side effect that we were running initialisers/finalisers in a
different oder to the parsers. In this commit we change things so that we
instead key on an index value for each extension.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Remove some functions that are no longer needed now that we have the new
extension framework.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
The _clienthello_ in the extensions parsing functions is overly specific.
Better to keep the convention to just _client_
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
This lays the foundation for a later move to have the extensions built and
placed into the correct message for TLSv1.3 (e.g. ServerHello or
EncryptedExtensions).
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Later we will have client extensions code too.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Add support for construction of extensions
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
This builds on the work started in 1ab3836b3 and extends is so that
each extension has its own identified parsing functions, as well as an
allowed context identifying which messages and protocols it is relevant for.
Subsequent commits will do a similar job for the ServerHello extensions.
This will enable us to have common functions for processing extension blocks
no matter which of the multiple messages they are received from. In TLSv1.3
a number of different messages have extension blocks, and some extensions
have moved from one message to another when compared to TLSv1.2.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Subsequent commits will pull other extensions code into this file.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
At this stage the message is just empty. We need to fill it in with
extension data.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
There are some minor differences in the format of a ServerHello in TLSv1.3.
Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Improves the readability of the code, and reduces the liklihood of errors.
Also made a few minor style changes.
Reviewed-by: Rich Salz <rsalz@openssl.org>
At the moment the msg callback only received the record header with the
outer record type in it. We never pass the inner record type - we probably
need to at some point.
Reviewed-by: Rich Salz <rsalz@openssl.org>
This updates the record layer to use the TLSv1.3 style nonce construciton.
It also updates TLSProxy and ossltest to be able to recognise the new
layout.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Calling SSL_set_accept_state() after DTLSv1_listen() clears the state, so
SSL_accept() no longer works. In 1.0.2 calling DTLSv1_listen() would set
the accept state automatically. We should still do that.
Fixes#1989
Reviewed-by: Andy Polyakov <appro@openssl.org>
Verify that the encrypt-then-mac negotiation is handled
correctly. Additionally, when compiled with no-asm, this test ensures
coverage for the constant-time MAC copying code in
ssl3_cbc_copy_mac. The proxy-based CBC padding test covers that as
well but it's nevertheless better to have an explicit handshake test
for mac-then-encrypt.
Reviewed-by: Andy Polyakov <appro@openssl.org>
The SSL_IS_TLS13() macro wasn't quite right. It would come back with true
in the case where we haven't yet negotiated TLSv1.3, but it could be
negotiated.
Reviewed-by: Rich Salz <rsalz@openssl.org>
There is a set of miscellaneous processing for OCSP, CT etc at the end of
the ServerDone processing. In TLS1.3 we don't have a ServerDone, so this
needs to move elsewhere.
Reviewed-by: Rich Salz <rsalz@openssl.org>
This is a major overhaul of the TLSv1.3 state machine. Currently it still
looks like TLSv1.2. This commit changes things around so that it starts
to look a bit less like TLSv1.2 and bit more like TLSv1.3.
After this commit we have:
ClientHello
+ key_share ---->
ServerHello
+key_share
{CertificateRequest*}
{Certificate*}
{CertificateStatus*}
<---- {Finished}
{Certificate*}
{CertificateVerify*}
{Finished} ---->
[ApplicationData] <---> [Application Data]
Key differences between this intermediate position and the final TLSv1.3
position are:
- No EncryptedExtensions message yet
- No server side CertificateVerify message yet
- CertificateStatus still exists as a separate message
- A number of the messages are still in the TLSv1.2 format
- Still running on the TLSv1.2 record layer
Reviewed-by: Rich Salz <rsalz@openssl.org>
TLSv1.3 has a NewSessionTicket message, but it is *completely* different to
the TLSv1.2 one and may as well have been called something else. This commit
removes the old style NewSessionTicket from TLSv1.3. We will have to add the
new style one back in later.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Commit 94ed2c6 dropped a ! operator by mistake, which causes extended
master secret connections to fail. This puts in back.
Reviewed-by: Richard Levitte <levitte@openssl.org>
ssl_test_old was reaching inside the SSL structure and changing the internal
BIO values. This is completely unneccessary, and was causing an abort in the
test when enabling TLSv1.3.
I also removed the need for ssl_test_old to include ssl_locl.h. This
required the addition of some missing accessors for SSL_COMP name and id
fields.
Reviewed-by: Rich Salz <rsalz@openssl.org>
The previous commits put in place the logic to exchange key_share data. We
now need to do something with that information. In <= TLSv1.2 the equivalent
of the key_share extension is the ServerKeyExchange and ClientKeyExchange
messages. With key_share those two messages are no longer necessary.
The commit removes the SKE and CKE messages from the TLSv1.3 state machine.
TLSv1.3 is completely different to TLSv1.2 in the messages that it sends
and the transitions that are allowed. Therefore, rather than extend the
existing <=TLS1.2 state transition functions, we create a whole new set for
TLSv1.3. Intially these are still based on the TLSv1.2 ones, but over time
they will be amended.
The new TLSv1.3 transitions remove SKE and CKE completely. There's also some
cleanup for some stuff which is not relevant to TLSv1.3 and is easy to
remove, e.g. the DTLS support (we're not doing DTLSv1.3 yet) and NPN.
I also disable EXTMS for TLSv1.3. Using it was causing some added
complexity, so rather than fix it I removed it, since eventually it will not
be needed anyway.
Reviewed-by: Rich Salz <rsalz@openssl.org>
This is a temporary fix for while we are still using the old session
resumption logic in the TLSv1.3 code. Due to differences in EXTMS support
we can't resume a <=TLSv1.2 session in a TLSv1.3 connection (the EXTMS
consistency check causes the connection to abort). This causes test
failures.
Ultimately we will rewrite the session resumption logic for TLSv1.3 so this
problem will go away. But until then we need a quick fix to keep the tests
happy.
Reviewed-by: Rich Salz <rsalz@openssl.org>
At the moment the server doesn't yet do anything with this information.
We still need to send the server's key_share info back to the client. That
will happen in subsequent commits.
Reviewed-by: Rich Salz <rsalz@openssl.org>
In this commit we just generate the extension on the client side, but don't
yet do anything with it. Subsequent commits, will add the server side
capability.
At the moment we hard code a single key_share. In the future we should make
this configurable.
Reviewed-by: Rich Salz <rsalz@openssl.org>
This is a skin deep change, which simply renames most places where we talk
about curves in a TLS context to groups. This is because TLS1.3 has renamed
the extension, and it can now include DH groups too. We still only support
curves, but this rename should pave the way for a future extension for DH
groups.
Reviewed-by: Rich Salz <rsalz@openssl.org>
We can end up with a NULL SSL_METHOD function if a method has been
disabled. If that happens then we shouldn't call vent->smeth().
Reviewed-by: Rich Salz <rsalz@openssl.org>
Nothing is using this yet, it just adds the underlying functions necesary
for generating the TLS1.3 secrets.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Normally WPACKETs will use a BUF_MEM which can grow as required. Sometimes
though that may be overkill for what is needed - a static buffer may be
sufficient. This adds that capability.
Reviewed-by: Rich Salz <rsalz@openssl.org>
There were a few places where they could be declared const so this commit
does that.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
The name and type of the argument to ssl_check_for_safari() has changed.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
The size if fixed by the protocol and won't change even if
sizeof(clienthello.random) does.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
Add a blank line, take one away - due to feedback received during review.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
We should be freeing up the raw extension data after we've finished with it.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
In the case of an SSLv2 compat ClientHello we weren't setting up the
compression methods correctly, which could lead to uninit reads or crashes.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
This partially reverts commit c636c1c47. It also tweaks the documentation
and comments in this area. On the client side the documented interface for
SSL_CTX_set_verify()/SSL_set_verify() is that setting the flag
SSL_VERIFY_PEER causes verfication of the server certificate to take place.
Previously what was implemented was that if *any* flag was set then
verification would take place. The above commit improved the semantics to
be as per the documented interface.
However, we have had a report of at least one application where an
application was incorrectly using the interface and used *only*
SSL_VERIFY_FAIL_IF_NO_PEER_CERT on the client side. In OpenSSL prior to
the above commit this still caused verification of the server certificate
to take place. After this commit the application silently failed to verify
the server certificate.
Ideally SSL_CTX_set_verify()/SSL_set_verify() could be modified to indicate
if invalid flags were being used. However these are void functions!
The simplest short term solution is to revert to the previous behaviour
which at least means we "fail closed" rather than "fail open".
Thanks to Cory Benfield for reporting this issue.
Reviewed-by: Richard Levitte <levitte@openssl.org>
We read it later in grow_init_buf(). If CCS is the first thing received in
a flight, then it will use the init_msg from the last flight we received. If
the init_buf has been grown in the meantime then it will point to some
arbitrary other memory location. This is likely to result in grow_init_buf()
attempting to grow to some excessively large amount which is likely to
fail. In practice this should never happen because the only time we receive
a CCS as the first thing in a flight is in an abbreviated handshake. None
of the preceding messages from the server flight would be large enough to
trigger this.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Travis is reporting one file at a time shadowed variable warnings where
"read" has been used. This attempts to go through all of libssl and replace
"read" with "readbytes" to fix all the problems in one go.
Reviewed-by: Rich Salz <rsalz@openssl.org>
We also modify the SSL_get_error() function to handle the fact that with
SSL_write_ex() the error return is 0 not -1, and fix some bugs in the
SSL BIO reading.
Reviewed-by: Rich Salz <rsalz@openssl.org>
TLS1.0 and TLS1.1 say you SHOULD ignore unrecognised record types, but
TLS 1.2 says you MUST send an unexpected message alert. We swap to the
TLS 1.2 behaviour for all protocol versions to prevent issues where no
progress is being made and the peer continually sends unrecognised record
types, using up resources processing them.
Issue reported by 郭志攀
Reviewed-by: Tim Hudson <tjh@openssl.org>
The function ssl3_read_n() takes a parameter |clearold| which, if set,
causes any old data in the read buffer to be forgotten, and any unread data
to be moved to the start of the buffer. This is supposed to happen when we
first read the record header.
However, the data move was only taking place if there was not already
sufficient data in the buffer to satisfy the request. If read_ahead is set
then the record header could be in the buffer already from when we read the
preceding record. So with read_ahead we can get into a situation where even
though |clearold| is set, the data does not get moved to the start of the
read buffer when we read the record header. This means there is insufficient
room in the read buffer to consume the rest of the record body, resulting in
an internal error.
This commit moves the |clearold| processing to earlier in ssl3_read_n()
to ensure that it always takes place.
Reviewed-by: Richard Levitte <levitte@openssl.org>
We add ssl_cipher_get_overhead() as an internal function, to avoid
having too much ciphersuite-specific knowledge in DTLS_get_data_mtu()
itself. It's going to need adjustment for TLSv1.3... but then again, so
is fairly much *all* of the SSL_CIPHER handling. This bit is in the noise.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
When matching a ciphersuite if we are given an id, make sure we use it
otherwise we will match another ciphersuite which is identical except for
the TLS version.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Includes addition of the various options to s_server/s_client. Also adds
one of the new TLS1.3 ciphersuites.
This isn't "real" TLS1.3!! It's identical to TLS1.2 apart from the protocol
and the ciphersuite...and the ciphersuite is just a renamed TLS1.2 one (not
a "real" TLS1.3 ciphersuite).
Reviewed-by: Rich Salz <rsalz@openssl.org>
For convenience, combine getting a new ref for the new SSL_CTX
with assigning the store and freeing the old one.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1755)
