wbrawner/openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Propagate TLS 1.3 sigalgs through tls1_set_sigalgs()

Browse source 
Our historical SSL{,_CTX}_set_sigalgs() APIs take an array of
NID pairs (hash and signature), and our parser for manually
specifying unified sigalgs (that do not necessarily correspond
to an actual signature+hash pair) was transiting via (the implementation
of) this historical API.  The TLS 1.3 draft-23 has introduced
signature schemes that have identical signature type and hash type,
differing only in the (RSA) public key OID, which prevents
the rsa_pss_pss_* schemes from being properly identified and
sent on the wire.

To fix the issue, parse sigalg strings directly into SIGALG_LOOKUP
objects, and pass around an array of uint16 wire protocol values
instead of NID pairs.  The old interface is retained for API
compatibility but will become less and less useful with time.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5068)
This commit is contained in:
Benjamin Kaduk 2018-01-17 11:55:29 -06:00
parent f55e99f7dd
commit fd5e1a8c4a
2 changed files with 59 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
ssl/ssl_locl.h
View file

@ -2445,6 +2445,8 @@ __owur int tls_use_ticket(SSL *s);
void ssl_set_sig_mask(uint32_t *pmask_a, SSL *s, int op);
__owur int tls1_set_sigalgs_list(CERT *c, const char *str, int client);
__owur int tls1_set_raw_sigalgs(CERT *c, const uint16_t *psigs, size_t salglen,
int client);
__owur int tls1_set_sigalgs(CERT *c, const int *salg, size_t salglen,
int client);
int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain,

73
ssl/t1_lib.c
View file

@ -1697,7 +1697,8 @@ int SSL_get_shared_sigalgs(SSL *s, int idx,
typedef struct {
size_t sigalgcnt;
int sigalgs[TLS_MAX_SIGALGCNT];
/* TLSEXT_SIGALG_XXX values */
uint16_t sigalgs[TLS_MAX_SIGALGCNT];
} sig_cb_st;
static void get_sigorhash(int *psig, int *phash, const char *str)
@ -1723,6 +1724,7 @@ static int sig_cb(const char *elem, int len, void *arg)
{
sig_cb_st *sarg = arg;
size_t i;
const SIGALG_LOOKUP *s;
char etmp[TLS_MAX_SIGSTRING_LEN], *p;
int sig_alg = NID_undef, hash_alg = NID_undef;
if (elem == NULL)
@ -1734,18 +1736,25 @@ static int sig_cb(const char *elem, int len, void *arg)
memcpy(etmp, elem, len);
etmp[len] = 0;
p = strchr(etmp, '+');
/* See if we have a match for TLS 1.3 names */
/*
* We only allow SignatureSchemes listed in the sigalg_lookup_tbl;
* if there's no '+' in the provided name, look for the new-style combined
* name. If not, match both sig+hash to find the needed SIGALG_LOOKUP.
* Just sig+hash is not unique since TLS 1.3 adds rsa_pss_pss_* and
* rsa_pss_rsae_* that differ only by public key OID; in such cases
* we will pick the _rsae_ variant, by virtue of them appearing earlier
* in the table.
*/
if (p == NULL) {
const SIGALG_LOOKUP *s;
for (i = 0, s = sigalg_lookup_tbl; i < OSSL_NELEM(sigalg_lookup_tbl);
i++, s++) {
if (s->name != NULL && strcmp(etmp, s->name) == 0) {
sig_alg = s->sig;
hash_alg = s->hash;
sarg->sigalgs[sarg->sigalgcnt++] = s->sigalg;
break;
}
}
if (i == OSSL_NELEM(sigalg_lookup_tbl))
return 0;
} else {
*p = 0;
p++;
@ -1753,17 +1762,26 @@ static int sig_cb(const char *elem, int len, void *arg)
return 0;
get_sigorhash(&sig_alg, &hash_alg, etmp);
get_sigorhash(&sig_alg, &hash_alg, p);
}
if (sig_alg == NID_undef || (p != NULL && hash_alg == NID_undef))
return 0;
for (i = 0; i < sarg->sigalgcnt; i += 2) {
if (sarg->sigalgs[i] == sig_alg && sarg->sigalgs[i + 1] == hash_alg)
if (sig_alg == NID_undef || hash_alg == NID_undef)
return 0;
for (i = 0, s = sigalg_lookup_tbl; i < OSSL_NELEM(sigalg_lookup_tbl);
i++, s++) {
if (s->hash == hash_alg && s->sig == sig_alg) {
sarg->sigalgs[sarg->sigalgcnt++] = s->sigalg;
break;
}
}
if (i == OSSL_NELEM(sigalg_lookup_tbl))
return 0;
}
sarg->sigalgs[sarg->sigalgcnt++] = hash_alg;
sarg->sigalgs[sarg->sigalgcnt++] = sig_alg;
/* Reject duplicates */
for (i = 0; i < sarg->sigalgcnt - 1; i++) {
if (sarg->sigalgs[i] == sarg->sigalgs[sarg->sigalgcnt]) {
sarg->sigalgcnt--;
return 0;
}
}
return 1;
}
@ -1779,7 +1797,30 @@ int tls1_set_sigalgs_list(CERT *c, const char *str, int client)
return 0;
if (c == NULL)
return 1;
return tls1_set_sigalgs(c, sig.sigalgs, sig.sigalgcnt, client);
return tls1_set_raw_sigalgs(c, sig.sigalgs, sig.sigalgcnt, client);
}
int tls1_set_raw_sigalgs(CERT *c, const uint16_t *psigs, size_t salglen,
int client)
{
uint16_t *sigalgs;
sigalgs = OPENSSL_malloc(salglen * sizeof(*sigalgs));
if (sigalgs == NULL)
return 0;
memcpy(sigalgs, psigs, salglen * sizeof(*sigalgs));
if (client) {
OPENSSL_free(c->client_sigalgs);
c->client_sigalgs = sigalgs;
c->client_sigalgslen = salglen;
} else {
OPENSSL_free(c->conf_sigalgs);
c->conf_sigalgs = sigalgs;
c->conf_sigalgslen = salglen;
}
return 1;
}
int tls1_set_sigalgs(CERT *c, const int *psig_nids, size_t salglen, int client)