Skip to content

Commit

Permalink
Don't use a ssl specific DRBG anymore
Browse files Browse the repository at this point in the history 
Since the public and private DRBG are per thread we don't need one
per ssl object anymore. It could also try to get entropy from a DRBG
that's really from an other thread because the SSL object moved to an
other thread.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from #5547)
  • Loading branch information
@kroeckx @mspncp
kroeckx authored and mspncp committed Mar 19, 2018
1 parent 7caf122 commit 16cfc2c
Show file tree
Hide file tree
Showing 22 changed files with 35 additions and 165 deletions.
22 changes: 5 additions & 17 deletions crypto/evp/e_aes.c
View file
Expand Up @@ -17,7 +17,6 @@
#include "internal/evp_int.h"
#include "modes_lcl.h"
#include <openssl/rand.h>
#include <openssl/rand_drbg.h>
#include "evp_locl.h"

typedef struct {
Expand Down Expand Up @@ -1405,14 +1404,8 @@ static int s390x_aes_gcm_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr)
memcpy(gctx->iv, ptr, arg);

enc = EVP_CIPHER_CTX_encrypting(c);
if (enc) {
if (c->drbg != NULL) {
if (RAND_DRBG_bytes(c->drbg, gctx->iv + arg, gctx->ivlen - arg) == 0)
return 0;
} else if (RAND_bytes(gctx->iv + arg, gctx->ivlen - arg) <= 0) {
return 0;
}
}
if (enc && RAND_bytes(gctx->iv + arg, gctx->ivlen - arg) <= 0)
return 0;

gctx->iv_gen = 1;
return 1;
Expand Down Expand Up @@ -2639,14 +2632,9 @@ static int aes_gcm_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr)
return 0;
if (arg)
memcpy(gctx->iv, ptr, arg);
if (EVP_CIPHER_CTX_encrypting(c)) {
if (c->drbg != NULL) {
if (RAND_DRBG_bytes(c->drbg, gctx->iv + arg, gctx->ivlen - arg) == 0)
return 0;
} else if (RAND_bytes(gctx->iv + arg, gctx->ivlen - arg) <= 0) {
return 0;
}
}
if (EVP_CIPHER_CTX_encrypting(c)
&& RAND_bytes(gctx->iv + arg, gctx->ivlen - arg) <= 0)
return 0;
gctx->iv_gen = 1;
return 1;

Expand Down
15 changes: 3 additions & 12 deletions crypto/evp/e_aes_cbc_hmac_sha1.c
View file
Expand Up @@ -17,11 +17,9 @@
#include <openssl/aes.h>
#include <openssl/sha.h>
#include <openssl/rand.h>
#include <openssl/rand_drbg.h>
#include "modes_lcl.h"
#include "internal/evp_int.h"
#include "internal/constant_time_locl.h"
#include "evp_locl.h"

typedef struct {
AES_KEY ks;
Expand Down Expand Up @@ -156,8 +154,7 @@ void aesni_multi_cbc_encrypt(CIPH_DESC *, void *, int);
static size_t tls1_1_multi_block_encrypt(EVP_AES_HMAC_SHA1 *key,
unsigned char *out,
const unsigned char *inp,
size_t inp_len, int n4x,
RAND_DRBG *drbg)
size_t inp_len, int n4x)
{ /* n4x is 1 or 2 */
HASH_DESC hash_d[8], edges[8];
CIPH_DESC ciph_d[8];
Expand All @@ -177,13 +174,8 @@ static size_t tls1_1_multi_block_encrypt(EVP_AES_HMAC_SHA1 *key,
# endif

/* ask for IVs in bulk */
IVs = blocks[0].c;
if (drbg != NULL) {
if (RAND_DRBG_bytes(drbg, IVs, 16 * x4) == 0)
return 0;
} else if (RAND_bytes(IVs, 16 * x4) <= 0) {
if (RAND_bytes((IVs = blocks[0].c), 16 * x4) <= 0)
return 0;
}

ctx = (SHA1_MB_CTX *) (storage + 32 - ((size_t)storage % 32)); /* align */

Expand Down Expand Up @@ -901,8 +893,7 @@ static int aesni_cbc_hmac_sha1_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,

return (int)tls1_1_multi_block_encrypt(key, param->out,
param->inp, param->len,
param->interleave / 4,
ctx->drbg);
param->interleave / 4);
}
case EVP_CTRL_TLS1_1_MULTIBLOCK_DECRYPT:
# endif
Expand Down
15 changes: 3 additions & 12 deletions crypto/evp/e_aes_cbc_hmac_sha256.c
View file
Expand Up @@ -18,11 +18,9 @@
#include <openssl/aes.h>
#include <openssl/sha.h>
#include <openssl/rand.h>
#include <openssl/rand_drbg.h>
#include "modes_lcl.h"
#include "internal/constant_time_locl.h"
#include "internal/evp_int.h"
#include "evp_locl.h"

