Univention Bugzilla – Attachment 7510 Details for
Bug 40189
openssl: Denial of service (3.2)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
CVE-2016-0798.patch
CVE-2016-0798.patch (text/plain), 12.36 KB, created by
Arvid Requate
on 2016-03-01 16:53 CET
(
hide
)
Description:
CVE-2016-0798.patch
Filename:
MIME Type:
Creator:
Arvid Requate
Created:
2016-03-01 16:53 CET
Size:
12.36 KB
patch
obsolete
>commit 59a908f1e8380412a81392c468b83bf6071beb2a >Author: Emilia Kasper <emilia@openssl.org> >Date: Wed Feb 24 12:59:59 2016 +0100 > > CVE-2016-0798: avoid memory leak in SRP > > The SRP user database lookup method SRP_VBASE_get_by_user had confusing > memory management semantics; the returned pointer was sometimes newly > allocated, and sometimes owned by the callee. The calling code has no > way of distinguishing these two cases. > > Specifically, SRP servers that configure a secret seed to hide valid > login information are vulnerable to a memory leak: an attacker > connecting with an invalid username can cause a memory leak of around > 300 bytes per connection. > > Servers that do not configure SRP, or configure SRP but do not configure > a seed are not vulnerable. > > In Apache, the seed directive is known as SSLSRPUnknownUserSeed. > > To mitigate the memory leak, the seed handling in SRP_VBASE_get_by_user > is now disabled even if the user has configured a seed. > > Applications are advised to migrate to SRP_VBASE_get1_by_user. However, > note that OpenSSL makes no strong guarantees about the > indistinguishability of valid and invalid logins. In particular, > computations are currently not carried out in constant time. > > Reviewed-by: Rich Salz <rsalz@openssl.org> > >diff --git a/CHANGES b/CHANGES >index cdc4e6f..b95a3ed 100644 >--- a/CHANGES >+++ b/CHANGES >@@ -4,7 +4,24 @@ > > Changes between 1.0.1r and 1.0.1s [xx XXX xxxx] > >- *) >+ *) Disable SRP fake user seed to address a server memory leak. >+ >+ Add a new method SRP_VBASE_get1_by_user that handles the seed properly. >+ >+ SRP_VBASE_get_by_user had inconsistent memory management behaviour. >+ In order to fix an unavoidable memory leak, SRP_VBASE_get_by_user >+ was changed to ignore the "fake user" SRP seed, even if the seed >+ is configured. >+ >+ Users should use SRP_VBASE_get1_by_user instead. Note that in >+ SRP_VBASE_get1_by_user, caller must free the returned value. Note >+ also that even though configuring the SRP seed attempts to hide >+ invalid usernames by continuing the handshake with fake >+ credentials, this behaviour is not constant time and no strong >+ guarantees are made that the handshake is indistinguishable from >+ that of a valid user. >+ (CVE-2016-0798) >+ [Emilia Käsper] > > Changes between 1.0.1q and 1.0.1r [28 Jan 2016] > >diff --git a/apps/s_server.c b/apps/s_server.c >index a8aee77..a53cadd 100644 >--- a/apps/s_server.c >+++ b/apps/s_server.c >@@ -416,6 +416,8 @@ typedef struct srpsrvparm_st { > static int MS_CALLBACK ssl_srp_server_param_cb(SSL *s, int *ad, void *arg) > { > srpsrvparm *p = (srpsrvparm *) arg; >+ int ret = SSL3_AL_FATAL; >+ > if (p->login == NULL && p->user == NULL) { > p->login = SSL_get_srp_username(s); > BIO_printf(bio_err, "SRP username = \"%s\"\n", p->login); >@@ -424,21 +426,25 @@ static int MS_CALLBACK ssl_srp_server_param_cb(SSL *s, int *ad, void *arg) > > if (p->user == NULL) { > BIO_printf(bio_err, "User %s doesn't exist\n", p->login); >- return SSL3_AL_FATAL; >+ goto err; > } >+ > if (SSL_set_srp_server_param > (s, p->user->N, p->user->g, p->user->s, p->user->v, > p->user->info) < 0) { > *ad = SSL_AD_INTERNAL_ERROR; >- return SSL3_AL_FATAL; >+ goto err; > } > BIO_printf(bio_err, > "SRP parameters set: username = \"%s\" info=\"%s\" \n", > p->login, p->user->info); >- /* need to check whether there are memory leaks */ >+ ret = SSL_ERROR_NONE; >+ >+err: >+ SRP_user_pwd_free(p->user); > p->user = NULL; > p->login = NULL; >- return SSL_ERROR_NONE; >+ return ret; > } > > #endif >@@ -2244,9 +2250,10 @@ static int sv_body(char *hostname, int s, unsigned char *context) > #ifndef OPENSSL_NO_SRP > while (SSL_get_error(con, k) == SSL_ERROR_WANT_X509_LOOKUP) { > BIO_printf(bio_s_out, "LOOKUP renego during write\n"); >+ SRP_user_pwd_free(srp_callback_parm.user); > srp_callback_parm.user = >- SRP_VBASE_get_by_user(srp_callback_parm.vb, >- srp_callback_parm.login); >+ SRP_VBASE_get1_by_user(srp_callback_parm.vb, >+ srp_callback_parm.login); > if (srp_callback_parm.user) > BIO_printf(bio_s_out, "LOOKUP done %s\n", > srp_callback_parm.user->info); >@@ -2300,9 +2307,10 @@ static int sv_body(char *hostname, int s, unsigned char *context) > #ifndef OPENSSL_NO_SRP > while (SSL_get_error(con, i) == SSL_ERROR_WANT_X509_LOOKUP) { > BIO_printf(bio_s_out, "LOOKUP renego during read\n"); >+ SRP_user_pwd_free(srp_callback_parm.user); > srp_callback_parm.user = >- SRP_VBASE_get_by_user(srp_callback_parm.vb, >- srp_callback_parm.login); >+ SRP_VBASE_get1_by_user(srp_callback_parm.vb, >+ srp_callback_parm.login); > if (srp_callback_parm.user) > BIO_printf(bio_s_out, "LOOKUP done %s\n", > srp_callback_parm.user->info); >@@ -2387,9 +2395,10 @@ static int init_ssl_connection(SSL *con) > while (i <= 0 && SSL_get_error(con, i) == SSL_ERROR_WANT_X509_LOOKUP) { > BIO_printf(bio_s_out, "LOOKUP during accept %s\n", > srp_callback_parm.login); >+ SRP_user_pwd_free(srp_callback_parm.user); > srp_callback_parm.user = >- SRP_VBASE_get_by_user(srp_callback_parm.vb, >- srp_callback_parm.login); >+ SRP_VBASE_get1_by_user(srp_callback_parm.vb, >+ srp_callback_parm.login); > if (srp_callback_parm.user) > BIO_printf(bio_s_out, "LOOKUP done %s\n", > srp_callback_parm.user->info); >@@ -2616,9 +2625,10 @@ static int www_body(char *hostname, int s, unsigned char *context) > && SSL_get_error(con, i) == SSL_ERROR_WANT_X509_LOOKUP) { > BIO_printf(bio_s_out, "LOOKUP during accept %s\n", > srp_callback_parm.login); >+ SRP_user_pwd_free(srp_callback_parm.user); > srp_callback_parm.user = >- SRP_VBASE_get_by_user(srp_callback_parm.vb, >- srp_callback_parm.login); >+ SRP_VBASE_get1_by_user(srp_callback_parm.vb, >+ srp_callback_parm.login); > if (srp_callback_parm.user) > BIO_printf(bio_s_out, "LOOKUP done %s\n", > srp_callback_parm.user->info); >@@ -2658,9 +2668,10 @@ static int www_body(char *hostname, int s, unsigned char *context) > if (BIO_should_io_special(io) > && BIO_get_retry_reason(io) == BIO_RR_SSL_X509_LOOKUP) { > BIO_printf(bio_s_out, "LOOKUP renego during read\n"); >+ SRP_user_pwd_free(srp_callback_parm.user); > srp_callback_parm.user = >- SRP_VBASE_get_by_user(srp_callback_parm.vb, >- srp_callback_parm.login); >+ SRP_VBASE_get1_by_user(srp_callback_parm.vb, >+ srp_callback_parm.login); > if (srp_callback_parm.user) > BIO_printf(bio_s_out, "LOOKUP done %s\n", > srp_callback_parm.user->info); >diff --git a/crypto/srp/srp.h b/crypto/srp/srp.h >index d072536..028892a 100644 >--- a/crypto/srp/srp.h >+++ b/crypto/srp/srp.h >@@ -82,16 +82,21 @@ typedef struct SRP_gN_cache_st { > DECLARE_STACK_OF(SRP_gN_cache) > > typedef struct SRP_user_pwd_st { >+ /* Owned by us. */ > char *id; > BIGNUM *s; > BIGNUM *v; >+ /* Not owned by us. */ > const BIGNUM *g; > const BIGNUM *N; >+ /* Owned by us. */ > char *info; > } SRP_user_pwd; > > DECLARE_STACK_OF(SRP_user_pwd) > >+void SRP_user_pwd_free(SRP_user_pwd *user_pwd); >+ > typedef struct SRP_VBASE_st { > STACK_OF(SRP_user_pwd) *users_pwd; > STACK_OF(SRP_gN_cache) *gN_cache; >@@ -115,7 +120,12 @@ DECLARE_STACK_OF(SRP_gN) > SRP_VBASE *SRP_VBASE_new(char *seed_key); > int SRP_VBASE_free(SRP_VBASE *vb); > int SRP_VBASE_init(SRP_VBASE *vb, char *verifier_file); >+ >+/* This method ignores the configured seed and fails for an unknown user. */ > SRP_user_pwd *SRP_VBASE_get_by_user(SRP_VBASE *vb, char *username); >+/* NOTE: unlike in SRP_VBASE_get_by_user, caller owns the returned pointer.*/ >+SRP_user_pwd *SRP_VBASE_get1_by_user(SRP_VBASE *vb, char *username); >+ > char *SRP_create_verifier(const char *user, const char *pass, char **salt, > char **verifier, const char *N, const char *g); > int SRP_create_verifier_BN(const char *user, const char *pass, BIGNUM **salt, >diff --git a/crypto/srp/srp_vfy.c b/crypto/srp/srp_vfy.c >index a3f1a8a..26ad3e0 100644 >--- a/crypto/srp/srp_vfy.c >+++ b/crypto/srp/srp_vfy.c >@@ -185,7 +185,7 @@ static char *t_tob64(char *dst, const unsigned char *src, int size) > return olddst; > } > >-static void SRP_user_pwd_free(SRP_user_pwd *user_pwd) >+void SRP_user_pwd_free(SRP_user_pwd *user_pwd) > { > if (user_pwd == NULL) > return; >@@ -247,6 +247,24 @@ static int SRP_user_pwd_set_sv_BN(SRP_user_pwd *vinfo, BIGNUM *s, BIGNUM *v) > return (vinfo->s != NULL && vinfo->v != NULL); > } > >+static SRP_user_pwd *srp_user_pwd_dup(SRP_user_pwd *src) >+{ >+ SRP_user_pwd *ret; >+ >+ if (src == NULL) >+ return NULL; >+ if ((ret = SRP_user_pwd_new()) == NULL) >+ return NULL; >+ >+ SRP_user_pwd_set_gN(ret, src->g, src->N); >+ if (!SRP_user_pwd_set_ids(ret, src->id, src->info) >+ || !SRP_user_pwd_set_sv_BN(ret, BN_dup(src->s), BN_dup(src->v))) { >+ SRP_user_pwd_free(ret); >+ return NULL; >+ } >+ return ret; >+} >+ > SRP_VBASE *SRP_VBASE_new(char *seed_key) > { > SRP_VBASE *vb = (SRP_VBASE *)OPENSSL_malloc(sizeof(SRP_VBASE)); >@@ -468,21 +486,50 @@ int SRP_VBASE_init(SRP_VBASE *vb, char *verifier_file) > > } > >-SRP_user_pwd *SRP_VBASE_get_by_user(SRP_VBASE *vb, char *username) >+static SRP_user_pwd *find_user(SRP_VBASE *vb, char *username) > { > int i; > SRP_user_pwd *user; >- unsigned char digv[SHA_DIGEST_LENGTH]; >- unsigned char digs[SHA_DIGEST_LENGTH]; >- EVP_MD_CTX ctxt; > > if (vb == NULL) > return NULL; >+ > for (i = 0; i < sk_SRP_user_pwd_num(vb->users_pwd); i++) { > user = sk_SRP_user_pwd_value(vb->users_pwd, i); > if (strcmp(user->id, username) == 0) > return user; > } >+ >+ return NULL; >+} >+ >+/* >+ * This method ignores the configured seed and fails for an unknown user. >+ * Ownership of the returned pointer is not released to the caller. >+ * In other words, caller must not free the result. >+ */ >+SRP_user_pwd *SRP_VBASE_get_by_user(SRP_VBASE *vb, char *username) >+{ >+ return find_user(vb, username); >+} >+ >+/* >+ * Ownership of the returned pointer is released to the caller. >+ * In other words, caller must free the result once done. >+ */ >+SRP_user_pwd *SRP_VBASE_get1_by_user(SRP_VBASE *vb, char *username) >+{ >+ SRP_user_pwd *user; >+ unsigned char digv[SHA_DIGEST_LENGTH]; >+ unsigned char digs[SHA_DIGEST_LENGTH]; >+ EVP_MD_CTX ctxt; >+ >+ if (vb == NULL) >+ return NULL; >+ >+ if ((user = find_user(vb, username)) != NULL) >+ return srp_user_pwd_dup(user); >+ > if ((vb->seed_key == NULL) || > (vb->default_g == NULL) || (vb->default_N == NULL)) > return NULL; >diff --git a/util/libeay.num b/util/libeay.num >index b594caf..a83c3be 100755 >--- a/util/libeay.num >+++ b/util/libeay.num >@@ -1807,6 +1807,8 @@ ASN1_UTCTIME_get 2350 NOEXIST::FUNCTION: > X509_REQ_digest 2362 EXIST::FUNCTION:EVP > X509_CRL_digest 2391 EXIST::FUNCTION:EVP > ASN1_STRING_clear_free 2392 EXIST::FUNCTION: >+SRP_VBASE_get1_by_user 2393 EXIST::FUNCTION:SRP >+SRP_user_pwd_free 2394 EXIST::FUNCTION:SRP > d2i_ASN1_SET_OF_PKCS7 2397 NOEXIST::FUNCTION: > X509_ALGOR_cmp 2398 EXIST::FUNCTION: > EVP_CIPHER_CTX_set_key_length 2399 EXIST::FUNCTION:
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
Actions:
View
|
Diff
Attachments on
bug 40189
:
7509
| 7510 |
7511
|
7512
|
7513