Skip to content

TLS1.3 WIP #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ extern "C" {
# define OPENSSL_NO_SCTP
#endif
#ifndef OPENSSL_NO_SSL_TRACE
# define OPENSSL_NO_SSL_TRACE
#endif
#ifndef OPENSSL_NO_SSL3
# define OPENSSL_NO_SSL3
Expand Down
13 changes: 13 additions & 0 deletions deps/openssl/openssl/CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@
https://github.com/openssl/openssl/commits/ and pick the appropriate
release branch.


Changes between 1.1.1a and 1.1.1b [xx XXX xxxx]

*) Change the info callback signals for the start and end of a post-handshake
message exchange in TLSv1.3. In 1.1.1/1.1.1a we used SSL_CB_HANDSHAKE_START
and SSL_CB_HANDSHAKE_DONE. Experience has shown that many applications get
confused by this and assume that a TLSv1.2 renegotiation has started. This
can break KeyUpdate handling. Instead we now use
SSL_CB_POST_HANDSHAKE_START and SSL_CB_POST_HANDSHAKE_DONE. This could
break some applications that were expecting the old signals. However
without this KeyUpdate is not usable for many applications.
[Matt Caswell]

Changes between 1.1.1 and 1.1.1a [20 Nov 2018]

*) Timing vulnerability in DSA signature generation
Expand Down
14 changes: 5 additions & 9 deletions deps/openssl/openssl/doc/man3/SSL_CTX_set_info_callback.pod
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,13 @@ Callback has been called due to an alert being sent or received.

=item SSL_CB_HANDSHAKE_START

Callback has been called because a new handshake is started. In TLSv1.3 this is
also used for the start of post-handshake message exchanges such as for the
exchange of session tickets, or for key updates. It also occurs when resuming a
handshake following a pause to handle early data.
Callback has been called because a new handshake is started. It also occurs when
resuming a handshake following a pause to handle early data.

=item SSL_CB_HANDSHAKE_DONE 0x20
=item SSL_CB_HANDSHAKE_DONE

Callback has been called because a handshake is finished. In TLSv1.3 this is
also used at the end of an exchange of post-handshake messages such as for
session tickets or key updates. It also occurs if the handshake is paused to
allow the exchange of early data.
Callback has been called because a handshake is finished. It also occurs if the
handshake is paused to allow the exchange of early data.

=back

Expand Down
2 changes: 2 additions & 0 deletions deps/openssl/openssl/include/openssl/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,8 @@ typedef enum {
# define SSL_CB_CONNECT_EXIT (SSL_ST_CONNECT|SSL_CB_EXIT)
# define SSL_CB_HANDSHAKE_START 0x10
# define SSL_CB_HANDSHAKE_DONE 0x20
# define SSL_CB_POST_HANDSHAKE_START 0x40
# define SSL_CB_POST_HANDSHAKE_DONE 0x80

/* Is the SSL_connection established? */
# define SSL_in_connect_init(a) (SSL_in_init(a) && !SSL_is_server(a))
Expand Down
6 changes: 4 additions & 2 deletions deps/openssl/openssl/ssl/statem/statem.c
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,10 @@ static int state_machine(SSL *s, int server)
}

s->server = server;
if (cb != NULL)
cb(s, SSL_CB_HANDSHAKE_START, 1);
if (cb != NULL) {
if (SSL_IS_FIRST_HANDSHAKE(s) || !SSL_IS_TLS13(s))
cb(s, SSL_CB_HANDSHAKE_START, 1);
}