typedef struct {
AES_KEY ks;
Expand Down Expand Up @@ -152,8 +150,7 @@ void aesni_multi_cbc_encrypt(CIPH_DESC *, void *, int);
static size_t tls1_1_multi_block_encrypt(EVP_AES_HMAC_SHA256 *key,
unsigned char *out,
const unsigned char *inp,
size_t inp_len, int n4x,
RAND_DRBG *drbg)
size_t inp_len, int n4x)
{ /* n4x is 1 or 2 */
HASH_DESC hash_d[8], edges[8];
CIPH_DESC ciph_d[8];
Expand All @@ -173,13 +170,8 @@ static size_t tls1_1_multi_block_encrypt(EVP_AES_HMAC_SHA256 *key,
# endif

/* ask for IVs in bulk */
IVs = blocks[0].c;
if (drbg != NULL) {
if (RAND_DRBG_bytes(drbg, IVs, 16 * x4) == 0)
return 0;
} else if (RAND_bytes(IVs, 16 * x4) <= 0) {
if (RAND_bytes((IVs = blocks[0].c), 16 * x4) <= 0)
return 0;
}

/* align */
ctx = (SHA256_MB_CTX *) (storage + 32 - ((size_t)storage % 32));
Expand Down Expand Up @@ -885,8 +877,7 @@ static int aesni_cbc_hmac_sha256_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,

return (int)tls1_1_multi_block_encrypt(key, param->out,
param->inp, param->len,
param->interleave / 4,
ctx->drbg);
param->interleave / 4);
}
case EVP_CTRL_TLS1_1_MULTIBLOCK_DECRYPT:
# endif
Expand Down
11 changes: 3 additions & 8 deletions crypto/evp/e_aria.c
View file
Expand Up @@ -302,14 +302,9 @@ static int aria_gcm_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr)
return 0;
if (arg)
memcpy(gctx->iv, ptr, arg);
if (EVP_CIPHER_CTX_encrypting(c)) {
if (c->drbg != NULL) {
if (RAND_DRBG_bytes(c->drbg, gctx->iv + arg, gctx->ivlen - arg) == 0)
return 0;
} else if (RAND_bytes(gctx->iv + arg, gctx->ivlen - arg) <= 0) {
return 0;
}
}
if (EVP_CIPHER_CTX_encrypting(c)
&& RAND_bytes(gctx->iv + arg, gctx->ivlen - arg) <= 0)
return 0;
gctx->iv_gen = 1;
return 1;

Expand Down
8 changes: 1 addition & 7 deletions crypto/evp/e_des.c
View file
Expand Up @@ -15,8 +15,6 @@
# include "internal/evp_int.h"
# include <openssl/des.h>
# include <openssl/rand.h>
# include <openssl/rand_drbg.h>
# include "evp_locl.h"

