[openssl-commits] [openssl] OpenSSL_1_0_2-stable update
Emilia Kasper
emilia at openssl.org
Thu Feb 25 14:48:57 UTC 2016
The branch OpenSSL_1_0_2-stable has been updated
via 259b664f950c2ba66fbf4b0fe5281327904ead21 (commit)
from 64333004a41a9f4aa587b8e5401420fb70d00687 (commit)
- Log -----------------------------------------------------------------
commit 259b664f950c2ba66fbf4b0fe5281327904ead21
Author: Emilia Kasper <emilia at 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 at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
CHANGES | 19 ++++++++++++++++++
apps/s_server.c | 49 +++++++++++++++++++++++++++-----------------
crypto/srp/srp.h | 10 +++++++++
crypto/srp/srp_vfy.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++-----
util/libeay.num | 2 ++
5 files changed, 114 insertions(+), 23 deletions(-)
diff --git a/CHANGES b/CHANGES
index 8039184..26a0291 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,25 @@
Changes between 1.0.2f and 1.0.2g [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]
+
*) Change the req app to generate a 2048-bit RSA/DSA key by default,
if no keysize is specified with default_bits. This fixes an
omission in an earlier change that changed all RSA/DSA key generation
diff --git a/apps/s_server.c b/apps/s_server.c
index 65cbaaf..09c755b 100644
--- a/apps/s_server.c
+++ b/apps/s_server.c
@@ -429,6 +429,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);
@@ -437,21 +439,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
@@ -2452,9 +2458,10 @@ static int sv_body(char *hostname, int s, int stype, 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);
@@ -2508,9 +2515,10 @@ static int sv_body(char *hostname, int s, int stype, 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);
@@ -2605,9 +2613,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);
@@ -2849,9 +2858,10 @@ static int www_body(char *hostname, int s, int stype, 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);
@@ -2891,9 +2901,10 @@ static int www_body(char *hostname, int s, int stype, 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);
@@ -3236,9 +3247,10 @@ static int rev_body(char *hostname, int s, int stype, 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 accept\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);
@@ -3264,9 +3276,10 @@ static int rev_body(char *hostname, int s, int stype, 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 7f7487d..e5b3c6e 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:
More information about the openssl-commits
mailing list