Sebastian Andrzej Siewior wrote... > BN_bin2bn() and EVP_MD_CTX_new() which were introduced as part of this > patch may return NULL. Not a single instance in the patch checks the > return value. This is just sloppy.
As I'd still like to fix validns for Debian: Mind I ask you for review
of the updated patch below? Are there more flaky things in the openssl
adjustment patch beside those you've mentioned?
As this is a NMU, I'd like to keep the change small, hence I blatantly
ignored "The whole locking callbacks and memory allocation is not
required for 1.1" remark since, as far as I understand it, this does
no harm. Marking pthreads_thread_id and pthreads_locking_callback as
non-static is required to avoid a compiler error.
Changes:
* Adjust whitespace to upstream form
* Catch failure from library calls in three places
By the way, #897882 will see a nicer solution as well.
Regards,
Christoph
Subject: Build against openssl 1.1.
Author: Chris West <solo-debianb...@goeswhere.com>
Bug: https://github.com/tobez/validns/pull/64
Bug-Debian: https://bugs.debian.org/859784
Last-Update: 2018-11-03
--- a/dnskey.c
+++ b/dnskey.c
@@ -154,6 +154,7 @@
unsigned int e_bytes;
unsigned char *pk;
int l;
+ BIGNUM *n, *e;
rsa = RSA_new();
if (!rsa)
@@ -174,11 +175,14 @@
if (l < e_bytes) /* public key is too short */
goto done;
- rsa->e = BN_bin2bn(pk, e_bytes, NULL);
+ e = BN_bin2bn(pk, e_bytes, NULL);
+ if (e == NULL) goto done;
pk += e_bytes;
l -= e_bytes;
- rsa->n = BN_bin2bn(pk, l, NULL);
+ n = BN_bin2bn(pk, l, NULL);
+ if (n == NULL) goto done;
+ RSA_set0_key(rsa, n, e, NULL);
pkey = EVP_PKEY_new();
if (!pkey)
--- a/nsec3checks.c
+++ b/nsec3checks.c
@@ -28,7 +28,7 @@
static struct binary_data name2hash(char *name, struct rr *param)
{
struct rr_nsec3param *p = (struct rr_nsec3param *)param;
- EVP_MD_CTX ctx;
+ EVP_MD_CTX *ctx;
unsigned char md0[EVP_MAX_MD_SIZE];
unsigned char md1[EVP_MAX_MD_SIZE];
unsigned char *md[2];
@@ -45,22 +45,24 @@
/* XXX Maybe use Init_ex and Final_ex for speed? */
- EVP_MD_CTX_init(&ctx);
- if (EVP_DigestInit(&ctx, EVP_sha1()) != 1)
+ ctx = EVP_MD_CTX_new();
+ if (ctx == NULL) return r;
+ if (EVP_DigestInit(ctx, EVP_sha1()) != 1)
return r;
- digest_size = EVP_MD_CTX_size(&ctx);
- EVP_DigestUpdate(&ctx, wire_name.data, wire_name.length);
- EVP_DigestUpdate(&ctx, p->salt.data, p->salt.length);
- EVP_DigestFinal(&ctx, md[mdi], NULL);
+ digest_size = EVP_MD_CTX_size(ctx);
+ EVP_DigestUpdate(ctx, wire_name.data, wire_name.length);
+ EVP_DigestUpdate(ctx, p->salt.data, p->salt.length);
+ EVP_DigestFinal(ctx, md[mdi], NULL);
for (i = 0; i < p->iterations; i++) {
- if (EVP_DigestInit(&ctx, EVP_sha1()) != 1)
+ if (EVP_DigestInit(ctx, EVP_sha1()) != 1)
return r;
- EVP_DigestUpdate(&ctx, md[mdi], digest_size);
+ EVP_DigestUpdate(ctx, md[mdi], digest_size);
mdi = (mdi + 1) % 2;
- EVP_DigestUpdate(&ctx, p->salt.data, p->salt.length);
- EVP_DigestFinal(&ctx, md[mdi], NULL);
+ EVP_DigestUpdate(ctx, p->salt.data, p->salt.length);
+ EVP_DigestFinal(ctx, md[mdi], NULL);
}
+ EVP_MD_CTX_free(ctx);
r.length = digest_size;
r.data = getmem(digest_size);
--- a/rrsig.c
+++ b/rrsig.c
@@ -26,7 +26,7 @@
struct verification_data
{
struct verification_data *next;
- EVP_MD_CTX ctx;
+ EVP_MD_CTX *ctx;
struct rr_dnskey *key;
struct rr_rrsig *rr;
int ok;
@@ -180,7 +180,7 @@
if (d) {
int r;
d->next = NULL;
- r = EVP_VerifyFinal(&d->ctx, (unsigned char
*)d->rr->signature.data, d->rr->signature.length, d->key->pkey);
+ r = EVP_VerifyFinal(d->ctx, (unsigned char
*)d->rr->signature.data, d->rr->signature.length, d->key->pkey);
if (r == 1) {
d->ok = 1;
} else {
@@ -232,7 +232,7 @@
} else {
int r;
G.stats.signatures_verified++;
- r = EVP_VerifyFinal(&d->ctx, (unsigned char
*)d->rr->signature.data, d->rr->signature.length, d->key->pkey);
+ r = EVP_VerifyFinal(d->ctx, (unsigned char
*)d->rr->signature.data, d->rr->signature.length, d->key->pkey);
if (r == 1) {
d->ok = 1;
} else {
@@ -250,21 +250,21 @@
struct rr *signed_rr;
int i;
- EVP_MD_CTX_init(&d->ctx);
+ d->ctx = EVP_MD_CTX_new();
switch (d->rr->algorithm) {
case ALG_DSA:
case ALG_RSASHA1:
case ALG_DSA_NSEC3_SHA1:
case ALG_RSASHA1_NSEC3_SHA1:
- if (EVP_VerifyInit(&d->ctx, EVP_sha1()) != 1)
+ if (EVP_VerifyInit(d->ctx, EVP_sha1()) != 1)
return 0;
break;
case ALG_RSASHA256:
- if (EVP_VerifyInit(&d->ctx, EVP_sha256()) != 1)
+ if (EVP_VerifyInit(d->ctx, EVP_sha256()) != 1)
return 0;
break;
case ALG_RSASHA512:
- if (EVP_VerifyInit(&d->ctx, EVP_sha512()) != 1)
+ if (EVP_VerifyInit(d->ctx, EVP_sha512()) != 1)
return 0;
break;
default:
@@ -274,7 +274,7 @@
chunk = rrsig_wirerdata_ex(&d->rr->rr, 0);
if (chunk.length < 0)
return 0;
- EVP_VerifyUpdate(&d->ctx, chunk.data, chunk.length);
+ EVP_VerifyUpdate(d->ctx, chunk.data, chunk.length);
set = getmem_temp(sizeof(*set) * signed_set->count);
@@ -294,12 +294,12 @@
chunk = name2wire_name(signed_set->named_rr->name);
if (chunk.length < 0)
return 0;
- EVP_VerifyUpdate(&d->ctx, chunk.data, chunk.length);
- b2 = htons(set[i].rr->rdtype); EVP_VerifyUpdate(&d->ctx,
&b2, 2);
- b2 = htons(1); /* class IN */ EVP_VerifyUpdate(&d->ctx, &b2,
2);
- b4 = htonl(set[i].rr->ttl); EVP_VerifyUpdate(&d->ctx,
&b4, 4);
- b2 = htons(set[i].wired.length); EVP_VerifyUpdate(&d->ctx, &b2,
2);
- EVP_VerifyUpdate(&d->ctx, set[i].wired.data,
set[i].wired.length);
+ EVP_VerifyUpdate(d->ctx, chunk.data, chunk.length);
+ b2 = htons(set[i].rr->rdtype); EVP_VerifyUpdate(d->ctx, &b2,
2);
+ b2 = htons(1); /* class IN */ EVP_VerifyUpdate(d->ctx, &b2,
2);
+ b4 = htonl(set[i].rr->ttl); EVP_VerifyUpdate(d->ctx, &b4,
4);
+ b2 = htons(set[i].wired.length); EVP_VerifyUpdate(d->ctx, &b2,
2);
+ EVP_VerifyUpdate(d->ctx, set[i].wired.data,
set[i].wired.length);
}
schedule_verification(d);
@@ -374,7 +374,7 @@
static pthread_mutex_t *lock_cs;
static long *lock_count;
-static unsigned long pthreads_thread_id(void)
+unsigned long pthreads_thread_id(void)
{
unsigned long ret;
@@ -382,7 +382,7 @@
return(ret);
}
-static void pthreads_locking_callback(int mode, int type, char *file, int line)
+void pthreads_locking_callback(int mode, int type, char *file, int line)
{
if (mode & CRYPTO_LOCK) {
pthread_mutex_lock(&(lock_cs[type]));
@@ -446,6 +446,7 @@
if (k->to_verify[i].openssl_error != 0)
e = k->to_verify[i].openssl_error;
}
+ EVP_MD_CTX_free(k->to_verify[i].ctx);
}
if (!ok) {
struct named_rr *named_rr;
signature.asc
Description: PGP signature