/*
* Fatal errors in this block don't send an alert because we have
Expand Down
11 changes: 8 additions & 3 deletions deps/openssl/openssl/ssl/statem/statem_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,7 @@ unsigned long ssl3_output_cert_chain(SSL *s, WPACKET *pkt, CERT_PKEY *cpk)
WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs, int stop)
{
void (*cb) (const SSL *ssl, int type, int val) = NULL;
int cleanuphand = s->statem.cleanuphand;

if (clearbufs) {
if (!SSL_IS_DTLS(s)) {
Expand All @@ -1054,7 +1055,7 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs, int stop)
* Only set if there was a Finished message and this isn't after a TLSv1.3
* post handshake exchange
*/
if (s->statem.cleanuphand) {
if (cleanuphand) {
/* skipped if we just sent a HelloRequest */
s->renegotiate = 0;
s->new_session = 0;
Expand Down Expand Up @@ -1132,8 +1133,12 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs, int stop)
/* The callback may expect us to not be in init at handshake done */
ossl_statem_set_in_init(s, 0);

if (cb != NULL)
cb(s, SSL_CB_HANDSHAKE_DONE, 1);
if (cb != NULL) {
if (cleanuphand
|| !SSL_IS_TLS13(s)
|| SSL_IS_FIRST_HANDSHAKE(s))
cb(s, SSL_CB_HANDSHAKE_DONE, 1);
}

if (!stop) {
/* If we've got more work to do we go back into init */
Expand Down
19 changes: 0 additions & 19 deletions deps/openssl/openssl/ssl/statem/statem_srvr.c
Original file line number Diff line number Diff line change
Expand Up @@ -4028,7 +4028,6 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
uint64_t nonce;
static const unsigned char nonce_label[] = "resumption";
const EVP_MD *md = ssl_handshake_md(s);
void (*cb) (const SSL *ssl, int type, int val) = NULL;
int hashleni = EVP_MD_size(md);

/* Ensure cast to size_t is safe */
Expand All @@ -4040,24 +4039,6 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
}
hashlen = (size_t)hashleni;

if (s->info_callback != NULL)
cb = s->info_callback;
else if (s->ctx->info_callback != NULL)
cb = s->ctx->info_callback;

if (cb != NULL) {
/*
* We don't start and stop the handshake in between each ticket when
* sending more than one - but it should appear that way to the info
* callback.
*/
if (s->sent_tickets != 0) {
ossl_statem_set_in_init(s, 0);
cb(s, SSL_CB_HANDSHAKE_DONE, 1);
ossl_statem_set_in_init(s, 1);
}
cb(s, SSL_CB_HANDSHAKE_START, 1);
}
/*
* If we already sent one NewSessionTicket, or we resumed then
* s->session may already be in a cache and so we must not modify it.
Expand Down
49 changes: 21 additions & 28 deletions deps/openssl/openssl/test/sslapitest.c
Original file line number Diff line number Diff line change
Expand Up @@ -4710,39 +4710,31 @@ static struct info_cb_states_st {
{SSL_CB_LOOP, "TWCCS"}, {SSL_CB_LOOP, "TWEE"}, {SSL_CB_LOOP, "TWSC"},
{SSL_CB_LOOP, "TRSCV"}, {SSL_CB_LOOP, "TWFIN"}, {SSL_CB_LOOP, "TED"},
{SSL_CB_EXIT, NULL}, {SSL_CB_LOOP, "TED"}, {SSL_CB_LOOP, "TRFIN"},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_HANDSHAKE_START, NULL},
{SSL_CB_LOOP, "TWST"}, {SSL_CB_HANDSHAKE_DONE, NULL},
{SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "TWST"},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_EXIT, NULL},
{SSL_CB_ALERT, NULL}, {SSL_CB_HANDSHAKE_START, NULL},
{SSL_CB_LOOP, "PINIT "}, {SSL_CB_LOOP, "PINIT "}, {SSL_CB_LOOP, "TRCH"},
{SSL_CB_LOOP, "TWSH"}, {SSL_CB_LOOP, "TWCCS"}, {SSL_CB_LOOP, "TWEE"},
{SSL_CB_LOOP, "TWFIN"}, {SSL_CB_LOOP, "TED"}, {SSL_CB_EXIT, NULL},
{SSL_CB_LOOP, "TED"}, {SSL_CB_LOOP, "TRFIN"},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_HANDSHAKE_START, NULL},
{SSL_CB_LOOP, "TWST"}, {SSL_CB_HANDSHAKE_DONE, NULL},
{SSL_CB_EXIT, NULL}, {0, NULL},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_LOOP, "TWST"},
{SSL_CB_LOOP, "TWST"}, {SSL_CB_EXIT, NULL}, {SSL_CB_ALERT, NULL},
{SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "PINIT "},
{SSL_CB_LOOP, "PINIT "}, {SSL_CB_LOOP, "TRCH"}, {SSL_CB_LOOP, "TWSH"},
{SSL_CB_LOOP, "TWCCS"}, {SSL_CB_LOOP, "TWEE"}, {SSL_CB_LOOP, "TWFIN"},
{SSL_CB_LOOP, "TED"}, {SSL_CB_EXIT, NULL}, {SSL_CB_LOOP, "TED"},
{SSL_CB_LOOP, "TRFIN"}, {SSL_CB_HANDSHAKE_DONE, NULL},
{SSL_CB_LOOP, "TWST"}, {SSL_CB_EXIT, NULL}, {0, NULL},
}, {
/* TLSv1.3 client followed by resumption */
{SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "PINIT "},
{SSL_CB_LOOP, "TWCH"}, {SSL_CB_EXIT, NULL}, {SSL_CB_LOOP, "TWCH"},
{SSL_CB_LOOP, "TRSH"}, {SSL_CB_LOOP, "TREE"}, {SSL_CB_LOOP, "TRSC"},
{SSL_CB_LOOP, "TRSCV"}, {SSL_CB_LOOP, "TRFIN"}, {SSL_CB_LOOP, "TWCCS"},
{SSL_CB_LOOP, "TWFIN"}, {SSL_CB_HANDSHAKE_DONE, NULL},
{SSL_CB_EXIT, NULL}, {SSL_CB_HANDSHAKE_START, NULL},
{SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "TRST"},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_EXIT, NULL},
{SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "SSLOK "},
{SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "TRST"},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_EXIT, NULL},
{SSL_CB_EXIT, NULL}, {SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "SSLOK "},
{SSL_CB_LOOP, "TRST"}, {SSL_CB_EXIT, NULL}, {SSL_CB_LOOP, "SSLOK "},
{SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "TRST"}, {SSL_CB_EXIT, NULL},
{SSL_CB_ALERT, NULL}, {SSL_CB_HANDSHAKE_START, NULL},
{SSL_CB_LOOP, "PINIT "}, {SSL_CB_LOOP, "TWCH"}, {SSL_CB_EXIT, NULL},
{SSL_CB_LOOP, "TWCH"}, {SSL_CB_LOOP, "TRSH"}, {SSL_CB_LOOP, "TREE"},
{SSL_CB_LOOP, "TRFIN"}, {SSL_CB_LOOP, "TWCCS"}, {SSL_CB_LOOP, "TWFIN"},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_EXIT, NULL},
{SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "SSLOK "},
{SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "TRST"},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_EXIT, NULL}, {0, NULL},
{SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "TRST"},
{SSL_CB_EXIT, NULL}, {0, NULL},
}, {
/* TLSv1.3 server, early_data */
{SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "PINIT "},
Expand All @@ -4751,8 +4743,7 @@ static struct info_cb_states_st {
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_EXIT, NULL},
{SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "TED"},
{SSL_CB_LOOP, "TED"}, {SSL_CB_LOOP, "TWEOED"}, {SSL_CB_LOOP, "TRFIN"},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_HANDSHAKE_START, NULL},
{SSL_CB_LOOP, "TWST"}, {SSL_CB_HANDSHAKE_DONE, NULL},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_LOOP, "TWST"},
{SSL_CB_EXIT, NULL}, {0, NULL},
}, {
/* TLSv1.3 client, early_data */
Expand All @@ -4763,9 +4754,8 @@ static struct info_cb_states_st {
{SSL_CB_LOOP, "TED"}, {SSL_CB_LOOP, "TRSH"}, {SSL_CB_LOOP, "TREE"},
{SSL_CB_LOOP, "TRFIN"}, {SSL_CB_LOOP, "TPEDE"}, {SSL_CB_LOOP, "TWEOED"},
{SSL_CB_LOOP, "TWFIN"}, {SSL_CB_HANDSHAKE_DONE, NULL},
{SSL_CB_EXIT, NULL}, {SSL_CB_HANDSHAKE_START, NULL},
{SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "TRST"},
{SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_EXIT, NULL}, {0, NULL},
{SSL_CB_EXIT, NULL}, {SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "SSLOK "},
{SSL_CB_LOOP, "TRST"}, {SSL_CB_EXIT, NULL}, {0, NULL},
}, {
{0, NULL},
}
Expand Down Expand Up @@ -4804,8 +4794,11 @@ static void sslapi_info_callback(const SSL *s, int where, int ret)
return;
}

