mirror of https://github.com/openssl/openssl
PSK related tweaks based on review feedback
Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/3670)
This commit is contained in:
parent
adfc37868e
commit
72257204bd
|
@ -220,7 +220,6 @@ static int psk_use_session_cb(SSL *s, const EVP_MD *md,
|
|||
}
|
||||
|
||||
cipher = SSL_SESSION_get0_cipher(usesess);
|
||||
|
||||
if (cipher == NULL)
|
||||
goto err;
|
||||
|
||||
|
@ -577,8 +576,7 @@ typedef enum OPTION_choice {
|
|||
OPT_DEBUG, OPT_TLSEXTDEBUG, OPT_STATUS, OPT_WDEBUG,
|
||||
OPT_MSG, OPT_MSGFILE, OPT_ENGINE, OPT_TRACE, OPT_SECURITY_DEBUG,
|
||||
OPT_SECURITY_DEBUG_VERBOSE, OPT_SHOWCERTS, OPT_NBIO_TEST, OPT_STATE,
|
||||
OPT_PSK_IDENTITY, OPT_PSK,
|
||||
OPT_PSK_SESS,
|
||||
OPT_PSK_IDENTITY, OPT_PSK, OPT_PSK_SESS,
|
||||
#ifndef OPENSSL_NO_SRP
|
||||
OPT_SRPUSER, OPT_SRPPASS, OPT_SRP_STRENGTH, OPT_SRP_LATEUSER,
|
||||
OPT_SRP_MOREGROUPS,
|
||||
|
|
|
@ -38,8 +38,8 @@ TLSv1.3 Pre-Shared Keys (PSKs) and PSKs for TLSv1.2 and below are not
|
|||
compatible.
|
||||
|
||||
A client application wishing to use PSK ciphersuites for TLSv1.2 and below must
|
||||
provide a callback function which is called when the client is sending the
|
||||
ClientKeyExchange message to the server.
|
||||
provide a callback function. This function will be called when the client is
|
||||
sending the ClientKeyExchange message to the server.
|
||||
|
||||
The purpose of the callback function is to select the PSK identity and
|
||||
the pre-shared key to use during the connection setup phase.
|
||||
|
@ -57,7 +57,7 @@ A client application wishing to use TLSv1.3 PSKs must set a different callback
|
|||
using either SSL_CTX_set_psk_use_session_callback() or
|
||||
SSL_set_psk_use_session_callback() as appropriate.
|
||||
|
||||
The callback function is given a reference to the SSL connection in B<ssl>.
|
||||
The callback function is given a pointer to the SSL connection in B<ssl>.
|
||||
|
||||
The first time the callback is called for a connection the B<md> parameter is
|
||||
NULL. In some circumstances the callback will be called a second time. In that
|
||||
|
@ -71,7 +71,7 @@ the PSK in B<*id>. The identifier length in bytes should be stored in B<*idlen>.
|
|||
The memory pointed to by B<*id> remains owned by the application and should
|
||||
be freed by it as required at any point after the handshake is complete.
|
||||
|
||||
Additionally the callback should store a reference to an SSL_SESSION object in
|
||||
Additionally the callback should store a pointer to an SSL_SESSION object in
|
||||
B<*sess>. This is used as the basis for the PSK, and should, at a minimum, have
|
||||
the following fields set:
|
||||
|
||||
|
@ -85,16 +85,16 @@ This can be set via a call to L<SSL_SESSION_set1_master_key(3)>.
|
|||
|
||||
Only the handshake digest associated with the ciphersuite is relevant for the
|
||||
PSK (the server may go on to negotiate any ciphersuite which is compatible with
|
||||
the digest). The application can use any TLSv1.3 ciphersuite. Where B<md> is
|
||||
non-NULL the handshake digest for the ciphersuite should be the same.
|
||||
the digest). The application can use any TLSv1.3 ciphersuite. If B<md> is
|
||||
not NULL the handshake digest for the ciphersuite should be the same.
|
||||
The ciphersuite can be set via a call to <SSL_SESSION_set_cipher(3)>. The
|
||||
handshake digest of an SSL_CIPHER object can be checked using
|
||||
<SSL_CIPHER_get_handshake_digest(3)>.
|
||||
|
||||
=item The protocol version
|
||||
|
||||
This can be set via a call to L<SSL_SESSION_set_protocol_version> and should be
|
||||
TLS1_3_VERSION.
|
||||
This can be set via a call to L<SSL_SESSION_set_protocol_version(3)> and should
|
||||
be TLS1_3_VERSION.
|
||||
|
||||
=back
|
||||
|
||||
|
@ -118,7 +118,7 @@ has occurred so that L<SSL_session_reused(3)> will return true.
|
|||
|
||||
=head1 RETURN VALUES
|
||||
|
||||
Return values from the SSL_psk_client_cb_func callback are interpreted as
|
||||
Return values from the B<SSL_psk_client_cb_func> callback are interpreted as
|
||||
follows:
|
||||
|
||||
On success (callback found a PSK identity and a pre-shared key to use)
|
||||
|
|
|
@ -43,8 +43,8 @@ compatible.
|
|||
|
||||
Identity hints are not relevant for TLSv1.3. A server application wishing to use
|
||||
PSK ciphersuites for TLSv1.2 and below may call SSL_CTX_use_psk_identity_hint()
|
||||
to set the given B<NULL>-terminated PSK identity hint B<hint> for SSL context
|
||||
object B<ctx>. SSL_use_psk_identity_hint() sets the given B<NULL>-terminated PSK
|
||||
to set the given B<NUL>-terminated PSK identity hint B<hint> for SSL context
|
||||
object B<ctx>. SSL_use_psk_identity_hint() sets the given B<NUL>-terminated PSK
|
||||
identity hint B<hint> for the SSL connection object B<ssl>. If B<hint> is
|
||||
B<NULL> the current hint from B<ctx> or B<ssl> is deleted.
|
||||
|
||||
|
@ -57,7 +57,7 @@ client. The purpose of the callback function is to validate the
|
|||
received PSK identity and to fetch the pre-shared key used during the
|
||||
connection setup phase. The callback is set using the functions
|
||||
SSL_CTX_set_psk_server_callback() or SSL_set_psk_server_callback(). The callback
|
||||
function is given the connection in parameter B<ssl>, B<NULL>-terminated PSK
|
||||
function is given the connection in parameter B<ssl>, B<NUL>-terminated PSK
|
||||
identity sent by the client in parameter B<identity>, and a buffer B<psk> of
|
||||
length B<max_psk_len> bytes where the pre-shared key is to be stored.
|
||||
|
||||
|
@ -65,7 +65,7 @@ A client application wishing to use TLSv1.3 PSKs must set a different callback
|
|||
using either SSL_CTX_set_psk_use_session_callback() or
|
||||
SSL_set_psk_use_session_callback() as appropriate.
|
||||
|
||||
The callback function is given a reference to the SSL connection in B<ssl> and
|
||||
The callback function is given a pointer to the SSL connection in B<ssl> and
|
||||
an identity in B<identity> of length B<identity_len>. The callback function
|
||||
should identify an SSL_SESSION object that provides the PSK details and store it
|
||||
in B<*sess>. The SSL_SESSION object should, as a minimum, set the master key,
|
||||
|
@ -84,7 +84,7 @@ has occurred so that L<SSL_session_reused(3)> will return true.
|
|||
|
||||
=head1 RETURN VALUES
|
||||
|
||||
SSL_CTX_use_psk_identity_hint() and SSL_use_psk_identity_hint() return
|
||||
B<SSL_CTX_use_psk_identity_hint()> and B<SSL_use_psk_identity_hint()> return
|
||||
1 on success, 0 otherwise.
|
||||
|
||||
Return values from the TLSv1.2 and below server callback are interpreted as
|
||||
|
@ -112,7 +112,7 @@ completely.
|
|||
|
||||
=back
|
||||
|
||||
The SSL_psk_find_session_cb_func callback should return 1 on success or 0 on
|
||||
The B<SSL_psk_find_session_cb_func> callback should return 1 on success or 0 on
|
||||
failure. In the event of failure the connection setup fails.
|
||||
|
||||
=head1 COPYRIGHT
|
||||
|
|
|
@ -39,10 +39,10 @@ can be dangerous if misused; see NOTES below.
|
|||
SSL_SESSION_set1_master_key() sets the master key value associated with the
|
||||
SSL_SESSION B<sess>. For example, this could be used to set up a session based
|
||||
PSK (see L<SSL_CTX_set_psk_use_session_callback(3)>). The master key of length
|
||||
B<len> should be provided at B<in>. A copy of the supplied master key is taken
|
||||
by the function, so the caller is responsible for freeing and cleaning any
|
||||
memory associated with B<in>. The caller must ensure that the length of the ke
|
||||
is suitable for the ciphersuite associated with the SSL_SESSION.
|
||||
B<len> should be provided at B<in>. The supplied master key is copied by the
|
||||
function, so the caller is responsible for freeing and cleaning any memory
|
||||
associated with B<in>. The caller must ensure that the length of the key is
|
||||
suitable for the ciphersuite associated with the SSL_SESSION.
|
||||
|
||||
=head1 NOTES
|
||||
|
||||
|
|
|
@ -1933,9 +1933,8 @@ int SSL_CIPHER_get_auth_nid(const SSL_CIPHER *c)
|
|||
|
||||
const EVP_MD *SSL_CIPHER_get_handshake_digest(const SSL_CIPHER *c)
|
||||
{
|
||||
int idx = c->algorithm2;
|
||||
int idx = c->algorithm2 & SSL_HANDSHAKE_MAC_MASK;
|
||||
|
||||
idx &= SSL_HANDSHAKE_MAC_MASK;
|
||||
if (idx < 0 || idx >= SSL_MD_NUM_IDX)
|
||||
return NULL;
|
||||
return ssl_digest_methods[idx];
|
||||
|
|
|
@ -3733,7 +3733,6 @@ int SSL_SESSION_set1_master_key(SSL_SESSION *sess, const unsigned char *in,
|
|||
|
||||
memcpy(sess->master_key, in, len);
|
||||
sess->master_key_length = len;
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
||||
|
|
|
@ -825,31 +825,35 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
|
|||
}
|
||||
|
||||
if (s->session->ext.ticklen != 0) {
|
||||
/* Get the digest associated with the ciphersuite in the session */
|
||||
if (s->session->cipher == NULL) {
|
||||
SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
|
||||
goto err;
|
||||
}
|
||||
|
||||
mdres = ssl_md(s->session->cipher->algorithm2);
|
||||
if (mdres == NULL) {
|
||||
/* Don't recognize this cipher so we can't use the session. Ignore it */
|
||||
/*
|
||||
* Don't recognize this cipher so we can't use the session.
|
||||
* Ignore it
|
||||
*/
|
||||
goto dopsksess;
|
||||
}
|
||||
|
||||
if (s->hello_retry_request && mdres != handmd) {
|
||||
/*
|
||||
* Selected ciphersuite hash does not match the hash for the session so
|
||||
* we can't use it.
|
||||
* Selected ciphersuite hash does not match the hash for the session
|
||||
* so we can't use it.
|
||||
*/
|
||||
goto dopsksess;
|
||||
}
|
||||
|
||||
/*
|
||||
* Technically the C standard just says time() returns a time_t and says
|
||||
* nothing about the encoding of that type. In practice most implementations
|
||||
* follow POSIX which holds it as an integral type in seconds since epoch.
|
||||
* We've already made the assumption that we can do this in multiple places
|
||||
* in the code, so portability shouldn't be an issue.
|
||||
* nothing about the encoding of that type. In practice most
|
||||
* implementations follow POSIX which holds it as an integral type in
|
||||
* seconds since epoch. We've already made the assumption that we can do
|
||||
* this in multiple places in the code, so portability shouldn't be an
|
||||
* issue.
|
||||
*/
|
||||
now = (uint32_t)time(NULL);
|
||||
agesec = now - (uint32_t)s->session->time;
|
||||
|
@ -867,15 +871,15 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
|
|||
|
||||
if (agesec != 0 && agems / (uint32_t)1000 != agesec) {
|
||||
/*
|
||||
* Overflow. Shouldn't happen unless this is a *really* old session. If
|
||||
* so we just ignore it.
|
||||
* Overflow. Shouldn't happen unless this is a *really* old session.
|
||||
* If so we just ignore it.
|
||||
*/
|
||||
goto dopsksess;
|
||||
}
|
||||
|
||||
/*
|
||||
* Obfuscate the age. Overflow here is fine, this addition is supposed to
|
||||
* be mod 2^32.
|
||||
* Obfuscate the age. Overflow here is fine, this addition is supposed
|
||||
* to be mod 2^32.
|
||||
*/
|
||||
agems += s->session->ext.tick_age_add;
|
||||
|
||||
|
@ -956,15 +960,16 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
|
|||
|
||||
msgstart = WPACKET_get_curr(pkt) - msglen;
|
||||
|
||||
if (dores && tls_psk_do_binder(s, mdres, msgstart, binderoffset, NULL,
|
||||
resbinder, s->session, 1, 0) != 1) {
|
||||
if (dores
|
||||
&& tls_psk_do_binder(s, mdres, msgstart, binderoffset, NULL,
|
||||
resbinder, s->session, 1, 0) != 1) {
|
||||
SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
|
||||
goto err;
|
||||
}
|
||||
|
||||
if (psksess != NULL && tls_psk_do_binder(s, mdpsk, msgstart,
|
||||
binderoffset, NULL, pskbinder,
|
||||
psksess, 1, 1) != 1) {
|
||||
if (psksess != NULL
|
||||
&& tls_psk_do_binder(s, mdpsk, msgstart, binderoffset, NULL,
|
||||
pskbinder, psksess, 1, 1) != 1) {
|
||||
SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
|
||||
goto err;
|
||||
}
|
||||
|
|
|
@ -1938,19 +1938,23 @@ static int find_session_cb_cnt = 0;
|
|||
static int use_session_cb(SSL *ssl, const EVP_MD *md, const unsigned char **id,
|
||||
size_t *idlen, SSL_SESSION **sess)
|
||||
{
|
||||
use_session_cb_cnt++;
|
||||
switch (++use_session_cb_cnt) {
|
||||
case 1:
|
||||
/* The first call should always have a NULL md */
|
||||
if (md != NULL)
|
||||
return 0;
|
||||
break;
|
||||
|
||||
/* The first call should always have a NULL md */
|
||||
if (use_session_cb_cnt == 1 && md != NULL)
|
||||
return 0;
|
||||
case 2:
|
||||
/* The second call should always have an md */
|
||||
if (md == NULL)
|
||||
return 0;
|
||||
break;
|
||||
|
||||
/* The second call should always have an md */
|
||||
if (use_session_cb_cnt == 2 && md == NULL)
|
||||
return 0;
|
||||
|
||||
/* We should only be called a maximum of twice */
|
||||
if (use_session_cb_cnt == 3)
|
||||
default:
|
||||
/* We should only be called a maximum of twice */
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (psk != NULL)
|
||||
SSL_SESSION_up_ref(psk);
|
||||
|
|
Loading…
Reference in New Issue