1 From 9c14752f8872401de413fb46a96146b0d6bf926e Mon Sep 17 00:00:00 2001 2 From: Alex Klyubin <klyubin (a] google.com> 3 Date: Tue, 8 Apr 2014 16:02:24 -0700 4 Subject: tls_psk_hint 5 6 Fix TLS-PSK identity hint implementation issues. 7 8 PSK identity hint can be stored in SSL_CTX and in SSL/SSL_SESSION, 9 similar to other TLS parameters, with the value in SSL/SSL_SESSION 10 taking precedence over the one in SSL_CTX. The value in SSL_CTX is 11 shared (used as the default) between all SSL instances associated 12 with that SSL_CTX, whereas the value in SSL/SSL_SESSION is confined 13 to that particular TLS/SSL connection/session. 14 15 The existing implementation of TLS-PSK does not correctly distinguish 16 between PSK identity hint in SSL_CTX and in SSL/SSL_SESSION. This 17 change fixes these issues: 18 1. SSL_use_psk_identity_hint does nothing and returns "success" when 19 the SSL object does not have an associated SSL_SESSION. 20 2. On the client, the hint in SSL_CTX (which is shared between 21 multiple SSL instances) is overwritten with the hint received from 22 server or reset to NULL if no hint was received. 23 3. On the client, psk_client_callback is invoked with the hint from 24 SSL_CTX rather than from current SSL/SSL_SESSION (i.e., the one 25 received from the server). Issue #2 above masks this issue. 26 4. On the server, the hint in SSL/SSL_SESSION is ignored and the hint 27 from SSL_CTX is sent to the client. 28 5. On the server, the hint in SSL/SSL_SESSION is reset to the one in 29 SSL_CTX after the ClientKeyExchange message step. 30 31 This change fixes the issues by: 32 * Adding storage for the hint in the SSL object. The idea being that 33 the hint in the associated SSL_SESSION takes precedence. 34 * Reading the hint during the handshake only from the associated 35 SSL_SESSION object. 36 * Initializing the hint in SSL object with the one from the SSL_CTX 37 object. 38 * Initializing the hint in SSL_SESSION object with the one from the 39 SSL object. 40 * Making SSL_use_psk_identity_hint and SSL_get_psk_identity_hint 41 set/get the hint to/from SSL_SESSION associated with the provided 42 SSL object, or, if no SSL_SESSION is available, set/get the hint 43 to/from the provided SSL object. 44 * Removing code which resets the hint during handshake. 45 --- 46 ssl/d1_clnt.c | 13 +------------ 47 ssl/d1_srvr.c | 10 +++++----- 48 ssl/s3_clnt.c | 37 +++++++++++++------------------------ 49 ssl/s3_srvr.c | 44 ++++++++++++++++---------------------------- 50 ssl/ssl.h | 4 ++++ 51 ssl/ssl_lib.c | 56 +++++++++++++++++++++++++++++++++++++++++++++----------- 52 ssl/ssl_sess.c | 12 ++++++++++++ 53 7 files changed, 96 insertions(+), 80 deletions(-) 54 55 diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c 56 index f857946..b017139 100644 57 --- a/ssl/d1_clnt.c 58 +++ b/ssl/d1_clnt.c 59 @@ -1434,7 +1434,7 @@ int dtls1_send_client_key_exchange(SSL *s) 60 goto err; 61 } 62 63 - psk_len = s->psk_client_callback(s, s->ctx->psk_identity_hint, 64 + psk_len = s->psk_client_callback(s, s->session->psk_identity_hint, 65 identity, PSK_MAX_IDENTITY_LEN, 66 psk_or_pre_ms, sizeof(psk_or_pre_ms)); 67 if (psk_len > PSK_MAX_PSK_LEN) 68 @@ -1459,17 +1459,6 @@ int dtls1_send_client_key_exchange(SSL *s) 69 t+=psk_len; 70 s2n(psk_len, t); 71 72 - if (s->session->psk_identity_hint != NULL) 73 - OPENSSL_free(s->session->psk_identity_hint); 74 - s->session->psk_identity_hint = BUF_strdup(s->ctx->psk_identity_hint); 75 - if (s->ctx->psk_identity_hint != NULL && 76 - s->session->psk_identity_hint == NULL) 77 - { 78 - SSLerr(SSL_F_DTLS1_SEND_CLIENT_KEY_EXCHANGE, 79 - ERR_R_MALLOC_FAILURE); 80 - goto psk_err; 81 - } 82 - 83 if (s->session->psk_identity != NULL) 84 OPENSSL_free(s->session->psk_identity); 85 s->session->psk_identity = BUF_strdup(identity); 86 diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c 87 index 1384ab0..c181db6 100644 88 --- a/ssl/d1_srvr.c 89 +++ b/ssl/d1_srvr.c 90 @@ -471,7 +471,7 @@ int dtls1_accept(SSL *s) 91 /* PSK: send ServerKeyExchange if PSK identity 92 * hint if provided */ 93 #ifndef OPENSSL_NO_PSK 94 - || ((alg_k & SSL_kPSK) && s->ctx->psk_identity_hint) 95 + || ((alg_k & SSL_kPSK) && s->session->psk_identity_hint) 96 #endif 97 || (alg_k & (SSL_kEDH|SSL_kDHr|SSL_kDHd)) 98 || (alg_k & SSL_kEECDH) 99 @@ -1288,7 +1288,7 @@ int dtls1_send_server_key_exchange(SSL *s) 100 if (type & SSL_kPSK) 101 { 102 /* reserve size for record length and PSK identity hint*/ 103 - n+=2+strlen(s->ctx->psk_identity_hint); 104 + n+=2+strlen(s->session->psk_identity_hint); 105 } 106 else 107 #endif /* !OPENSSL_NO_PSK */ 108 @@ -1365,9 +1365,9 @@ int dtls1_send_server_key_exchange(SSL *s) 109 if (type & SSL_kPSK) 110 { 111 /* copy PSK identity hint */ 112 - s2n(strlen(s->ctx->psk_identity_hint), p); 113 - strncpy((char *)p, s->ctx->psk_identity_hint, strlen(s->ctx->psk_identity_hint)); 114 - p+=strlen(s->ctx->psk_identity_hint); 115 + s2n(strlen(s->session->psk_identity_hint), p); 116 + strncpy((char *)p, s->session->psk_identity_hint, strlen(s->session->psk_identity_hint)); 117 + p+=strlen(s->session->psk_identity_hint); 118 } 119 #endif 120 121 diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c 122 index 12c3fe8..17367a2 100644 123 --- a/ssl/s3_clnt.c 124 +++ b/ssl/s3_clnt.c 125 @@ -1374,9 +1374,11 @@ int ssl3_get_key_exchange(SSL *s) 126 if (s->s3->tmp.new_cipher->algorithm_auth & SSL_aPSK) 127 { 128 s->session->sess_cert=ssl_sess_cert_new(); 129 - if (s->ctx->psk_identity_hint) 130 - OPENSSL_free(s->ctx->psk_identity_hint); 131 - s->ctx->psk_identity_hint = NULL; 132 + if (s->session->psk_identity_hint) 133 + { 134 + OPENSSL_free(s->session->psk_identity_hint); 135 + s->session->psk_identity_hint = NULL; 136 + } 137 } 138 #endif 139 s->s3->tmp.reuse_message=1; 140 @@ -1426,7 +1428,11 @@ int ssl3_get_key_exchange(SSL *s) 141 } 142 n2s(p,i); 143 144 - s->ctx->psk_identity_hint = NULL; 145 + if (s->session->psk_identity_hint) 146 + { 147 + OPENSSL_free(s->session->psk_identity_hint); 148 + s->session->psk_identity_hint = NULL; 149 + } 150 if (i != 0) 151 { 152 /* Store PSK identity hint for later use, hint is used 153 @@ -1452,10 +1458,8 @@ int ssl3_get_key_exchange(SSL *s) 154 * NULL-terminated string. */ 155 memcpy(tmp_id_hint, p, i); 156 memset(tmp_id_hint+i, 0, PSK_MAX_IDENTITY_LEN+1-i); 157 - if (s->ctx->psk_identity_hint != NULL) 158 - OPENSSL_free(s->ctx->psk_identity_hint); 159 - s->ctx->psk_identity_hint = BUF_strdup(tmp_id_hint); 160 - if (s->ctx->psk_identity_hint == NULL) 161 + s->session->psk_identity_hint = BUF_strdup(tmp_id_hint); 162 + if (s->session->psk_identity_hint == NULL) 163 { 164 SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); 165 goto f_err; 166 @@ -2338,7 +2342,8 @@ int ssl3_send_client_key_exchange(SSL *s) 167 goto err; 168 } 169 170 + memset(identity, 0, sizeof(identity)); 171 - psk_len = s->psk_client_callback(s, s->ctx->psk_identity_hint, 172 + psk_len = s->psk_client_callback(s, s->session->psk_identity_hint, 173 identity, sizeof(identity) - 1, psk, sizeof(psk)); 174 if (psk_len > PSK_MAX_PSK_LEN) 175 { 176 @@ -2374,21 +2378,6 @@ int ssl3_send_client_key_exchange(SSL *s) 177 n += 2; 178 } 179 180 - if (s->session->psk_identity_hint != NULL) 181 - OPENSSL_free(s->session->psk_identity_hint); 182 - s->session->psk_identity_hint = NULL; 183 - if (s->ctx->psk_identity_hint) 184 - { 185 - s->session->psk_identity_hint = BUF_strdup(s->ctx->psk_identity_hint); 186 - if (s->ctx->psk_identity_hint != NULL && 187 - s->session->psk_identity_hint == NULL) 188 - { 189 - SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE, 190 - ERR_R_MALLOC_FAILURE); 191 - goto psk_err; 192 - } 193 - } 194 - 195 if (s->session->psk_identity != NULL) 196 OPENSSL_free(s->session->psk_identity); 197 s->session->psk_identity = BUF_strdup(identity); 198 diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c 199 index d6f1a35..c360337 100644 200 --- a/ssl/s3_srvr.c 201 +++ b/ssl/s3_srvr.c 202 @@ -492,7 +492,7 @@ int ssl3_accept(SSL *s) 203 * - the key exchange is kEECDH. 204 */ 205 #ifndef OPENSSL_NO_PSK 206 - || ((alg_a & SSL_aPSK) && ((alg_k & SSL_kEECDH) || s->ctx->psk_identity_hint)) 207 + || ((alg_a & SSL_aPSK) && ((alg_k & SSL_kEECDH) || s->session->psk_identity_hint)) 208 #endif 209 #ifndef OPENSSL_NO_SRP 210 /* SRP: send ServerKeyExchange */ 211 @@ -1702,6 +1702,10 @@ int ssl3_send_server_key_exchange(SSL *s) 212 int curve_id = 0; 213 BN_CTX *bn_ctx = NULL; 214 #endif 215 +#ifndef OPENSSL_NO_PSK 216 + const char* psk_identity_hint; 217 + size_t psk_identity_hint_len; 218 +#endif 219 EVP_PKEY *pkey; 220 const EVP_MD *md = NULL; 221 unsigned char *p,*d; 222 @@ -1730,9 +1734,12 @@ int ssl3_send_server_key_exchange(SSL *s) 223 if (alg_a & SSL_aPSK) 224 { 225 /* size for PSK identity hint */ 226 - n+=2; 227 - if (s->ctx->psk_identity_hint) 228 - n+=strlen(s->ctx->psk_identity_hint); 229 + psk_identity_hint = s->session->psk_identity_hint; 230 + if (psk_identity_hint) 231 + psk_identity_hint_len = strlen(psk_identity_hint); 232 + else 233 + psk_identity_hint_len = 0; 234 + n+=2+psk_identity_hint_len; 235 } 236 #endif /* !OPENSSL_NO_PSK */ 237 #ifndef OPENSSL_NO_RSA 238 @@ -2025,20 +2032,12 @@ int ssl3_send_server_key_exchange(SSL *s) 239 #ifndef OPENSSL_NO_PSK 240 if (alg_a & SSL_aPSK) 241 { 242 - if (s->ctx->psk_identity_hint) 243 - { 244 - /* copy PSK identity hint */ 245 - s2n(strlen(s->ctx->psk_identity_hint), p); 246 - strncpy((char *)p, s->ctx->psk_identity_hint, strlen(s->ctx->psk_identity_hint)); 247 - p+=strlen(s->ctx->psk_identity_hint); 248 - } 249 - else 250 + /* copy PSK identity hint (if provided) */ 251 + s2n(psk_identity_hint_len, p); 252 + if (psk_identity_hint_len > 0) 253 { 254 - /* No identity hint is provided. */ 255 - *p = 0; 256 - p += 1; 257 - *p = 0; 258 - p += 1; 259 + memcpy(p, psk_identity_hint, psk_identity_hint_len); 260 + p+=psk_identity_hint_len; 261 } 262 } 263 #endif /* OPENSSL_NO_PSK */ 264 @@ -2393,17 +2392,6 @@ int ssl3_get_client_key_exchange(SSL *s) 265 goto psk_err; 266 } 267 268 - if (s->session->psk_identity_hint != NULL) 269 - OPENSSL_free(s->session->psk_identity_hint); 270 - s->session->psk_identity_hint = BUF_strdup(s->ctx->psk_identity_hint); 271 - if (s->ctx->psk_identity_hint != NULL && 272 - s->session->psk_identity_hint == NULL) 273 - { 274 - SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, 275 - ERR_R_MALLOC_FAILURE); 276 - goto psk_err; 277 - } 278 - 279 p += i; 280 n -= (i + 2); 281 psk_err = 0; 282 diff --git a/ssl/ssl.h b/ssl/ssl.h 283 index a7e1455..f044cd1 100644 284 --- a/ssl/ssl.h 285 +++ b/ssl/ssl.h 286 @@ -1441,6 +1441,10 @@ struct ssl_st 287 #endif /* OPENSSL_NO_KRB5 */ 288 289 #ifndef OPENSSL_NO_PSK 290 + /* PSK identity hint is stored here only to enable setting a hint on an SSL object before an 291 + * SSL_SESSION is associated with it. Once an SSL_SESSION is associated with this SSL object, 292 + * the psk_identity_hint from the session takes precedence over this one. */ 293 + char *psk_identity_hint; 294 unsigned int (*psk_client_callback)(SSL *ssl, const char *hint, char *identity, 295 unsigned int max_identity_len, unsigned char *psk, 296 unsigned int max_psk_len); 297 diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c 298 index 3e49cab..cf24292 100644 299 --- a/ssl/ssl_lib.c 300 +++ b/ssl/ssl_lib.c 301 @@ -388,6 +388,13 @@ SSL *SSL_new(SSL_CTX *ctx) 302 CRYPTO_new_ex_data(CRYPTO_EX_INDEX_SSL, s, &s->ex_data); 303 304 #ifndef OPENSSL_NO_PSK 305 + s->psk_identity_hint = NULL; 306 + if (ctx->psk_identity_hint) 307 + { 308 + s->psk_identity_hint = BUF_strdup(ctx->psk_identity_hint); 309 + if (s->psk_identity_hint == NULL) 310 + goto err; 311 + } 312 s->psk_client_callback=ctx->psk_client_callback; 313 s->psk_server_callback=ctx->psk_server_callback; 314 #endif 315 @@ -648,6 +655,11 @@ void SSL_free(SSL *s) 316 OPENSSL_free(s->alpn_client_proto_list); 317 #endif 318 319 +#ifndef OPENSSL_NO_PSK 320 + if (s->psk_identity_hint) 321 + OPENSSL_free(s->psk_identity_hint); 322 +#endif 323 + 324 if (s->client_CA != NULL) 325 sk_X509_NAME_pop_free(s->client_CA,X509_NAME_free); 326 327 @@ -3361,32 +3373,54 @@ int SSL_use_psk_identity_hint(SSL *s, const char *identity_hint) 328 if (s == NULL) 329 return 0; 330 331 - if (s->session == NULL) 332 - return 1; /* session not created yet, ignored */ 333 - 334 if (identity_hint != NULL && strlen(identity_hint) > PSK_MAX_IDENTITY_LEN) 335 { 336 SSLerr(SSL_F_SSL_USE_PSK_IDENTITY_HINT, SSL_R_DATA_LENGTH_TOO_LONG); 337 return 0; 338 } 339 - if (s->session->psk_identity_hint != NULL) 340 + 341 + /* Clear hint in SSL and associated SSL_SESSION (if any). */ 342 + if (s->psk_identity_hint != NULL) 343 + { 344 + OPENSSL_free(s->psk_identity_hint); 345 + s->psk_identity_hint = NULL; 346 + } 347 + if (s->session != NULL && s->session->psk_identity_hint != NULL) 348 + { 349 OPENSSL_free(s->session->psk_identity_hint); 350 + s->session->psk_identity_hint = NULL; 351 + } 352 + 353 if (identity_hint != NULL) 354 { 355 - s->session->psk_identity_hint = BUF_strdup(identity_hint); 356 - if (s->session->psk_identity_hint == NULL) 357 - return 0; 358 + /* The hint is stored in SSL and SSL_SESSION with the one in 359 + * SSL_SESSION taking precedence. Thus, if SSL_SESSION is avaiable, 360 + * we store the hint there, otherwise we store it in SSL. */ 361 + if (s->session != NULL) 362 + { 363 + s->session->psk_identity_hint = BUF_strdup(identity_hint); 364 + if (s->session->psk_identity_hint == NULL) 365 + return 0; 366 + } 367 + else 368 + { 369 + s->psk_identity_hint = BUF_strdup(identity_hint); 370 + if (s->psk_identity_hint == NULL) 371 + return 0; 372 + } 373 } 374 - else 375 - s->session->psk_identity_hint = NULL; 376 return 1; 377 } 378 379 const char *SSL_get_psk_identity_hint(const SSL *s) 380 { 381 - if (s == NULL || s->session == NULL) 382 + if (s == NULL) 383 return NULL; 384 - return(s->session->psk_identity_hint); 385 + /* The hint is stored in SSL and SSL_SESSION with the one in SSL_SESSION 386 + * taking precedence. */ 387 + if (s->session != NULL) 388 + return(s->session->psk_identity_hint); 389 + return(s->psk_identity_hint); 390 } 391 392 const char *SSL_get_psk_identity(const SSL *s) 393 diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c 394 index 44268e7..cdd198c 100644 395 --- a/ssl/ssl_sess.c 396 +++ b/ssl/ssl_sess.c 397 @@ -437,6 +437,18 @@ int ssl_get_new_session(SSL *s, int session) 398 } 399 #endif 400 #endif 401 +#ifndef OPENSSL_NO_PSK 402 + if (s->psk_identity_hint) 403 + { 404 + ss->psk_identity_hint = BUF_strdup(s->psk_identity_hint); 405 + if (ss->psk_identity_hint == NULL) 406 + { 407 + SSLerr(SSL_F_SSL_GET_NEW_SESSION, ERR_R_MALLOC_FAILURE); 408 + SSL_SESSION_free(ss); 409 + return 0; 410 + } 411 + } 412 +#endif 413 } 414 else 415 { 416 -- 417 2.0.0.526.g5318336 418 419