/* Check that, if we've got SSL_CB_HANDSHAKE_DONE we are not in init */
if ((where & SSL_CB_HANDSHAKE_DONE) && SSL_in_init((SSL *)s) != 0) {
/*
* Check that, if we've got SSL_CB_HANDSHAKE_DONE we are not in init
*/
if ((where & SSL_CB_HANDSHAKE_DONE)
&& SSL_in_init((SSL *)s) != 0) {
info_cb_failed = 1;
return;
}
Expand Down
43 changes: 39 additions & 4 deletions lib/_tls_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const {
TLS1_VERSION,
TLS1_1_VERSION,
TLS1_2_VERSION,
TLS1_3_VERSION,
} = internalBinding('constants').crypto;

// Lazily loaded from internal/crypto/util.
Expand All @@ -45,6 +46,7 @@ function toV(which, v, def) {
if (v === 'TLSv1') return TLS1_VERSION;
if (v === 'TLSv1.1') return TLS1_1_VERSION;
if (v === 'TLSv1.2') return TLS1_2_VERSION;
if (v === 'TLSv1.3') return TLS1_3_VERSION;
throw new ERR_TLS_INVALID_PROTOCOL_VERSION(v, which);
}

Expand Down Expand Up @@ -148,10 +150,43 @@ exports.createSecureContext = function createSecureContext(options) {
}
}

