| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
| |
|
|
|
|
|
|
|
| |
the session id in case the copied session id is shorter than
SSL_MAX_SESSION_ID_LENGTH.
long standing bug pointed out by jsing
|
|
|
|
| |
get_session_cb is completed.
|
|
|
|
| |
the function name, document alert and make it fit into 80 columns.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In case the session ticket was empty or missing, an attempt is made to
retrieve the session from the internal cache or via a callback. This
code can easily be flattened a bit and factored into two functions. I
decided to wrap those into a third function to make the call from the
switch easier on the eye.
I could have kept the try_session_cache flag, but it now seems rather
pointless and awkwardly named anyway, so I took its negation and named
it ticket_decrypted.
To top things off, a little bit of polish in the exit path.
ok beck inoguchi jsing (with the usual healthy dose of nits)
|
|
|
|
|
|
|
|
|
|
|
|
| |
ssl_get_prev_session() hands the session id down to tls_decrypt_ticket()
which then copies it into the session pointer that it is about to return.
It's a lot simpler to retrieve the session pointer and copy the session id
inside ssl_get_prev_session().
Also, 'goto err' directly in TLS1_TICKET_NOT_DECRYPTED instead of skipping
a couple of long if clauses before doing so.
ok inoguchi jsing
|
| |
|
| |
|
| |
|
|
|
|
|
|
| |
ret is a confusing name for a pointer in a function that returns int.
ret is only returned in the sense that it ultimately replaces the current
s->session on success.
|
|
|
|
|
|
|
|
|
| |
The only path that sets TLS1_TICKET_NOT_DECRPYTED is through this label
and the ERR_clear_error() is called conditionally on this. We clear the
errors to make decrypt errors non-fatal. The free functions should not
set the errors and if they do, we don't want to hide that.
discussed with jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
tls1_process_ticket() - the only caller of tls_decrypt_ticket() - ends
in a switch over the return value of tls_decrypt_ticket() to decide
whether or not to set s->internal->tlsext_ticket_expected = 1.
Since tls_decrypt_ticket() already knows what it will return and
partly bases its decision on what to return on whether or not the
ticket needs to be renewed, it can also take care of setting this flag.
This way we don't need to have a confusing switch that conflates some
return values and sets this flag. Moreover, we can get rid of the ugly
TLS1_TICKET_DECRYPTED_RENEW whose only purpose is to signal that the
flag should be set.
ok jsing
|
|
|
|
|
|
|
|
| |
In tls1_process_ticket() and tls_decrypt_ticket() use #defines with
descriptive names instead of hardcoding -1 1 2 3 4 and occasionally
explaining the magic numbers with comments.
ok beck inoguchi
|
|
|
|
|
|
|
|
|
|
|
|
| |
ssl_get_prev_session() can fail for various reasons some of which
may be internal_error others decode_error alerts. Propagate the
appropriate alert up to the caller so we can abort the handshake
by sending a fatal alert instead of rudely closing the pipe.
Currently only 28 of 292 test cases of tlsfuzzer's test-extension.py pass.
With this diff, 272 pass. The rest will require fixes elsewhere.
ok beck inoguchi jsing
|
|
|
|
|
|
|
|
|
|
| |
This takes the same design/approach used in TLSv1.3 and provides an
opaque struct that is self contained and cannot reach back into other
layers. For now this just implements/replaces the writing of records
for DTLSv1/TLSv1.0/TLSv1.1/TLSv1.2. In doing so we stop copying the
plaintext into the same buffer that is used to transmit to the wire.
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
|
| |
When record protection is engaged, the plaintext must be followed by a
non-zero content type and optional zero padding. If the plaintext is zero
length or only consists of zero bytes then it is not a valid message,
since the content type is unspecified.
ok tb@
|
|
|
|
| |
ok inoguchi@ tb@
|
|
|
|
| |
ok inoguchi@ tb@
|
|
|
|
| |
ok inoguchi@ tb@
|
|
|
|
|
|
| |
The error path does the same as the currently duplicated code.
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
| |
If a peer sends a bogus record consisting of all-zero plaintext,
the content_len would be decremented to -1 and cause a crash in
freezero.
ok inoguchi jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A certain VPN provider appears to have configured their servers to only
accept P-521 for TLSv1.3 key exchange. The particular VPN software in use
also does not currently allow for the TLSv1.3 key share groups to be
configured, which means that there is no way to easily use LibreSSL in
this situation.
Include P-521 in the list of curves that are supported by default in the
client, in order to increase interoperability.
Discussed at length with beck@, inoguchi@ and tb@.
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously we used CBB to build the record headers, but not the entire
record. Use CBB_init_fixed() upfront, then build the record header and
add space for the record content. However, in order to do this we need
to determine the length of the record upfront.
This simplifies the code, removes a number of manual bounds checks and
makes way for further improvements.
ok inoguchi@ tb@
|
|
|
|
| |
ok inoguchi@ tb@
|
|
|
|
| |
ok jsing@ tb@
|
|
|
|
|
|
|
|
| |
A client should only send a status_request as part of the CH.
Pointed out by Michael Forney
ok inoguchi jsing
|
|
|
|
|
|
|
|
|
|
| |
The current code might cause a client to send a status_request
containing a CertificateStatusRequest with its certificate. This
makes no sense.
Pointed out by Michael Forney
ok inoguchi jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
According to RFC 8446, 4.4.2.1, a server may request that a client
present an OCSP response with its certificate by sending an empty
status_request extension as part of the certificate request. The
current code expects a full CertificateStatus structure, which is
only sent if the server sends an OCSP response with its certificate.
This causes interoperability issues with Go's TLS server and with
newer GnuTLS where we would abort the handshake with a decode_error
alert and length mismatch error.
Issue reported and diagnosed by Michael Forney
Problem also found by Mikolaj Kucharski and inoguchi.
ok inoguchi jsing
|
|
|
|
|
|
|
|
|
| |
The write path can return a failure in the AEAD path and there is no reason
not to check a return value.
Spotted by tb@ during another review.
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
| |
- Make the DTLS code much more consistent with the ssl3 code.
- Avoid assigning wr->input and wr->length just so they can be used as
arguments to memcpy().
- Remove the arc4random_buf() call for the explicit IV, since tls1_enc()
already does this for us.
ok tb@
|
|
|
|
|
|
| |
ssl3_create_record().
ok tb@
|
|
|
|
|
|
| |
ourselves.
Spotted by tb@ during a previous review.
|
|
|
|
|
|
|
|
|
|
|
| |
This will allow for further changes to be made with less complexity and
easier review.
In particular, decide if we need an empty fragment early on and only do
the alignment calculation once (rather than in two separate parts of the
function.
ok tb@ inoguchi@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As abieber@ found the hard way, some python frameworks (twisted, synapse)
thought it a great idea to use the info callback mechanism (designed to
get state information about SSL objects) to modify state information such
as setting and verifying the SNI. The switch of TLS_method() to default
to TLSv1.3 broke these contraptions. Further bits of the info callback
mechanism will likely metastasize throughout the TLSv1.3 stack if we
need them, so we only do what's really necessary now.
Lots of debugging, crucial hint and testing by abieber
input & ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Both Perl's HTTP::Tiny and IO::Socket::SSL know about SSL_MODE_AUTO_RETRY
and try to work around the fact that OpenSSL enabled it by default.
However, this can lead to the mode being disabled prior to the TLSv1.3
handshake and then enabled after the handshake has completed.
In order to handle this correctly we have to check the mode and inform the
record layer prior to every read.
Issue reported and test case provided by Nathanael Rensen
<nathanael@polymorpheus.com>.
ok inoguchi@ tb@
|
|
|
|
| |
ok inoguchi@ tb@
|
|
|
|
|
|
|
|
| |
This is no longer necessary since the TLS_method() now supports TLSv1.3.
Reverts r1.211 of ssl_lib.c.
ok beck@ inoguchi@ tb@
|
|
|
|
|
|
| |
ssl_version is completely unused and get_timeout is the same everywhere.
ok beck@ inoguchi@ tb@
|
|
|
|
|
|
| |
This can be done now that we have both TLSv1.3 client and server.
ok beck@ inoguchi@ tb@
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Some TLS extensions need to be treated differently depending on the
handshake message they appear in. Over time, various workarounds and
hacks were used to deal with the unavailability of the message type
in these functions, but this is getting fragile and unwieldy. Having
the message type available will enable us to clean this code up and
will allow simple fixes for a number of bugs in our handling of the
status_request extension reported by Michael Forney.
This approach was suggested a while ago by jsing.
ok beck jsing
|
|
|
|
|
|
|
|
| |
Move is_server and msg_type right after the SSL object so that CBS
and CBB and alert come last. This brings these functions more in
line with other internal functions and separates state from data.
requested by jsing
|
|
|
|
| |
to match the order they are listed in the RFC. No functional change.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When first called, queue and send a close notify, before returning 0 or 1
to indicate if a close notify has already been received from the peer. If
called again only attempt to read a close notify if there is no pending
application data and only read one record from the wire. In particular,
this avoids continuing to read application data where the peer continues
to send application data.
Issue noted by naddy@ with ftp(1).
ok jca@ tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
RFC 8446 section 9.2 imposes some requirements on the extensions sent
in the ClientHello: key_share and supported_groups must either both be
present or both be absent. If no pre_shared_key was sent, the CH must
contain both signature_algorithms and supported_groups. If either of
these conditions is violated, servers must abort the handshake with a
missing_extensions alert. Add a function that enforces this. If we are
going to enforce that clients send an SNI, we can also do this in this
function.
Fixes failing test case in tlsfuzzer's test-tls13-keyshare-omitted.py
ok beck inoguchi jsing
|
|
|
|
|
|
|
|
|
|
|
| |
missed a subsequent fix for an off-by-one in that code. If the first
byte of a CBC padding of length 255 is mangled, we don't detect that.
Adam Langley's BoringSSL commit 80842bdb44855dd7f1dde64a3fa9f4e782310fc7
Fixes the failing tlsfuzzer lucky 13 test case.
ok beck inoguchi
|
|
|
|
|
|
| |
how our tree gets built. If this was done in all the libraries (imagine
sys/dev), it would disrupt the development process hugely. So it should
not be done here either. use 'make includes' by hand instead.
|
|
|
|
|
|
|
|
| |
section 4.1.2 to ensure subsequent ClientHello messages after a
HelloRetryRequest messages must be unchanged from the initial
ClientHello.
ok tb@ jsing@
|
|
|
|
|
|
|
|
|
|
|
| |
IANA has allocated numbers for GOST ClientCertificateType. Use them in
addition to private values (left in place for compatibility).
Diff from Dmitry Baryshkov <dbaryshkov@gmail.com>
Sponsored by ROSA Linux
ok inoguchi@ tb@
|