You cannot select more than 25 topics
			Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
		
		
		
		
		
			
		
			
				
	
	
		
			418 lines
		
	
	
		
			14 KiB
		
	
	
	
		
			Diff
		
	
			
		
		
	
	
			418 lines
		
	
	
		
			14 KiB
		
	
	
	
		
			Diff
		
	
| From 9c14752f8872401de413fb46a96146b0d6bf926e Mon Sep 17 00:00:00 2001
 | |
| From: Alex Klyubin <klyubin@google.com>
 | |
| Date: Tue, 8 Apr 2014 16:02:24 -0700
 | |
| Subject: tls_psk_hint
 | |
| 
 | |
| 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.
 | |
| ---
 | |
|  ssl/d1_clnt.c  | 13 +------------
 | |
|  ssl/d1_srvr.c  | 10 +++++-----
 | |
|  ssl/s3_clnt.c  | 37 +++++++++++++------------------------
 | |
|  ssl/s3_srvr.c  | 44 ++++++++++++++++----------------------------
 | |
|  ssl/ssl.h      |  4 ++++
 | |
|  ssl/ssl_lib.c  | 56 +++++++++++++++++++++++++++++++++++++++++++++-----------
 | |
|  ssl/ssl_sess.c | 12 ++++++++++++
 | |
|  7 files changed, 96 insertions(+), 80 deletions(-)
 | |
| 
 | |
| diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c
 | |
| index f857946..b017139 100644
 | |
| --- a/ssl/d1_clnt.c
 | |
| +++ b/ssl/d1_clnt.c
 | |
| @@ -1434,7 +1434,7 @@ int dtls1_send_client_key_exchange(SSL *s)
 | |
|  				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)
 | |
| @@ -1459,17 +1459,6 @@ int dtls1_send_client_key_exchange(SSL *s)
 | |
|  			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 1384ab0..c181db6 100644
 | |
| --- a/ssl/d1_srvr.c
 | |
| +++ b/ssl/d1_srvr.c
 | |
| @@ -471,7 +471,7 @@ int dtls1_accept(SSL *s)
 | |
|  			/* 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 @@ int dtls1_send_server_key_exchange(SSL *s)
 | |
|  			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 */
 | |
| @@ -1365,9 +1365,9 @@ int dtls1_send_server_key_exchange(SSL *s)
 | |
|  		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 12c3fe8..17367a2 100644
 | |
| --- a/ssl/s3_clnt.c
 | |
| +++ b/ssl/s3_clnt.c
 | |
| @@ -1374,9 +1374,11 @@ int ssl3_get_key_exchange(SSL *s)
 | |
|  		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;
 | |
| @@ -1426,7 +1428,11 @@ int ssl3_get_key_exchange(SSL *s)
 | |
|  		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
 | |
| @@ -1452,10 +1458,8 @@ int ssl3_get_key_exchange(SSL *s)
 | |
|  			 * 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;
 | |
| @@ -2338,7 +2342,7 @@ int ssl3_send_client_key_exchange(SSL *s)
 | |
|  				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)
 | |
|  				{
 | |
| @@ -2374,21 +2378,6 @@ int ssl3_send_client_key_exchange(SSL *s)
 | |
|  				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 d6f1a35..c360337 100644
 | |
| --- a/ssl/s3_srvr.c
 | |
| +++ b/ssl/s3_srvr.c
 | |
| @@ -492,7 +492,7 @@ int ssl3_accept(SSL *s)
 | |
|  			 *   - 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 */
 | |
| @@ -1702,6 +1702,10 @@ int ssl3_send_server_key_exchange(SSL *s)
 | |
|  	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;
 | |
| @@ -1730,9 +1734,12 @@ int ssl3_send_server_key_exchange(SSL *s)
 | |
|  		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
 | |
| @@ -2025,20 +2032,12 @@ int ssl3_send_server_key_exchange(SSL *s)
 | |
|  #ifndef OPENSSL_NO_PSK
 | |
|  		if (alg_a & SSL_aPSK)
 | |
|  			{
 | |
| -			if (s->ctx->psk_identity_hint)
 | |
| -				{
 | |
| -				/* 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
 | |
| +			/* copy PSK identity hint (if provided) */
 | |
| +			s2n(psk_identity_hint_len, p);
 | |
| +			if (psk_identity_hint_len > 0)
 | |
|  				{
 | |
| -				/* 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 */
 | |
| @@ -2393,17 +2392,6 @@ int ssl3_get_client_key_exchange(SSL *s)
 | |
|  			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 a7e1455..f044cd1 100644
 | |
| --- a/ssl/ssl.h
 | |
| +++ b/ssl/ssl.h
 | |
| @@ -1441,6 +1441,10 @@ struct ssl_st
 | |
|  #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 3e49cab..cf24292 100644
 | |
| --- a/ssl/ssl_lib.c
 | |
| +++ b/ssl/ssl_lib.c
 | |
| @@ -388,6 +388,13 @@ SSL *SSL_new(SSL_CTX *ctx)
 | |
|  	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
 | |
| @@ -648,6 +655,11 @@ void SSL_free(SSL *s)
 | |
|  		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);
 | |
|  
 | |
| @@ -3361,32 +3373,54 @@ int SSL_use_psk_identity_hint(SSL *s, const char *identity_hint)
 | |
|  	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 44268e7..cdd198c 100644
 | |
| --- a/ssl/ssl_sess.c
 | |
| +++ b/ssl/ssl_sess.c
 | |
| @@ -437,6 +437,18 @@ int ssl_get_new_session(SSL *s, int session)
 | |
|  			}
 | |
|  #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
 | |
|  		{
 | |
| -- 
 | |
| 2.0.0.526.g5318336
 | |
| 
 |