On 02/07/2012 09:25 PM, Trevor Perrin via RT wrote:
> Hello,
>
> I think the "srp" ClientHello extension is being sent incorrectly in
> 1.0.1 beta 2.
trevor's patch corrects an immediate problem but there were others:
- the length fields are not correctly assured to be within 1 an 255
- receiving two srp extensions creates a memory leak
- extra data after the extension are ignored
fixed and some comments added in the attached patch.
diff -r -c -p openssl-1.0.1-beta2/ssl/s3_lib.c openssl-1.0.1-beta2ps/ssl/s3_lib.c
*** openssl-1.0.1-beta2/ssl/s3_lib.c 2012-01-01 00:00:35.000000000 +0100
--- openssl-1.0.1-beta2ps/ssl/s3_lib.c 2012-02-08 13:29:09.878025062 +0100
*************** long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd
*** 3589,3595 ****
ctx->srp_ctx.login = NULL;
if (parg == NULL)
break;
! if (strlen((char *)parg) > 254)
{
SSLerr(SSL_F_SSL3_CTX_CTRL, SSL_R_INVALID_SRP_USERNAME);
return 0;
--- 3589,3595 ----
ctx->srp_ctx.login = NULL;
if (parg == NULL)
break;
! if (strlen((const char *)parg) > 255 || strlen((const char *)parg) < 1)
{
SSLerr(SSL_F_SSL3_CTX_CTRL, SSL_R_INVALID_SRP_USERNAME);
return 0;
diff -r -c -p openssl-1.0.1-beta2/ssl/t1_lib.c openssl-1.0.1-beta2ps/ssl/t1_lib.c
*** openssl-1.0.1-beta2/ssl/t1_lib.c 2012-01-05 01:23:31.000000000 +0100
--- openssl-1.0.1-beta2ps/ssl/t1_lib.c 2012-02-08 13:29:04.489973355 +0100
*************** unsigned char *ssl_add_clienthello_tlsex
*** 432,456 ****
}
#ifndef OPENSSL_NO_SRP
! #define MIN(x,y) (((x)<(y))?(x):(y))
! /* we add SRP username the first time only if we have one! */
if (s->srp_ctx.login != NULL)
! {/* Add TLS extension SRP username to the Client Hello message */
! int login_len = MIN(strlen(s->srp_ctx.login) + 1, 255);
! long lenmax;
! if ((lenmax = limit - ret - 5) < 0) return NULL;
! if (login_len > lenmax) return NULL;
! if (login_len > 255)
{
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return NULL;
! }
s2n(TLSEXT_TYPE_srp,ret);
s2n(login_len+1,ret);
!
! (*ret++) = (unsigned char) MIN(strlen(s->srp_ctx.login), 254);
! memcpy(ret, s->srp_ctx.login, MIN(strlen(s->srp_ctx.login), 254));
ret+=login_len;
}
#endif
--- 432,460 ----
}
#ifndef OPENSSL_NO_SRP
! /* Add SRP username if there is one */
if (s->srp_ctx.login != NULL)
! { /* Add TLS extension SRP username to the Client Hello message */
! int login_len = strlen(s->srp_ctx.login);
! if (login_len > 255 || login_len == 0)
{
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return NULL;
! }
!
! /* check for enough space.
! 4 for the srp type type and entension length
! 1 for the srp user identity
! + srp user identity length
! */
! if ((limit - ret - 5 - login_len) < 0) return NULL;
!
! /* fill in the extension */
s2n(TLSEXT_TYPE_srp,ret);
s2n(login_len+1,ret);
! (*ret++) = (unsigned char) login_len;
! memcpy(ret, s->srp_ctx.login, login_len);
ret+=login_len;
}
#endif
*************** int ssl_parse_clienthello_tlsext(SSL *s,
*** 1007,1019 ****
#ifndef OPENSSL_NO_SRP
else if (type == TLSEXT_TYPE_srp)
{
! if (size > 0)
{
! len = data[0];
! if ((s->srp_ctx.login = OPENSSL_malloc(len+1)) == NULL)
! return -1;
! memcpy(s->srp_ctx.login, &data[1], len);
! s->srp_ctx.login[len]='\0';
}
}
#endif
--- 1011,1035 ----
#ifndef OPENSSL_NO_SRP
else if (type == TLSEXT_TYPE_srp)
{
! if (size <= 0 || ((len = data[0])) != (size -1))
! {
! *al = SSL_AD_DECODE_ERROR;
! return 0;
! }
! if (s->srp_ctx.login != NULL)
! {
! *al = SSL_AD_DECODE_ERROR;
! return 0;
! }
! if ((s->srp_ctx.login = OPENSSL_malloc(len+1)) == NULL)
! return -1;
! memcpy(s->srp_ctx.login, &data[1], len);
! s->srp_ctx.login[len]='\0';
!
! if (strlen(s->srp_ctx.login) != len)
{
! *al = SSL_AD_DECODE_ERROR;
! return 0;
}
}
#endif
