Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x509_vfy.c: Fix a regression in find_issuer(); extend and re-organize some tests #13762

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions apps/crl.c
Original file line number Diff line number Diff line change
Expand Up @@ -287,22 +287,33 @@ int crl_main(int argc, char **argv)
}
if (crlnumber == i) {
ASN1_INTEGER *crlnum;

crlnum = X509_CRL_get_ext_d2i(x, NID_crl_number, NULL, NULL);
BIO_printf(bio_out, "crlNumber=");
if (crlnum) {
BIO_puts(bio_out, "0x");
i2a_ASN1_INTEGER(bio_out, crlnum);
ASN1_INTEGER_free(crlnum);
} else
} else {
BIO_puts(bio_out, "<NONE>");
}
BIO_printf(bio_out, "\n");
}
if (hash == i) {
BIO_printf(bio_out, "%08lx\n",
X509_NAME_hash(X509_CRL_get_issuer(x)));
int ok;
unsigned long hash_value =
X509_NAME_hash_ex(X509_CRL_get_issuer(x), app_get0_libctx(),
app_get0_propq(), &ok);

BIO_printf(bio_out, "issuer name hash=");
if (ok)
BIO_printf(bio_out, "%08lx\n", hash_value);
else
BIO_puts(bio_out, "<ERROR>");
}
#ifndef OPENSSL_NO_MD5
if (hash_old == i) {
BIO_printf(bio_out, "issuer name old hash=");
BIO_printf(bio_out, "%08lx\n",
X509_NAME_hash_old(X509_CRL_get_issuer(x)));
}
Expand Down
19 changes: 16 additions & 3 deletions apps/rehash.c
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,23 @@ static int do_file(const char *filename, const char *fullpath, enum Hash h)
goto end;
}
if (name != NULL) {
if ((h == HASH_NEW) || (h == HASH_BOTH))
errs += add_entry(type, X509_NAME_hash(name), filename, digest, 1, ~0);
if (h == HASH_NEW || h == HASH_BOTH) {
int ok;
unsigned long hash_value =
X509_NAME_hash_ex(name,
app_get0_libctx(), app_get0_propq(), &ok);

if (ok) {
errs += add_entry(type, hash_value, filename, digest, 1, ~0);
} else {
BIO_printf(bio_err, "%s: error calculating SHA1 hash value\n",
opt_getprog());
errs++;
}
}
if ((h == HASH_OLD) || (h == HASH_BOTH))
errs += add_entry(type, X509_NAME_hash_old(name), filename, digest, 1, ~0);
errs += add_entry(type, X509_NAME_hash_old(name),
filename, digest, 1, ~0);
}

end:
Expand Down
13 changes: 8 additions & 5 deletions crypto/pem/pem_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ STACK_OF(X509_INFO) *PEM_X509_INFO_read(FILE *fp, STACK_OF(X509_INFO) *sk,
}
#endif