typedef struct {
union {
Expand Down Expand Up @@ -231,12 +229,8 @@ static int des_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr)

switch (type) {
case EVP_CTRL_RAND_KEY:
if (c->drbg != NULL) {
if (RAND_DRBG_bytes(c->drbg, ptr, 8) == 0)
return 0;
} else if (RAND_bytes(ptr, 8) <= 0) {
if (RAND_bytes(ptr, 8) <= 0)
return 0;
}
DES_set_odd_parity((DES_cblock *)ptr);
return 1;

Expand Down
13 changes: 2 additions & 11 deletions crypto/evp/e_des3.c
View file
Expand Up @@ -15,7 +15,6 @@
# include "internal/evp_int.h"
# include <openssl/des.h>
# include <openssl/rand.h>
# include <openssl/rand_drbg.h>
# include "evp_locl.h"

typedef struct {
Expand Down Expand Up @@ -284,12 +283,8 @@ static int des3_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, void *ptr)

switch (type) {
case EVP_CTRL_RAND_KEY:
if (ctx->drbg != NULL) {
if (RAND_DRBG_bytes(ctx->drbg, ptr, EVP_CIPHER_CTX_key_length(ctx)) == 0)
return 0;
} else if (RAND_bytes(ptr, EVP_CIPHER_CTX_key_length(ctx)) <= 0) {
if (RAND_bytes(ptr, EVP_CIPHER_CTX_key_length(ctx)) <= 0)
return 0;
}
DES_set_odd_parity(deskey);
if (EVP_CIPHER_CTX_key_length(ctx) >= 16)
DES_set_odd_parity(deskey + 1);
Expand Down Expand Up @@ -377,12 +372,8 @@ static int des_ede3_wrap(EVP_CIPHER_CTX *ctx, unsigned char *out,
memcpy(out + inl + 8, sha1tmp, 8);
OPENSSL_cleanse(sha1tmp, SHA_DIGEST_LENGTH);
/* Generate random IV */
if (ctx->drbg != NULL) {
if (RAND_DRBG_bytes(ctx->drbg, EVP_CIPHER_CTX_iv_noconst(ctx), 8) == 0)
return -1;
} else if (RAND_bytes(EVP_CIPHER_CTX_iv_noconst(ctx), 8) <= 0) {
if (RAND_bytes(EVP_CIPHER_CTX_iv_noconst(ctx), 8) <= 0)
return -1;
}
memcpy(out, EVP_CIPHER_CTX_iv_noconst(ctx), 8);
/* Encrypt everything after IV in place */
des_ede_cbc_cipher(ctx, out + 8, out + 8, inl + 8);
Expand Down
14 changes: 1 addition & 13 deletions crypto/evp/evp_enc.c
View file
Expand Up @@ -579,14 +579,6 @@ int EVP_CIPHER_CTX_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, void *ptr)
{
int ret;

if (type == EVP_CTRL_GET_DRBG) {
*(RAND_DRBG **)ptr = ctx->drbg;
return 1;
}
if (type == EVP_CTRL_SET_DRBG) {
ctx->drbg = ptr;
return 1;
}
if (!ctx->cipher) {
EVPerr(EVP_F_EVP_CIPHER_CTX_CTRL, EVP_R_NO_CIPHER_SET);
return 0;
Expand All @@ -610,12 +602,8 @@ int EVP_CIPHER_CTX_rand_key(EVP_CIPHER_CTX *ctx, unsigned char *key)
{
if (ctx->cipher->flags & EVP_CIPH_RAND_KEY)
return EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_RAND_KEY, 0, key);
if (ctx->drbg) {
if (RAND_DRBG_bytes(ctx->drbg, key, ctx->key_len) == 0)
return 0;
} else if (RAND_bytes(key, ctx->key_len) <= 0) {
if (RAND_bytes(key, ctx->key_len) <= 0)
return 0;
}
return 1;
}

Expand Down
1 change: 0 additions & 1 deletion crypto/evp/evp_locl.h
View file
Expand Up @@ -39,7 +39,6 @@ struct evp_cipher_ctx_st {
int final_used;
int block_mask;
unsigned char final[EVP_MAX_BLOCK_LENGTH]; /* possible final block */
RAND_DRBG *drbg;
} /* EVP_CIPHER_CTX */ ;

int PKCS5_v2_PBKDF2_keyivgen(EVP_CIPHER_CTX *ctx, const char *pass,
Expand Down
13 changes: 3 additions & 10 deletions crypto/evp/p_seal.c
View file
Expand Up @@ -14,8 +14,6 @@
#include <openssl/evp.h>
#include <openssl/objects.h>
#include <openssl/x509.h>
#include <openssl/rand_drbg.h>
#include "evp_locl.h"

int EVP_SealInit(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *type,
unsigned char **ek, int *ekl, unsigned char *iv,
Expand All @@ -33,14 +31,9 @@ int EVP_SealInit(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *type,
return 1;
if (EVP_CIPHER_CTX_rand_key(ctx, key) <= 0)
return 0;
if (EVP_CIPHER_CTX_iv_length(ctx)) {
if (ctx->drbg) {
if (RAND_DRBG_bytes(ctx->drbg, iv, EVP_CIPHER_CTX_iv_length(ctx)) == 0)
return 0;
} else if (RAND_bytes(iv, EVP_CIPHER_CTX_iv_length(ctx)) <= 0) {
return 0;
}
}
if (EVP_CIPHER_CTX_iv_length(ctx)
&& RAND_bytes(iv, EVP_CIPHER_CTX_iv_length(ctx)) <= 0)
return 0;

if (!EVP_EncryptInit_ex(ctx, NULL, NULL, key, iv))
return 0;
Expand Down
14 changes: 0 additions & 14 deletions doc/man3/EVP_EncryptInit.pod
View file
Expand Up @@ -457,20 +457,6 @@ This call is only valid when decrypting data.

=back

=head1 Random numbers

The following can be used to select the DRBG that is used to generate the random
numbers:

EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_SET_DRBG, 0, drbg)

The following can be used to get the DRBG:

EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GET_DRBG, 0, &drbg)

By default it's set to NULL which results in RAND_bytes() being used.


=head1 NOTES

Where possible the B<EVP> interface to symmetric ciphers should be used in
Expand Down
2 changes: 0 additions & 2 deletions include/openssl/evp.h
View file
Expand Up @@ -347,8 +347,6 @@ int (*EVP_CIPHER_meth_get_ctrl(const EVP_CIPHER *cipher))(EVP_CIPHER_CTX *,
# define EVP_CTRL_SET_PIPELINE_INPUT_BUFS 0x23
/* Set the input buffer lengths to use for a pipelined operation */
# define EVP_CTRL_SET_PIPELINE_INPUT_LENS 0x24
# define EVP_CTRL_GET_DRBG 0x25
# define EVP_CTRL_SET_DRBG 0x26

/* Padding modes */
#define EVP_PADDING_PKCS7 1
Expand Down
2 changes: 1 addition & 1 deletion ssl/record/ssl3_record.c
View file
Expand Up @@ -972,7 +972,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
ERR_R_INTERNAL_ERROR);
return -1;
} else if (ssl_randbytes(s, recs[ctr].input, ivlen) <= 0) {
} else if (RAND_bytes(recs[ctr].input, ivlen) <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
ERR_R_INTERNAL_ERROR);
return -1;
Expand Down
1 change: 0 additions & 1 deletion ssl/s3_enc.c
View file
Expand Up @@ -168,7 +168,6 @@ int ssl3_change_cipher_state(SSL *s, int which)
*/
EVP_CIPHER_CTX_reset(s->enc_write_ctx);
}
EVP_CIPHER_CTX_ctrl(s->enc_write_ctx, EVP_CTRL_SET_DRBG, 0, s->drbg);
dd = s->enc_write_ctx;
if (ssl_replace_hash(&s->write_hash, m) == NULL) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_CHANGE_CIPHER_STATE,
Expand Down
6 changes: 3 additions & 3 deletions ssl/s3_lib.c
View file
Expand Up @@ -4524,12 +4524,12 @@ int ssl_fill_hello_random(SSL *s, int server, unsigned char *result, size_t len,
unsigned char *p = result;

l2n(Time, p);
ret = ssl_randbytes(s, p, len - 4);
ret = RAND_bytes(p, len - 4);
} else {
ret = ssl_randbytes(s, result, len);
ret = RAND_bytes(result, len);
}
#ifndef OPENSSL_NO_TLS13DOWNGRADE
if (ret) {
if (ret > 0) {
if (!ossl_assert(sizeof(tls11downgrade) < len)
|| !ossl_assert(sizeof(tls12downgrade) < len))
return 0;
Expand Down
37 changes: 0 additions & 37 deletions ssl/ssl_lib.c
View file
Expand Up @@ -690,20 +690,6 @@ SSL *SSL_new(SSL_CTX *ctx)
goto err;
}

/*
* If not using the standard RAND (say for fuzzing), then don't use a
* chained DRBG.
*/
if (RAND_get_rand_method() == RAND_OpenSSL()) {
s->drbg =
RAND_DRBG_new(0, 0, RAND_DRBG_get0_public());
if (s->drbg == NULL
|| RAND_DRBG_instantiate(s->drbg,
(const unsigned char *) SSL_version_str,
sizeof(SSL_version_str) - 1) == 0)
goto err;
}

RECORD_LAYER_init(&s->rlayer, s);

s->options = ctx->options;
Expand Down Expand Up @@ -1220,7 +1206,6 @@ void SSL_free(SSL *s)
sk_SRTP_PROTECTION_PROFILE_free(s->srtp_profiles);
#endif

RAND_DRBG_free(s->drbg);
CRYPTO_THREAD_lock_free(s->lock);

OPENSSL_free(s);
Expand Down Expand Up @@ -5397,28 +5382,6 @@ uint32_t SSL_get_max_early_data(const SSL *s)
return s->max_early_data;
}

int ssl_randbytes(SSL *s, unsigned char *rnd, size_t size)
{
if (s->drbg != NULL) {
/*
* Currently, it's the duty of the caller to serialize the generate
* requests to the DRBG. So formally we have to check whether
* s->drbg->lock != NULL and take the lock if this is the case.
* However, this DRBG is unique to a given SSL object, and we already
* require that SSL objects are only accessed by a single thread at
* a given time. Also, SSL DRBGs have no child DRBG, so there is
* no risk that this DRBG is accessed by a child DRBG in parallel
* for reseeding. As such, we can rely on the application's
* serialization of SSL accesses for the needed concurrency protection
* here.
*/
return RAND_DRBG_bytes(s->drbg, rnd, size);
}
if (size > INT_MAX)
return 0;
return RAND_bytes(rnd, size);
}

__owur unsigned int ssl_get_max_send_fragment(const SSL *ssl)
{
/* Return any active Max Fragment Len extension */
Expand Down

0 comments on commit 16cfc2c

Please sign in to comment.