Skip to content

Commit

Permalink
Move parsing and construction of CA names to separate functions
Browse files Browse the repository at this point in the history 
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #2918)
  • Loading branch information
@snhenson
snhenson committed Mar 17, 2017
1 parent fa013b6 commit 5d6cca0
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 82 deletions.
1 change: 1 addition & 0 deletions include/openssl/ssl.h
View file
Expand Up @@ -2174,6 +2174,7 @@ int ERR_load_SSL_strings(void);
# define SSL_F_OSSL_STATEM_SERVER13_READ_TRANSITION 437
# define SSL_F_OSSL_STATEM_SERVER_CONSTRUCT_MESSAGE 431
# define SSL_F_OSSL_STATEM_SERVER_READ_TRANSITION 418
# define SSL_F_PARSE_CA_NAMES 541
# define SSL_F_PROCESS_KEY_SHARE_EXT 439
# define SSL_F_READ_STATE_MACHINE 352
# define SSL_F_SET_CLIENT_CIPHERSUITE 540
Expand Down
1 change: 1 addition & 0 deletions ssl/ssl_err.c
View file
Expand Up @@ -74,6 +74,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
"ossl_statem_server_construct_message"},
{ERR_FUNC(SSL_F_OSSL_STATEM_SERVER_READ_TRANSITION),
"ossl_statem_server_read_transition"},
{ERR_FUNC(SSL_F_PARSE_CA_NAMES), "parse_ca_names"},
{ERR_FUNC(SSL_F_PROCESS_KEY_SHARE_EXT), "process_key_share_ext"},
{ERR_FUNC(SSL_F_READ_STATE_MACHINE), "read_state_machine"},
{ERR_FUNC(SSL_F_SET_CLIENT_CIPHERSUITE), "set_client_ciphersuite"},
Expand Down
58 changes: 4 additions & 54 deletions ssl/statem/statem_clnt.c
View file
Expand Up @@ -65,7 +65,6 @@ static MSG_PROCESS_RETURN tls_process_encrypted_extensions(SSL *s, PACKET *pkt);

static ossl_inline int cert_req_allowed(SSL *s);
static int key_exchange_expected(SSL *s);
static int ca_dn_cmp(const X509_NAME *const *a, const X509_NAME *const *b);
static int ssl_cipher_list_to_bytes(SSL *s, STACK_OF(SSL_CIPHER) *sk,
WPACKET *pkt);

Expand Down Expand Up @@ -2328,16 +2327,8 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt)
MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt)
{
int ret = MSG_PROCESS_ERROR;
unsigned int i, name_len;
X509_NAME *xn = NULL;
const unsigned char *namestart, *namebytes;
STACK_OF(X509_NAME) *ca_sk = NULL;
PACKET cadns;

if ((ca_sk = sk_X509_NAME_new(ca_dn_cmp)) == NULL) {
SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_MALLOC_FAILURE);
goto err;
}
int al = -1;
unsigned int i;

if (SSL_IS_TLS13(s)) {
PACKET reqctx;
Expand Down Expand Up @@ -2396,42 +2387,11 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt)
}

/* get the CA RDNs */
if (!PACKET_get_length_prefixed_2(pkt, &cadns)) {
ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, SSL_R_LENGTH_MISMATCH);
if (!parse_ca_names(s, pkt, &al)) {
ssl3_send_alert(s, SSL3_AL_FATAL, al);
goto err;
}

while (PACKET_remaining(&cadns)) {
if (!PACKET_get_net_2(&cadns, &name_len)
|| !PACKET_get_bytes(&cadns, &namebytes, name_len)) {
ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
SSL_R_LENGTH_MISMATCH);
goto err;
}

namestart = namebytes;

if ((xn = d2i_X509_NAME(NULL, (const unsigned char **)&namebytes,
name_len)) == NULL) {
ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_ASN1_LIB);
goto err;
}

if (namebytes != (namestart + name_len)) {
ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST,
SSL_R_CA_DN_LENGTH_MISMATCH);
goto err;
}
if (!sk_X509_NAME_push(ca_sk, xn)) {
SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_MALLOC_FAILURE);
goto err;
}
xn = NULL;
}
/* TODO(TLS1.3) need to parse and process extensions, for now ignore */
if (SSL_IS_TLS13(s)) {
PACKET reqexts;
Expand All @@ -2452,25 +2412,15 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt)

/* we should setup a certificate to return.... */
s->s3->tmp.cert_req = 1;
sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);
s->s3->tmp.ca_names = ca_sk;
ca_sk = NULL;

ret = MSG_PROCESS_CONTINUE_PROCESSING;
goto done;
err:
ossl_statem_set_error(s);
done:
X509_NAME_free(xn);
sk_X509_NAME_pop_free(ca_sk, X509_NAME_free);
return ret;
}

static int ca_dn_cmp(const X509_NAME *const *a, const X509_NAME *const *b)
{
return (X509_NAME_cmp(*a, *b));
}

MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
{
int al = SSL_AD_DECODE_ERROR;
Expand Down
95 changes: 95 additions & 0 deletions ssl/statem/statem_lib.c
View file
Expand Up @@ -1907,3 +1907,98 @@ int create_synthetic_message_hash(SSL *s)

return 1;
}