if (options.ciphers)
c.context.setCiphers(options.ciphers);
else
c.context.setCiphers(tls.DEFAULT_CIPHERS);
if (options.ciphers && typeof options.ciphers !== 'string') {
// Compatible with THROW_ERR_MISSING_ARGS() in SecureContext::SetCiphers().
const err = new TypeError('Ciphers must be a string');
err.code = 'ERR_INVALID_ARG_TYPE';
throw err;

// XXX modern code would do this, change now, or in semver-major update?
throw new ERR_INVALID_ARG_TYPE(
'options.ciphers',
'string',
options.ciphers
);
}

// Work around an OpenSSL API quirk. cipherList is for TLSv1.2 and below,
// cipherSuites is for TLSv1.3 (and presumably any later versions). TLSv1.3
// cipher suites all have a standard name format beginning with TLS_, so split
// the ciphers and pass them to the appropriate API.
let ciphers = (options.ciphers || tls.DEFAULT_CIPHERS).split(':');
let cipherList = ciphers.filter(_ => !_.match(/^TLS_/)).join(':');
let cipherSuites = ciphers.filter(_ => _.match(/^TLS_/)).join(':');

// Set cipher suites first, so that setCiphers() will error if there are
// neither TLSv1.2 nor TLSv1.3 cipher suites enabled.
// XXX test to confirm this works as intended, particularly that it detects
// bad TLS1.3 ciphers and also when a bad TLS1.2 cipher list causes there to
// be no cipher suites at all.
c.context.setCipherSuites(cipherSuites);
c.context.setCiphers(cipherList);

if (cipherSuites === '' && c.context.getMaxProto() > TLS1_2_VERSION
&& c.context.getMinProto() < TLS1_3_VERSION)
c.context.setMaxProto(TLS1_2_VERSION);

if (cipherList === '' && c.context.getMinProto() < TLS1_3_VERSION
&& c.context.getMaxProto() > TLS1_2_VERSION)
c.context.setMinProto(TLS1_3_VERSION);

if (options.ecdhCurve === undefined)
c.context.setECDHCurve(tls.DEFAULT_ECDH_CURVE);
Expand Down
Loading