STACK_OF(X509_INFO)
*PEM_X509_INFO_read_bio_ex(BIO *bp, STACK_OF(X509_INFO) *sk,
pem_password_cb *cb, void *u, OSSL_LIB_CTX *libctx,
const char *propq)
STACK_OF(X509_INFO) *PEM_X509_INFO_read_bio_ex(BIO *bp, STACK_OF(X509_INFO) *sk,
pem_password_cb *cb, void *u,
OSSL_LIB_CTX *libctx,
const char *propq)
{
X509_INFO *xi = NULL;
char *name = NULL, *header = NULL;
Expand All @@ -77,15 +77,18 @@ STACK_OF(X509_INFO)
for (;;) {
raw = 0;
ptype = 0;
ERR_set_mark();
i = PEM_read_bio(bp, &name, &header, &data, &len);
if (i == 0) {
error = ERR_GET_REASON(ERR_peek_last_error());
if (error == PEM_R_NO_START_LINE) {
ERR_clear_error();
ERR_pop_to_mark();
break;
}
ERR_clear_last_mark();
goto err;
}
ERR_clear_last_mark();
start:
if ((strcmp(name, PEM_STRING_X509) == 0) ||
(strcmp(name, PEM_STRING_X509_OLD) == 0)) {
Expand Down
5 changes: 3 additions & 2 deletions crypto/x509/by_dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,9 @@ static int get_cert_by_subject_ex(X509_LOOKUP *xl, X509_LOOKUP_TYPE type,
}

ctx = (BY_DIR *)xl->method_data;

h = X509_NAME_hash(name);
h = X509_NAME_hash_ex(name, libctx, propq, &i);
if (i == 0)
goto finish;
for (i = 0; i < sk_BY_DIR_ENTRY_num(ctx->dirs); i++) {
BY_DIR_ENTRY *ent;
int idx;
Expand Down
27 changes: 16 additions & 11 deletions crypto/x509/x509_cmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ X509_NAME *X509_get_issuer_name(const X509 *a)

unsigned long X509_issuer_name_hash(X509 *x)
{
return X509_NAME_hash(x->cert_info.issuer);
return X509_NAME_hash_ex(x->cert_info.issuer, NULL, NULL, NULL);
}

#ifndef OPENSSL_NO_MD5
Expand All @@ -120,7 +120,7 @@ const ASN1_INTEGER *X509_get0_serialNumber(const X509 *a)

unsigned long X509_subject_name_hash(X509 *x)
{
return X509_NAME_hash(x->cert_info.subject);
return X509_NAME_hash_ex(x->cert_info.subject, NULL, NULL, NULL);
}

#ifndef OPENSSL_NO_MD5
Expand Down Expand Up @@ -250,20 +250,26 @@ int X509_NAME_cmp(const X509_NAME *a, const X509_NAME *b)
return ret < 0 ? -1 : ret > 0;
}

unsigned long X509_NAME_hash(const X509_NAME *x)
unsigned long X509_NAME_hash_ex(const X509_NAME *x, OSSL_LIB_CTX *libctx,
const char *propq, int *ok)
{
unsigned long ret = 0;
unsigned char md[SHA_DIGEST_LENGTH];
EVP_MD *sha1 = EVP_MD_fetch(libctx, "SHA1", propq);

/* Make sure X509_NAME structure contains valid cached encoding */
i2d_X509_NAME(x, NULL);
if (!EVP_Digest(x->canon_enc, x->canon_enclen, md, NULL, EVP_sha1(),
NULL))
return 0;

ret = (((unsigned long)md[0]) | ((unsigned long)md[1] << 8L) |
((unsigned long)md[2] << 16L) | ((unsigned long)md[3] << 24L)
) & 0xffffffffL;
if (ok != NULL)
*ok = 0;
if (sha1 != NULL
&& EVP_Digest(x->canon_enc, x->canon_enclen, md, NULL, sha1, NULL)) {
ret = (((unsigned long)md[0]) | ((unsigned long)md[1] << 8L) |
((unsigned long)md[2] << 16L) | ((unsigned long)md[3] << 24L)
) & 0xffffffffL;
if (ok != NULL)
*ok = 1;
}
EVP_MD_free(sha1);
return ret;
}

Expand All @@ -272,7 +278,6 @@ unsigned long X509_NAME_hash(const X509_NAME *x)
* I now DER encode the name and hash it. Since I cache the DER encoding,
* this is reasonably efficient.
*/

unsigned long X509_NAME_hash_old(const X509_NAME *x)
{
EVP_MD *md5 = EVP_MD_fetch(NULL, OSSL_DIGEST_NAME_MD5, "-fips");
Expand Down
19 changes: 9 additions & 10 deletions crypto/x509/x509_vfy.c
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,9 @@ static X509 *lookup_cert_match(X509_STORE_CTX *ctx, X509 *x)
X509 *xtmp = NULL;
int i;
/* Lookup all certs with matching subject name */
ERR_set_mark();
certs = ctx->lookup_certs(ctx, X509_get_subject_name(x));
ERR_pop_to_mark();
if (certs == NULL)
return NULL;
/* Look for exact match */
Expand Down Expand Up @@ -314,9 +316,10 @@ static int sk_X509_contains(STACK_OF(X509) *sk, X509 *cert)
}

/*
* Find in given STACK_OF(X509) sk a non-expired issuer cert (if any) of given cert x.
* The issuer must not be the same as x and must not yet be in ctx->chain, where the
* exceptional case x is self-issued and ctx->chain has just one element is allowed.
* Find in given STACK_OF(X509) sk an issuer cert of given cert x.
* The issuer must not yet be in ctx->chain, where the exceptional case
* that x is self-issued and ctx->chain has just one element is allowed.
* Prefer the first one that is not expired, else take the last expired one.
*/
static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x)
{
Expand All @@ -325,16 +328,12 @@ static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x)

for (i = 0; i < sk_X509_num(sk); i++) {
issuer = sk_X509_value(sk, i);
/*
* Below check 'issuer != x' is an optimization and safety precaution:
* Candidate issuer cert cannot be the same as the subject cert 'x'.
*/
if (issuer != x && ctx->check_issued(ctx, x, issuer)
if (ctx->check_issued(ctx, x, issuer)
&& (((x->ex_flags & EXFLAG_SI) != 0 && sk_X509_num(ctx->chain) == 1)
|| !sk_X509_contains(ctx->chain, issuer))) {
if (x509_check_cert_time(ctx, issuer, -1))
DDvO marked this conversation as resolved.
Show resolved Hide resolved
return issuer;
rv = issuer;
if (x509_check_cert_time(ctx, rv, -1))
break;
}
}
return rv;
Expand Down
4 changes: 2 additions & 2 deletions doc/man3/X509_LOOKUP_hash_dir.pod
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ the directory.
The directory should contain one certificate or CRL per file in PEM format,
with a filename of the form I<hash>.I<N> for a certificate, or
I<hash>.B<r>I<N> for a CRL.
The I<hash> is the value returned by the L<X509_NAME_hash(3)> function applied
to the subject name for certificates or issuer name for CRLs.
The I<hash> is the value returned by the L<X509_NAME_hash_ex(3)> function
applied to the subject name for certificates or issuer name for CRLs.
The hash can also be obtained via the B<-hash> option of the
L<openssl-x509(1)> or L<openssl-crl(1)> commands.

Expand Down
58 changes: 47 additions & 11 deletions doc/man3/X509_get_subject_name.pod
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,29 @@

=head1 NAME

X509_get_subject_name, X509_set_subject_name, X509_get_issuer_name,
X509_set_issuer_name, X509_REQ_get_subject_name, X509_REQ_set_subject_name,
X509_CRL_get_issuer, X509_CRL_set_issuer_name - get and set issuer or
subject names
X509_NAME_hash_ex, X509_NAME_hash,
X509_get_subject_name, X509_set_subject_name, X509_subject_name_hash,
X509_get_issuer_name, X509_set_issuer_name, X509_issuer_name_hash,
X509_REQ_get_subject_name, X509_REQ_set_subject_name,
X509_CRL_get_issuer, X509_CRL_set_issuer_name -
get X509_NAME hashes or get and set issuer or subject names

=head1 SYNOPSIS

#include <openssl/x509.h>

unsigned long X509_NAME_hash_ex(const X509_NAME *x, OSSL_LIB_CTX *libctx,
const char *propq, int *ok);
Deprecated since OpenSSL 3.0:
#define X509_NAME_hash(x) X509_NAME_hash_ex(x, NULL, NULL, NULL)

X509_NAME *X509_get_subject_name(const X509 *x);
int X509_set_subject_name(X509 *x, const X509_NAME *name);
unsigned long X509_subject_name_hash(X509 *x);

X509_NAME *X509_get_issuer_name(const X509 *x);
int X509_set_issuer_name(X509 *x, const X509_NAME *name);
unsigned long X509_issuer_name_hash(X509 *x);

X509_NAME *X509_REQ_get_subject_name(const X509_REQ *req);
int X509_REQ_set_subject_name(X509_REQ *req, const X509_NAME *name);
Expand All @@ -25,16 +34,29 @@ subject names

=head1 DESCRIPTION

X509_get_subject_name() returns the subject name of certificate B<x>. The
X509_NAME_hash_ex() returns a hash value of name I<x> or 0 on failure,
using any given library context I<libctx> and property query I<propq>.
The I<ok> result argument may be NULL
or else is used to return 1 for success and 0 for failure.
Failure may happen on malloc error or if no SHA1 implementation is available.

X509_NAME_hash() returns a hash value of name I<x> or 0 on failure,
using the default library context and default property query.

X509_get_subject_name() returns the subject name of certificate I<x>. The
returned value is an internal pointer which B<MUST NOT> be freed.

X509_set_subject_name() sets the issuer name of certificate B<x> to
B<name>. The B<name> parameter is copied internally and should be freed
X509_set_subject_name() sets the issuer name of certificate I<x> to
I<name>. The I<name> parameter is copied internally and should be freed
up when it is no longer needed.

X509_get_issuer_name() and X509_set_issuer_name() are identical to
X509_get_subject_name() and X509_set_subject_name() except the get and
set the issuer name of B<x>.
X509_subject_name_hash() returns a hash value of the subject name of
certificate I<x>.

X509_get_issuer_name(), X509_set_issuer_name(), and X509_issuer_name_hash()
are identical to
X509_get_subject_name(), X509_set_subject_name(), and X509_subject_name_hash()
except they relate to the issuer name of I<x>.

Similarly X509_REQ_get_subject_name(), X509_REQ_set_subject_name(),
X509_CRL_get_issuer() and X509_CRL_set_issuer_name() get or set the subject
Expand All @@ -45,9 +67,21 @@ or issuer names of certificate requests of CRLs respectively.
X509_get_subject_name(), X509_get_issuer_name(), X509_REQ_get_subject_name()
and X509_CRL_get_issuer() return an B<X509_NAME> pointer.

X509_NAME_hash_ex(), X509_NAME_hash(),
X509_subject_name_hash() and X509_issuer_name_hash()
return the first four bytes of the SHA1 hash value,
DDvO marked this conversation as resolved.
Show resolved Hide resolved
converted to B<unsigned long> in little endian order,
or 0 on failure.

X509_set_subject_name(), X509_set_issuer_name(), X509_REQ_set_subject_name()
and X509_CRL_set_issuer_name() return 1 for success and 0 for failure.

=head1 BUGS

In case X509_NAME_hash(), X509_subject_name_hash(), or X509_issuer_name_hash()
returns 0 it remains unclear if this is the real hash value or due to failure.
Better use X509_NAME_hash_ex() instead.

=head1 SEE ALSO

L<d2i_X509(3)>,
Expand All @@ -74,9 +108,11 @@ earlier versions.
X509_CRL_get_issuer() is a function in OpenSSL 1.1.0. It was previously
added in OpenSSL 1.0.0 as a macro.

X509_NAME_hash() was turned into a macro and deprecated in OpenSSL 3.0.

=head1 COPYRIGHT

Copyright 2015-2020 The OpenSSL Project Authors. All Rights Reserved.
Copyright 2015-2021 The OpenSSL Project Authors. All Rights Reserved.

Licensed under the Apache License 2.0 (the "License"). You may not use
this file except in compliance with the License. You can obtain a copy
Expand Down
3 changes: 2 additions & 1 deletion engines/e_loader_attic.c
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,8 @@ static int file_find(OSSL_STORE_LOADER_CTX *ctx,
return 0;
}

hash = X509_NAME_hash(OSSL_STORE_SEARCH_get0_name(search));
hash = X509_NAME_hash_ex(OSSL_STORE_SEARCH_get0_name(search),
NULL, NULL, NULL);
BIO_snprintf(ctx->_.dir.search_name, sizeof(ctx->_.dir.search_name),
"%08lx", hash);
return 1;
Expand Down
6 changes: 5 additions & 1 deletion include/openssl/x509.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,11 @@ int X509_add_certs(STACK_OF(X509) *sk, STACK_OF(X509) *certs, int flags);

int X509_cmp(const X509 *a, const X509 *b);
int X509_NAME_cmp(const X509_NAME *a, const X509_NAME *b);
unsigned long X509_NAME_hash(const X509_NAME *x);
#ifndef OPENSSL_NO_DEPRECATED_3_0
# define X509_NAME_hash(x) X509_NAME_hash_ex(x, NULL, NULL, NULL)
#endif
unsigned long X509_NAME_hash_ex(const X509_NAME *x, OSSL_LIB_CTX *libctx,
const char *propq, int *ok);
unsigned long X509_NAME_hash_old(const X509_NAME *x);

int X509_CRL_cmp(const X509_CRL *a, const X509_CRL *b);
Expand Down
7 changes: 6 additions & 1 deletion providers/implementations/storemgmt/file_store.c
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@ static int file_set_ctx_params(void *loaderctx, const OSSL_PARAM params[])
size_t der_len = 0;
X509_NAME *x509_name;
unsigned long hash;
int ok;

if (ctx->type != IS_DIR) {
ERR_raise(ERR_LIB_PROV,
Expand All @@ -481,10 +482,14 @@ static int file_set_ctx_params(void *loaderctx, const OSSL_PARAM params[])
if (!OSSL_PARAM_get_octet_string_ptr(p, (const void **)&der, &der_len)
|| (x509_name = d2i_X509_NAME(NULL, &der, der_len)) == NULL)
return 0;
hash = X509_NAME_hash(x509_name);
hash = X509_NAME_hash_ex(x509_name,
ossl_prov_ctx_get0_libctx(ctx->provctx), NULL,
&ok);
BIO_snprintf(ctx->_.dir.search_name, sizeof(ctx->_.dir.search_name),
"%08lx", hash);
X509_NAME_free(x509_name);
if (ok == 0)
return 0;
}
return 1;
}
Expand Down
3 changes: 2 additions & 1 deletion ssl/ssl_cert.c
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,8 @@ static int xname_sk_cmp(const X509_NAME *const *a, const X509_NAME *const *b)

static unsigned long xname_hash(const X509_NAME *a)
{
return X509_NAME_hash((X509_NAME *)a);
/* This returns 0 also if SHA1 is not available */
return X509_NAME_hash_ex((X509_NAME *)a, NULL, NULL, NULL);
}

STACK_OF(X509_NAME) *SSL_load_client_CA_file_ex(const char *file,
Expand Down
2 changes: 1 addition & 1 deletion test/build.info
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ IF[{- !$disabled{tests} -}]
LIBS{noinst,has_main}=libtestutil.a
SOURCE[libtestutil.a]=testutil/basic_output.c testutil/output.c \
testutil/driver.c testutil/tests.c testutil/cb.c testutil/stanza.c \
testutil/format_output.c \
testutil/format_output.c testutil/load.c \
testutil/test_cleanup.c testutil/main.c testutil/testutil_init.c \
testutil/options.c testutil/test_options.c testutil/provider.c \
testutil/apps_mem.c testutil/random.c $LIBAPPSSRC
Expand Down
Loading