static int ca_dn_cmp(const X509_NAME *const *a, const X509_NAME *const *b)
{
return X509_NAME_cmp(*a, *b);
}

int parse_ca_names(SSL *s, PACKET *pkt, int *al)
{
STACK_OF(X509_NAME) *ca_sk = sk_X509_NAME_new(ca_dn_cmp);
X509_NAME *xn = NULL;
PACKET cadns;

if (ca_sk == NULL) {
SSLerr(SSL_F_PARSE_CA_NAMES, ERR_R_MALLOC_FAILURE);
goto decerr;
}
/* get the CA RDNs */
if (!PACKET_get_length_prefixed_2(pkt, &cadns)) {
*al = SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_PARSE_CA_NAMES, SSL_R_LENGTH_MISMATCH);
goto decerr;
}

while (PACKET_remaining(&cadns)) {
const unsigned char *namestart, *namebytes;
unsigned int name_len;

if (!PACKET_get_net_2(&cadns, &name_len)
|| !PACKET_get_bytes(&cadns, &namebytes, name_len)) {
SSLerr(SSL_F_PARSE_CA_NAMES, SSL_R_LENGTH_MISMATCH);
goto decerr;
}

namestart = namebytes;
if ((xn = d2i_X509_NAME(NULL, &namebytes, name_len)) == NULL) {
SSLerr(SSL_F_PARSE_CA_NAMES, ERR_R_ASN1_LIB);
goto decerr;
}
if (namebytes != (namestart + name_len)) {
SSLerr(SSL_F_PARSE_CA_NAMES, SSL_R_CA_DN_LENGTH_MISMATCH);
goto decerr;
}

if (!sk_X509_NAME_push(ca_sk, xn)) {
SSLerr(SSL_F_PARSE_CA_NAMES, ERR_R_MALLOC_FAILURE);
*al = SSL_AD_INTERNAL_ERROR;
goto err;
}
xn = NULL;
}

sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);
s->s3->tmp.ca_names = ca_sk;

return 1;

decerr:
*al = SSL_AD_DECODE_ERROR;
err:
sk_X509_NAME_pop_free(ca_sk, X509_NAME_free);
X509_NAME_free(xn);
return 0;
}

int construct_ca_names(SSL *s, WPACKET *pkt)
{
STACK_OF(X509_NAME) *ca_sk = SSL_get_client_CA_list(s);

/* Start sub-packet for client CA list */
if (!WPACKET_start_sub_packet_u16(pkt))
return 0;

if (ca_sk != NULL) {
int i;

for (i = 0; i < sk_X509_NAME_num(ca_sk); i++) {
unsigned char *namebytes;
X509_NAME *name = sk_X509_NAME_value(ca_sk, i);
int namelen;

if (name == NULL
|| (namelen = i2d_X509_NAME(name, NULL)) < 0
|| !WPACKET_sub_allocate_bytes_u16(pkt, namelen,
&namebytes)
|| i2d_X509_NAME(name, &namebytes) != namelen) {
return 0;
}
}
}

if (!WPACKET_close(pkt))
return 0;

return 1;
}
3 changes: 3 additions & 0 deletions ssl/statem/statem_locl.h
View file
Expand Up @@ -80,6 +80,9 @@ typedef int (*confunc_f) (SSL *s, WPACKET *pkt);
int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups,
size_t num_groups, int checkallow);
int create_synthetic_message_hash(SSL *s);
int parse_ca_names(SSL *s, PACKET *pkt, int *al);
int construct_ca_names(SSL *s, WPACKET *pkt);

/*
* TLS/DTLS client state machine functions
*/
Expand Down
29 changes: 1 addition & 28 deletions ssl/statem/statem_srvr.c
View file
Expand Up @@ -2499,9 +2499,6 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)

int tls_construct_certificate_request(SSL *s, WPACKET *pkt)
{
int i;
STACK_OF(X509_NAME) *sk = NULL;

if (SSL_IS_TLS13(s)) {
/* TODO(TLS1.3) for now send empty request context */
if (!WPACKET_put_bytes_u8(pkt, 0)) {
Expand Down Expand Up @@ -2533,35 +2530,11 @@ int tls_construct_certificate_request(SSL *s, WPACKET *pkt)
}
}

/* Start sub-packet for client CA list */
if (!WPACKET_start_sub_packet_u16(pkt)) {
if (!construct_ca_names(s, pkt)) {
SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
goto err;
}

sk = SSL_get_client_CA_list(s);
if (sk != NULL) {
for (i = 0; i < sk_X509_NAME_num(sk); i++) {
unsigned char *namebytes;
X509_NAME *name = sk_X509_NAME_value(sk, i);
int namelen;

if (name == NULL
|| (namelen = i2d_X509_NAME(name, NULL)) < 0
|| !WPACKET_sub_allocate_bytes_u16(pkt, namelen,
&namebytes)
|| i2d_X509_NAME(name, &namebytes) != namelen) {
SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST,
ERR_R_INTERNAL_ERROR);
goto err;
}
}
}
/* else no CA names */
if (!WPACKET_close(pkt)) {
SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
goto err;
}
/*
* TODO(TLS1.3) implement configurable certificate_extensions
* For now just send zero length extensions.
Expand Down

0 comments on commit 5d6cca0

Please sign in to comment.