Move 'shared_sigalgs' from cert_st to ssl_st
It was only ever in cert_st because ssl_st was a public structure
and could not be modified without breaking the API. However, both
structures are now opaque, and thus we can freely change their layout
without breaking applications. In this case, keeping the shared
sigalgs in the SSL object prevents complications wherein they would
inadvertently get cleared during SSL_set_SSL_CTX() (e.g., as run
during a cert_cb).
Fixes #9099
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9157)
(cherry picked from commit 29948ac80c
)
This commit is contained in:
parent
572492aaf0
commit
915430a0a9
4 changed files with 42 additions and 40 deletions
|
@ -154,8 +154,6 @@ CERT *ssl_cert_dup(CERT *cert)
|
|||
ret->client_sigalgslen = cert->client_sigalgslen;
|
||||
} else
|
||||
ret->client_sigalgs = NULL;
|
||||
/* Shared sigalgs also NULL */
|
||||
ret->shared_sigalgs = NULL;
|
||||
/* Copy any custom client certificate types */
|
||||
if (cert->ctype) {
|
||||
ret->ctype = OPENSSL_memdup(cert->ctype, cert->ctype_len);
|
||||
|
@ -240,7 +238,6 @@ void ssl_cert_free(CERT *c)
|
|||
ssl_cert_clear_certs(c);
|
||||
OPENSSL_free(c->conf_sigalgs);
|
||||
OPENSSL_free(c->client_sigalgs);
|
||||
OPENSSL_free(c->shared_sigalgs);
|
||||
OPENSSL_free(c->ctype);
|
||||
X509_STORE_free(c->verify_store);
|
||||
X509_STORE_free(c->chain_store);
|
||||
|
|
|
@ -628,6 +628,11 @@ int SSL_clear(SSL *s)
|
|||
/* Clear the verification result peername */
|
||||
X509_VERIFY_PARAM_move_peername(s->param, NULL);
|
||||
|
||||
/* Clear any shared connection state */
|
||||
OPENSSL_free(s->shared_sigalgs);
|
||||
s->shared_sigalgs = NULL;
|
||||
s->shared_sigalgslen = 0;
|
||||
|
||||
/*
|
||||
* Check to see if we were changed into a different method, if so, revert
|
||||
* back.
|
||||
|
@ -1173,6 +1178,7 @@ void SSL_free(SSL *s)
|
|||
clear_ciphers(s);
|
||||
|
||||
ssl_cert_free(s->cert);
|
||||
OPENSSL_free(s->shared_sigalgs);
|
||||
/* Free up if allocated */
|
||||
|
||||
OPENSSL_free(s->ext.hostname);
|
||||
|
|
|
@ -1474,6 +1474,13 @@ struct ssl_st {
|
|||
/* Callback to determine if early_data is acceptable or not */
|
||||
SSL_allow_early_data_cb_fn allow_early_data_cb;
|
||||
void *allow_early_data_cb_data;
|
||||
|
||||
/*
|
||||
* Signature algorithms shared by client and server: cached because these
|
||||
* are used most often.
|
||||
*/
|
||||
const struct sigalg_lookup_st **shared_sigalgs;
|
||||
size_t shared_sigalgslen;
|
||||
};
|
||||
|
||||
/*
|
||||
|
@ -1907,12 +1914,6 @@ typedef struct cert_st {
|
|||
uint16_t *client_sigalgs;
|
||||
/* Size of above array */
|
||||
size_t client_sigalgslen;
|
||||
/*
|
||||
* Signature algorithms shared by client and server: cached because these
|
||||
* are used most often.
|
||||
*/
|
||||
const SIGALG_LOOKUP **shared_sigalgs;
|
||||
size_t shared_sigalgslen;
|
||||
/*
|
||||
* Certificate setup callback: if set is called whenever a certificate
|
||||
* may be required (client or server). the callback can then examine any
|
||||
|
|
60
ssl/t1_lib.c
60
ssl/t1_lib.c
|
@ -578,7 +578,6 @@ static int tls1_check_cert_param(SSL *s, X509 *x, int check_ee_md)
|
|||
if (check_ee_md && tls1_suiteb(s)) {
|
||||
int check_md;
|
||||
size_t i;
|
||||
CERT *c = s->cert;
|
||||
|
||||
/* Check to see we have necessary signing algorithm */
|
||||
if (group_id == TLSEXT_curve_P_256)
|
||||
|
@ -587,8 +586,8 @@ static int tls1_check_cert_param(SSL *s, X509 *x, int check_ee_md)
|
|||
check_md = NID_ecdsa_with_SHA384;
|
||||
else
|
||||
return 0; /* Should never happen */
|
||||
for (i = 0; i < c->shared_sigalgslen; i++) {
|
||||
if (check_md == c->shared_sigalgs[i]->sigandhash)
|
||||
for (i = 0; i < s->shared_sigalgslen; i++) {
|
||||
if (check_md == s->shared_sigalgs[i]->sigandhash)
|
||||
return 1;;
|
||||
}
|
||||
return 0;
|
||||
|
@ -1215,9 +1214,9 @@ int tls1_set_server_sigalgs(SSL *s)
|
|||
size_t i;
|
||||
|
||||
/* Clear any shared signature algorithms */
|
||||
OPENSSL_free(s->cert->shared_sigalgs);
|
||||
s->cert->shared_sigalgs = NULL;
|
||||
s->cert->shared_sigalgslen = 0;
|
||||
OPENSSL_free(s->shared_sigalgs);
|
||||
s->shared_sigalgs = NULL;
|
||||
s->shared_sigalgslen = 0;
|
||||
/* Clear certificate validity flags */
|
||||
for (i = 0; i < SSL_PKEY_NUM; i++)
|
||||
s->s3->tmp.valid_flags[i] = 0;
|
||||
|
@ -1252,7 +1251,7 @@ int tls1_set_server_sigalgs(SSL *s)
|
|||
SSL_F_TLS1_SET_SERVER_SIGALGS, ERR_R_INTERNAL_ERROR);
|
||||
return 0;
|
||||
}
|
||||
if (s->cert->shared_sigalgs != NULL)
|
||||
if (s->shared_sigalgs != NULL)
|
||||
return 1;
|
||||
|
||||
/* Fatal error if no shared signature algorithms */
|
||||
|
@ -1724,9 +1723,9 @@ static int tls1_set_shared_sigalgs(SSL *s)
|
|||
CERT *c = s->cert;
|
||||
unsigned int is_suiteb = tls1_suiteb(s);
|
||||
|
||||
OPENSSL_free(c->shared_sigalgs);
|
||||
c->shared_sigalgs = NULL;
|
||||
c->shared_sigalgslen = 0;
|
||||
OPENSSL_free(s->shared_sigalgs);
|
||||
s->shared_sigalgs = NULL;
|
||||
s->shared_sigalgslen = 0;
|
||||
/* If client use client signature algorithms if not NULL */
|
||||
if (!s->server && c->client_sigalgs && !is_suiteb) {
|
||||
conf = c->client_sigalgs;
|
||||
|
@ -1757,8 +1756,8 @@ static int tls1_set_shared_sigalgs(SSL *s)
|
|||
} else {
|
||||
salgs = NULL;
|
||||
}
|
||||
c->shared_sigalgs = salgs;
|
||||
c->shared_sigalgslen = nmatch;
|
||||
s->shared_sigalgs = salgs;
|
||||
s->shared_sigalgslen = nmatch;
|
||||
return 1;
|
||||
}
|
||||
|
||||
|
@ -1819,7 +1818,6 @@ int tls1_process_sigalgs(SSL *s)
|
|||
{
|
||||
size_t i;
|
||||
uint32_t *pvalid = s->s3->tmp.valid_flags;
|
||||
CERT *c = s->cert;
|
||||
|
||||
if (!tls1_set_shared_sigalgs(s))
|
||||
return 0;
|
||||
|
@ -1827,8 +1825,8 @@ int tls1_process_sigalgs(SSL *s)
|
|||
for (i = 0; i < SSL_PKEY_NUM; i++)
|
||||
pvalid[i] = 0;
|
||||
|
||||
for (i = 0; i < c->shared_sigalgslen; i++) {
|
||||
const SIGALG_LOOKUP *sigptr = c->shared_sigalgs[i];
|
||||
for (i = 0; i < s->shared_sigalgslen; i++) {
|
||||
const SIGALG_LOOKUP *sigptr = s->shared_sigalgs[i];
|
||||
int idx = sigptr->sig_idx;
|
||||
|
||||
/* Ignore PKCS1 based sig algs in TLSv1.3 */
|
||||
|
@ -1875,12 +1873,12 @@ int SSL_get_shared_sigalgs(SSL *s, int idx,
|
|||
unsigned char *rsig, unsigned char *rhash)
|
||||
{
|
||||
const SIGALG_LOOKUP *shsigalgs;
|
||||
if (s->cert->shared_sigalgs == NULL
|
||||
if (s->shared_sigalgs == NULL
|
||||
|| idx < 0
|
||||
|| idx >= (int)s->cert->shared_sigalgslen
|
||||
|| s->cert->shared_sigalgslen > INT_MAX)
|
||||
|| idx >= (int)s->shared_sigalgslen
|
||||
|| s->shared_sigalgslen > INT_MAX)
|
||||
return 0;
|
||||
shsigalgs = s->cert->shared_sigalgs[idx];
|
||||
shsigalgs = s->shared_sigalgs[idx];
|
||||
if (phash != NULL)
|
||||
*phash = shsigalgs->hash;
|
||||
if (psign != NULL)
|
||||
|
@ -1891,7 +1889,7 @@ int SSL_get_shared_sigalgs(SSL *s, int idx,
|
|||
*rsig = (unsigned char)(shsigalgs->sigalg & 0xff);
|
||||
if (rhash != NULL)
|
||||
*rhash = (unsigned char)((shsigalgs->sigalg >> 8) & 0xff);
|
||||
return (int)s->cert->shared_sigalgslen;
|
||||
return (int)s->shared_sigalgslen;
|
||||
}
|
||||
|
||||
/* Maximum possible number of unique entries in sigalgs array */
|
||||
|
@ -2072,7 +2070,7 @@ int tls1_set_sigalgs(CERT *c, const int *psig_nids, size_t salglen, int client)
|
|||
return 0;
|
||||
}
|
||||
|
||||
static int tls1_check_sig_alg(CERT *c, X509 *x, int default_nid)
|
||||
static int tls1_check_sig_alg(SSL *s, X509 *x, int default_nid)
|
||||
{
|
||||
int sig_nid;
|
||||
size_t i;
|
||||
|
@ -2081,8 +2079,8 @@ static int tls1_check_sig_alg(CERT *c, X509 *x, int default_nid)
|
|||
sig_nid = X509_get_signature_nid(x);
|
||||
if (default_nid)
|
||||
return sig_nid == default_nid ? 1 : 0;
|
||||
for (i = 0; i < c->shared_sigalgslen; i++)
|
||||
if (sig_nid == c->shared_sigalgs[i]->sigandhash)
|
||||
for (i = 0; i < s->shared_sigalgslen; i++)
|
||||
if (sig_nid == s->shared_sigalgs[i]->sigandhash)
|
||||
return 1;
|
||||
return 0;
|
||||
}
|
||||
|
@ -2240,14 +2238,14 @@ int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain,
|
|||
}
|
||||
}
|
||||
/* Check signature algorithm of each cert in chain */
|
||||
if (!tls1_check_sig_alg(c, x, default_nid)) {
|
||||
if (!tls1_check_sig_alg(s, x, default_nid)) {
|
||||
if (!check_flags)
|
||||
goto end;
|
||||
} else
|
||||
rv |= CERT_PKEY_EE_SIGNATURE;
|
||||
rv |= CERT_PKEY_CA_SIGNATURE;
|
||||
for (i = 0; i < sk_X509_num(chain); i++) {
|
||||
if (!tls1_check_sig_alg(c, sk_X509_value(chain, i), default_nid)) {
|
||||
if (!tls1_check_sig_alg(s, sk_X509_value(chain, i), default_nid)) {
|
||||
if (check_flags) {
|
||||
rv &= ~CERT_PKEY_CA_SIGNATURE;
|
||||
break;
|
||||
|
@ -2607,8 +2605,8 @@ int tls_choose_sigalg(SSL *s, int fatalerrs)
|
|||
#endif
|
||||
|
||||
/* Look for a certificate matching shared sigalgs */
|
||||
for (i = 0; i < s->cert->shared_sigalgslen; i++) {
|
||||
lu = s->cert->shared_sigalgs[i];
|
||||
for (i = 0; i < s->shared_sigalgslen; i++) {
|
||||
lu = s->shared_sigalgs[i];
|
||||
sig_idx = -1;
|
||||
|
||||
/* Skip SHA1, SHA224, DSA and RSA if not PSS */
|
||||
|
@ -2642,7 +2640,7 @@ int tls_choose_sigalg(SSL *s, int fatalerrs)
|
|||
}
|
||||
break;
|
||||
}
|
||||
if (i == s->cert->shared_sigalgslen) {
|
||||
if (i == s->shared_sigalgslen) {
|
||||
if (!fatalerrs)
|
||||
return 1;
|
||||
SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_TLS_CHOOSE_SIGALG,
|
||||
|
@ -2675,8 +2673,8 @@ int tls_choose_sigalg(SSL *s, int fatalerrs)
|
|||
* Find highest preference signature algorithm matching
|
||||
* cert type
|
||||
*/
|
||||
for (i = 0; i < s->cert->shared_sigalgslen; i++) {
|
||||
lu = s->cert->shared_sigalgs[i];
|
||||
for (i = 0; i < s->shared_sigalgslen; i++) {
|
||||
lu = s->shared_sigalgs[i];
|
||||
|
||||
if (s->server) {
|
||||
if ((sig_idx = tls12_get_cert_sigalg_idx(s, lu)) == -1)
|
||||
|
@ -2703,7 +2701,7 @@ int tls_choose_sigalg(SSL *s, int fatalerrs)
|
|||
#endif
|
||||
break;
|
||||
}
|
||||
if (i == s->cert->shared_sigalgslen) {
|
||||
if (i == s->shared_sigalgslen) {
|
||||
if (!fatalerrs)
|
||||
return 1;
|
||||
SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE,
|
||||
|
|
Loading…
Reference in a new issue