Fix TLS-PSK identity hint implementation issues.
PSK identity hint can be stored in SSL_CTX and in SSL/SSL_SESSION,
similar to other TLS parameters, with the value in SSL/SSL_SESSION
taking precedence over the one in SSL_CTX. The value in SSL_CTX is
shared (used as the default) between all SSL instances associated
with that SSL_CTX, whereas the value in SSL/SSL_SESSION is confined
to that particular TLS/SSL connection/session.
The existing implementation of TLS-PSK does not correctly distinguish
between PSK identity hint in SSL_CTX and in SSL/SSL_SESSION. This
change fixes these issues:
1. SSL_use_psk_identity_hint does nothing and returns "success" when
the SSL object does not have an associated SSL_SESSION.
2. On the client, the hint in SSL_CTX (which is shared between
multiple SSL instances) is overwritten with the hint received from
server or reset to NULL if no hint was received.
3. On the client, psk_client_callback is invoked with the hint from
SSL_CTX rather than from current SSL/SSL_SESSION (i.e., the one
received from the server). Issue #2 above masks this issue.
4. On the server, the hint in SSL/SSL_SESSION is ignored and the hint
from SSL_CTX is sent to the client.
5. On the server, the hint in SSL/SSL_SESSION is reset to the one in
SSL_CTX after the ClientKeyExchange message step.
This change fixes the issues by:
* Adding storage for the hint in the SSL object. The idea being that
the hint in the associated SSL_SESSION takes precedence.
* Reading the hint during the handshake only from the associated
SSL_SESSION object.
* Initializing the hint in SSL object with the one from the SSL_CTX
object.
* Initializing the hint in SSL_SESSION object with the one from the
SSL object.
* Making SSL_use_psk_identity_hint and SSL_get_psk_identity_hint
set/get the hint to/from SSL_SESSION associated with the provided
SSL object, or, if no SSL_SESSION is available, set/get the hint
to/from the provided SSL object.
* Removing code which resets the hint during handshake.
Change-Id: I13f51a5e942269a727c9f26f31155e3d5093903f
diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c
index 4fc4e1b..5ee8f58 100644
--- a/ssl/d1_clnt.c
+++ b/ssl/d1_clnt.c
@@ -1440,7 +1440,7 @@
goto err;
}
- psk_len = s->psk_client_callback(s, s->ctx->psk_identity_hint,
+ psk_len = s->psk_client_callback(s, s->session->psk_identity_hint,
identity, PSK_MAX_IDENTITY_LEN,
psk_or_pre_ms, sizeof(psk_or_pre_ms));
if (psk_len > PSK_MAX_PSK_LEN)
@@ -1465,17 +1465,6 @@
t+=psk_len;
s2n(psk_len, t);
- if (s->session->psk_identity_hint != NULL)
- OPENSSL_free(s->session->psk_identity_hint);
- s->session->psk_identity_hint = BUF_strdup(s->ctx->psk_identity_hint);
- if (s->ctx->psk_identity_hint != NULL &&
- s->session->psk_identity_hint == NULL)
- {
- SSLerr(SSL_F_DTLS1_SEND_CLIENT_KEY_EXCHANGE,
- ERR_R_MALLOC_FAILURE);
- goto psk_err;
- }
-
if (s->session->psk_identity != NULL)
OPENSSL_free(s->session->psk_identity);
s->session->psk_identity = BUF_strdup(identity);
diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c
index 9975e20..09f4762 100644
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -471,7 +471,7 @@
/* PSK: send ServerKeyExchange if PSK identity
* hint if provided */
#ifndef OPENSSL_NO_PSK
- || ((alg_k & SSL_kPSK) && s->ctx->psk_identity_hint)
+ || ((alg_k & SSL_kPSK) && s->session->psk_identity_hint)
#endif
|| (alg_k & (SSL_kEDH|SSL_kDHr|SSL_kDHd))
|| (alg_k & SSL_kEECDH)
@@ -1288,7 +1288,7 @@
if (type & SSL_kPSK)
{
/* reserve size for record length and PSK identity hint*/
- n+=2+strlen(s->ctx->psk_identity_hint);
+ n+=2+strlen(s->session->psk_identity_hint);
}
else
#endif /* !OPENSSL_NO_PSK */
@@ -1364,9 +1364,9 @@
if (type & SSL_kPSK)
{
/* copy PSK identity hint */
- s2n(strlen(s->ctx->psk_identity_hint), p);
- strncpy((char *)p, s->ctx->psk_identity_hint, strlen(s->ctx->psk_identity_hint));
- p+=strlen(s->ctx->psk_identity_hint);
+ s2n(strlen(s->session->psk_identity_hint), p);
+ strncpy((char *)p, s->session->psk_identity_hint, strlen(s->session->psk_identity_hint));
+ p+=strlen(s->session->psk_identity_hint);
}
#endif
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 6d2b002..34f5ab1 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1381,9 +1381,11 @@
if (s->s3->tmp.new_cipher->algorithm_auth & SSL_aPSK)
{
s->session->sess_cert=ssl_sess_cert_new();
- if (s->ctx->psk_identity_hint)
- OPENSSL_free(s->ctx->psk_identity_hint);
- s->ctx->psk_identity_hint = NULL;
+ if (s->session->psk_identity_hint)
+ {
+ OPENSSL_free(s->session->psk_identity_hint);
+ s->session->psk_identity_hint = NULL;
+ }
}
#endif
s->s3->tmp.reuse_message=1;
@@ -1433,7 +1435,11 @@
al=SSL_AD_HANDSHAKE_FAILURE;
n2s(p,i);
param_len=i+2;
- s->ctx->psk_identity_hint = NULL;
+ if (s->session->psk_identity_hint)
+ {
+ OPENSSL_free(s->session->psk_identity_hint);
+ s->session->psk_identity_hint = NULL;
+ }
if (i != 0)
{
/* Store PSK identity hint for later use, hint is used
@@ -1459,10 +1465,8 @@
* NULL-terminated string. */
memcpy(tmp_id_hint, p, i);
memset(tmp_id_hint+i, 0, PSK_MAX_IDENTITY_LEN+1-i);
- if (s->ctx->psk_identity_hint != NULL)
- OPENSSL_free(s->ctx->psk_identity_hint);
- s->ctx->psk_identity_hint = BUF_strdup(tmp_id_hint);
- if (s->ctx->psk_identity_hint == NULL)
+ s->session->psk_identity_hint = BUF_strdup(tmp_id_hint);
+ if (s->session->psk_identity_hint == NULL)
{
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE);
goto f_err;
@@ -2345,7 +2349,7 @@
goto err;
}
- psk_len = s->psk_client_callback(s, s->ctx->psk_identity_hint,
+ psk_len = s->psk_client_callback(s, s->session->psk_identity_hint,
identity, PSK_MAX_IDENTITY_LEN, psk, sizeof(psk));
if (psk_len > PSK_MAX_PSK_LEN)
{
@@ -2381,21 +2385,6 @@
n += 2;
}
- if (s->session->psk_identity_hint != NULL)
- OPENSSL_free(s->session->psk_identity_hint);
- s->session->psk_identity_hint = NULL;
- if (s->ctx->psk_identity_hint)
- {
- s->session->psk_identity_hint = BUF_strdup(s->ctx->psk_identity_hint);
- if (s->ctx->psk_identity_hint != NULL &&
- s->session->psk_identity_hint == NULL)
- {
- SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE,
- ERR_R_MALLOC_FAILURE);
- goto psk_err;
- }
- }
-
if (s->session->psk_identity != NULL)
OPENSSL_free(s->session->psk_identity);
s->session->psk_identity = BUF_strdup(identity);
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 68ed534..0ee781f 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -481,7 +481,7 @@
* - the key exchange is kEECDH.
*/
#ifndef OPENSSL_NO_PSK
- || ((alg_a & SSL_aPSK) && ((alg_k & SSL_kEECDH) || s->ctx->psk_identity_hint))
+ || ((alg_a & SSL_aPSK) && ((alg_k & SSL_kEECDH) || s->session->psk_identity_hint))
#endif
#ifndef OPENSSL_NO_SRP
/* SRP: send ServerKeyExchange */
@@ -1595,6 +1595,10 @@
int curve_id = 0;
BN_CTX *bn_ctx = NULL;
#endif
+#ifndef OPENSSL_NO_PSK
+ const char* psk_identity_hint;
+ size_t psk_identity_hint_len;
+#endif
EVP_PKEY *pkey;
const EVP_MD *md = NULL;
unsigned char *p,*d;
@@ -1623,9 +1627,12 @@
if (alg_a & SSL_aPSK)
{
/* size for PSK identity hint */
- n+=2;
- if (s->ctx->psk_identity_hint)
- n+=strlen(s->ctx->psk_identity_hint);
+ psk_identity_hint = s->session->psk_identity_hint;
+ if (psk_identity_hint)
+ psk_identity_hint_len = strlen(psk_identity_hint);
+ else
+ psk_identity_hint_len = 0;
+ n+=2+psk_identity_hint_len;
}
#endif /* !OPENSSL_NO_PSK */
#ifndef OPENSSL_NO_RSA
@@ -1918,20 +1925,12 @@
#ifndef OPENSSL_NO_PSK
if (alg_a & SSL_aPSK)
{
- if (s->ctx->psk_identity_hint)
+ /* copy PSK identity hint (if provided) */
+ s2n(psk_identity_hint_len, p);
+ if (psk_identity_hint_len > 0)
{
- /* copy PSK identity hint */
- s2n(strlen(s->ctx->psk_identity_hint), p);
- strncpy((char *)p, s->ctx->psk_identity_hint, strlen(s->ctx->psk_identity_hint));
- p+=strlen(s->ctx->psk_identity_hint);
- }
- else
- {
- /* No identity hint is provided. */
- *p = 0;
- p += 1;
- *p = 0;
- p += 1;
+ memcpy(p, psk_identity_hint, psk_identity_hint_len);
+ p+=psk_identity_hint_len;
}
}
#endif /* OPENSSL_NO_PSK */
@@ -2281,17 +2280,6 @@
goto psk_err;
}
- if (s->session->psk_identity_hint != NULL)
- OPENSSL_free(s->session->psk_identity_hint);
- s->session->psk_identity_hint = BUF_strdup(s->ctx->psk_identity_hint);
- if (s->ctx->psk_identity_hint != NULL &&
- s->session->psk_identity_hint == NULL)
- {
- SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE,
- ERR_R_MALLOC_FAILURE);
- goto psk_err;
- }
-
p += i;
n -= (i + 2);
psk_err = 0;
diff --git a/ssl/ssl.h b/ssl/ssl.h
index 40c4d9c..57335a9 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -1315,6 +1315,10 @@
#endif /* OPENSSL_NO_KRB5 */
#ifndef OPENSSL_NO_PSK
+ /* PSK identity hint is stored here only to enable setting a hint on an SSL object before an
+ * SSL_SESSION is associated with it. Once an SSL_SESSION is associated with this SSL object,
+ * the psk_identity_hint from the session takes precedence over this one. */
+ char *psk_identity_hint;
unsigned int (*psk_client_callback)(SSL *ssl, const char *hint, char *identity,
unsigned int max_identity_len, unsigned char *psk,
unsigned int max_psk_len);
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 43a2dfd..8d2c3a7 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -388,6 +388,13 @@
CRYPTO_new_ex_data(CRYPTO_EX_INDEX_SSL, s, &s->ex_data);
#ifndef OPENSSL_NO_PSK
+ s->psk_identity_hint = NULL;
+ if (ctx->psk_identity_hint)
+ {
+ s->psk_identity_hint = BUF_strdup(ctx->psk_identity_hint);
+ if (s->psk_identity_hint == NULL)
+ goto err;
+ }
s->psk_client_callback=ctx->psk_client_callback;
s->psk_server_callback=ctx->psk_server_callback;
#endif
@@ -596,6 +603,11 @@
OPENSSL_free(s->alpn_client_proto_list);
#endif
+#ifndef OPENSSL_NO_PSK
+ if (s->psk_identity_hint)
+ OPENSSL_free(s->psk_identity_hint);
+#endif
+
if (s->client_CA != NULL)
sk_X509_NAME_pop_free(s->client_CA,X509_NAME_free);
@@ -3303,32 +3315,54 @@
if (s == NULL)
return 0;
- if (s->session == NULL)
- return 1; /* session not created yet, ignored */
-
if (identity_hint != NULL && strlen(identity_hint) > PSK_MAX_IDENTITY_LEN)
{
SSLerr(SSL_F_SSL_USE_PSK_IDENTITY_HINT, SSL_R_DATA_LENGTH_TOO_LONG);
return 0;
}
- if (s->session->psk_identity_hint != NULL)
+
+ /* Clear hint in SSL and associated SSL_SESSION (if any). */
+ if (s->psk_identity_hint != NULL)
+ {
+ OPENSSL_free(s->psk_identity_hint);
+ s->psk_identity_hint = NULL;
+ }
+ if (s->session != NULL && s->session->psk_identity_hint != NULL)
+ {
OPENSSL_free(s->session->psk_identity_hint);
+ s->session->psk_identity_hint = NULL;
+ }
+
if (identity_hint != NULL)
{
- s->session->psk_identity_hint = BUF_strdup(identity_hint);
- if (s->session->psk_identity_hint == NULL)
- return 0;
+ /* The hint is stored in SSL and SSL_SESSION with the one in
+ * SSL_SESSION taking precedence. Thus, if SSL_SESSION is avaiable,
+ * we store the hint there, otherwise we store it in SSL. */
+ if (s->session != NULL)
+ {
+ s->session->psk_identity_hint = BUF_strdup(identity_hint);
+ if (s->session->psk_identity_hint == NULL)
+ return 0;
+ }
+ else
+ {
+ s->psk_identity_hint = BUF_strdup(identity_hint);
+ if (s->psk_identity_hint == NULL)
+ return 0;
+ }
}
- else
- s->session->psk_identity_hint = NULL;
return 1;
}
const char *SSL_get_psk_identity_hint(const SSL *s)
{
- if (s == NULL || s->session == NULL)
+ if (s == NULL)
return NULL;
- return(s->session->psk_identity_hint);
+ /* The hint is stored in SSL and SSL_SESSION with the one in SSL_SESSION
+ * taking precedence. */
+ if (s->session != NULL)
+ return(s->session->psk_identity_hint);
+ return(s->psk_identity_hint);
}
const char *SSL_get_psk_identity(const SSL *s)
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 920b763..ec08840 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -427,6 +427,18 @@
}
#endif
#endif
+#ifndef OPENSSL_NO_PSK
+ if (s->psk_identity_hint)
+ {
+ ss->psk_identity_hint = BUF_strdup(s->psk_identity_hint);
+ if (ss->psk_identity_hint == NULL)
+ {
+ SSLerr(SSL_F_SSL_GET_NEW_SESSION, ERR_R_MALLOC_FAILURE);
+ SSL_SESSION_free(ss);
+ return 0;
+ }
+ }
+#endif
}
else
{