openssl
/
openssl
mirror of https://github.com/openssl/openssl
1
0
Fork 0
Code Issues Releases Wiki Activity

X509{,_LOOKUP}: Improve distinction between not found and fatal/internal error 

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/14417)
This commit is contained in:
Dr. David von Oheimb 2021-03-03 20:10:34 +01:00 committed by Dr. David von Oheimb
parent 34959f7a22
commit 0ce8271c20
8 changed files with 154 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
crypto/x509/x509_lu.c
View File

@ -305,10 +305,15 @@ X509_OBJECT *X509_STORE_CTX_get_obj_by_subject(X509_STORE_CTX *vs,
return ret;
}
/* Also fill the cache with all matching certificates */
int X509_STORE_CTX_get_by_subject(const X509_STORE_CTX *vs,
X509_LOOKUP_TYPE type,
const X509_NAME *name, X509_OBJECT *ret)
/*
* Returns 1 if successful,
* 0 if not found or X509_LOOKUP_by_subject_ex() returns an error,
* -1 on failure
*/
static int ossl_x509_store_ctx_get_by_subject(const X509_STORE_CTX *vs,
X509_LOOKUP_TYPE type,
const X509_NAME *name,
X509_OBJECT *ret)
{
X509_STORE *store = vs->store;
X509_LOOKUP *lu;
@ -323,16 +328,19 @@ int X509_STORE_CTX_get_by_subject(const X509_STORE_CTX *vs,
if (!X509_STORE_lock(store))
return 0;
tmp = X509_OBJECT_retrieve_by_subject(store->objs, type, name);
X509_STORE_unlock(store);
if (tmp == NULL || type == X509_LU_CRL) {
for (i = 0; i < sk_X509_LOOKUP_num(store->get_cert_methods); i++) {
lu = sk_X509_LOOKUP_value(store->get_cert_methods, i);
j = X509_LOOKUP_by_subject_ex(lu, type, name, &stmp, vs->libctx,
vs->propq);
if (j) {
if (lu->skip)
continue;
if (lu->method == NULL)
return -1;
j = X509_LOOKUP_by_subject_ex(lu, type, name, &stmp,
vs->libctx, vs->propq);
if (j != 0) { /* non-zero value is considered success here */
tmp = &stmp;
break;
}
@ -340,16 +348,22 @@ int X509_STORE_CTX_get_by_subject(const X509_STORE_CTX *vs,
if (tmp == NULL)
return 0;
}
if (!X509_OBJECT_up_ref_count(tmp))
return 0;
return -1;
ret->type = tmp->type;
ret->data.ptr = tmp->data.ptr;
return 1;
}
/* Also fill the cache with all matching certificates */
int X509_STORE_CTX_get_by_subject(const X509_STORE_CTX *vs,
X509_LOOKUP_TYPE type,
const X509_NAME *name, X509_OBJECT *ret)
{
return ossl_x509_store_ctx_get_by_subject(vs, type, name, ret) > 0;
}
static int x509_store_add(X509_STORE *store, void *x, int crl) {
X509_OBJECT *obj;
int ret = 0, added = 0;
@ -499,13 +513,13 @@ void X509_OBJECT_free(X509_OBJECT *a)
OPENSSL_free(a);
}
/* Returns -1 if not found, but also on error */
static int x509_object_idx_cnt(STACK_OF(X509_OBJECT) *h, X509_LOOKUP_TYPE type,
const X509_NAME *name, int *pnmatch)
{
X509_OBJECT stmp;
X509 x509_s;
X509_CRL crl_s;
int idx;
stmp.type = type;
switch (type) {
@ -518,12 +532,12 @@ static int x509_object_idx_cnt(STACK_OF(X509_OBJECT) *h, X509_LOOKUP_TYPE type,
crl_s.crl.issuer = (X509_NAME *)name; /* won't modify it */
break;
case X509_LU_NONE:
default:
/* abort(); */
return -1;
}
idx = sk_X509_OBJECT_find_all(h, &stmp, pnmatch);
return idx;
return sk_X509_OBJECT_find_all(h, &stmp, pnmatch);
}
int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, X509_LOOKUP_TYPE type,
@ -536,8 +550,8 @@ X509_OBJECT *X509_OBJECT_retrieve_by_subject(STACK_OF(X509_OBJECT) *h,
X509_LOOKUP_TYPE type,
const X509_NAME *name)
{
int idx;
idx = X509_OBJECT_idx_by_subject(h, type, name);
int idx = X509_OBJECT_idx_by_subject(h, type, name);
if (idx == -1)
return NULL;
return sk_X509_OBJECT_value(h, idx);
@ -581,6 +595,7 @@ STACK_OF(X509) *X509_STORE_get1_all_certs(X509_STORE *store)
return NULL;
}
/* Returns NULL on internal/fatal error, empty stack if not found */
STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx,
const X509_NAME *nm)
{
@ -591,7 +606,7 @@ STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx,
X509_STORE *store = ctx->store;
if (store == NULL)
return NULL;
return sk_X509_new_null();
if (!X509_STORE_lock(store))
return NULL;
@ -605,24 +620,26 @@ STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx,
X509_OBJECT *xobj = X509_OBJECT_new();
X509_STORE_unlock(store);
if (xobj == NULL)
return NULL;
if (!X509_STORE_CTX_get_by_subject(ctx, X509_LU_X509, nm, xobj)) {
i = ossl_x509_store_ctx_get_by_subject(ctx, X509_LU_X509, nm, xobj);
if (i <= 0) {
X509_OBJECT_free(xobj);
return NULL;
return i < 0 ? NULL : sk_X509_new_null();
}
X509_OBJECT_free(xobj);
if (!X509_STORE_lock(store))
return NULL;
idx = x509_object_idx_cnt(store->objs, X509_LU_X509, nm, &cnt);
if (idx < 0) {
X509_STORE_unlock(store);
return NULL;
sk = sk_X509_new_null();
goto end;
}
}
sk = sk_X509_new_null();
if (sk == NULL)
goto end;
for (i = 0; i < cnt; i++, idx++) {
obj = sk_X509_OBJECT_value(store->objs, idx);
x = obj->data.x509;
@ -632,14 +649,16 @@ STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx,
return NULL;
}
}
end:
X509_STORE_unlock(store);
return sk;
}
/* Returns NULL on internal/fatal error, empty stack if not found */
STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(const X509_STORE_CTX *ctx,
const X509_NAME *nm)
{
int i, idx, cnt;
int i = 1, idx, cnt;
STACK_OF(X509_CRL) *sk = sk_X509_CRL_new_null();
X509_CRL *x;
X509_OBJECT *obj, *xobj = X509_OBJECT_new();
@ -647,14 +666,16 @@ STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(const X509_STORE_CTX *ctx,
/* Always do lookup to possibly add new CRLs to cache */
if (sk == NULL
|| xobj == NULL
|| store == NULL
|| !X509_STORE_CTX_get_by_subject(ctx, X509_LU_CRL, nm, xobj)) {
|| xobj == NULL
|| (i = ossl_x509_store_ctx_get_by_subject(ctx, X509_LU_CRL,
nm, xobj)) < 0) {
X509_OBJECT_free(xobj);
sk_X509_CRL_free(sk);
return NULL;
}
X509_OBJECT_free(xobj);
if (i == 0)
return sk;
if (!X509_STORE_lock(store)) {
sk_X509_CRL_free(sk);
return NULL;
@ -662,8 +683,7 @@ STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(const X509_STORE_CTX *ctx,
idx = x509_object_idx_cnt(store->objs, X509_LU_CRL, nm, &cnt);
if (idx < 0) {
X509_STORE_unlock(store);
sk_X509_CRL_free(sk);
return NULL;
return sk;
}
for (i = 0; i < cnt; i++, idx++) {
@ -733,10 +753,10 @@ int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x)
return -1;
*issuer = NULL;
xn = X509_get_issuer_name(x);
ok = X509_STORE_CTX_get_by_subject(ctx, X509_LU_X509, xn, obj);
ok = ossl_x509_store_ctx_get_by_subject(ctx, X509_LU_X509, xn, obj);
if (ok != 1) {
X509_OBJECT_free(obj);
return 0;
return ok;
}
/* If certificate matches and is currently valid all OK */
if (ctx->check_issued(ctx, x, obj->data.x509)) {

5
crypto/x509/x509_trust.c
View File

@ -62,6 +62,7 @@ int (*X509_TRUST_set_default(int (*trust) (int, X509 *, int))) (int, X509 *,
return oldtrust;
}
/* Returns X509_TRUST_TRUSTED, X509_TRUST_REJECTED, or X509_TRUST_UNTRUSTED */
int X509_check_trust(X509 *x, int id, int flags)
{
X509_TRUST *pt;
@ -253,7 +254,7 @@ static int obj_trust(int id, X509 *x, int flags)
X509_CERT_AUX *ax = x->aux;
int i;
if (ax && ax->reject) {
if (ax != NULL && ax->reject != NULL) {
for (i = 0; i < sk_ASN1_OBJECT_num(ax->reject); i++) {
ASN1_OBJECT *obj = sk_ASN1_OBJECT_value(ax->reject, i);
int nid = OBJ_obj2nid(obj);
@ -264,7 +265,7 @@ static int obj_trust(int id, X509 *x, int flags)
}
}
if (ax && ax->trust) {
if (ax != NULL && ax->trust != NULL) {
for (i = 0; i < sk_ASN1_OBJECT_num(ax->trust); i++) {
ASN1_OBJECT *obj = sk_ASN1_OBJECT_value(ax->trust, i);
int nid = OBJ_obj2nid(obj);

37
crypto/x509/x509_vfy.c
View File

@ -180,6 +180,7 @@ static int verify_cb_crl(X509_STORE_CTX *ctx, int err)
return ctx->verify_cb(0, ctx);
}
/* Sadly, returns 0 also on internal error in ctx->verify_cb(). */
static int check_auth_level(X509_STORE_CTX *ctx)
{
int i;
@ -207,7 +208,10 @@ static int check_auth_level(X509_STORE_CTX *ctx)
return 1;
}
/* Returns -1 on internal error */
/*-
* Returns -1 on internal error.
* Sadly, returns 0 also on internal error in ctx->verify_cb().
*/
static int verify_chain(X509_STORE_CTX *ctx)
{
int err;
@ -258,6 +262,10 @@ int X509_STORE_CTX_verify(X509_STORE_CTX *ctx)
return X509_verify_cert(ctx);
}
/*-
* Returns -1 on internal error.
* Sadly, returns 0 also on internal error in ctx->verify_cb().
*/
int X509_verify_cert(X509_STORE_CTX *ctx)
{
int ret;
@ -370,7 +378,7 @@ static int get_issuer_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x)
/*-
* Alternative lookup method: look from a STACK stored in other_ctx.
* Returns NULL on internal error (such as out of memory).
* Returns NULL on internal/fatal error, empty stack if not found.
*/
static STACK_OF(X509) *lookup_certs_sk(X509_STORE_CTX *ctx,
const X509_NAME *nm)
@ -397,7 +405,7 @@ static STACK_OF(X509) *lookup_certs_sk(X509_STORE_CTX *ctx,
/*
* Check EE or CA certificate purpose. For trusted certificates explicit local
* auxiliary trust can be used to override EKU-restrictions.
* Sadly, returns 0 also on internal error.
* Sadly, returns 0 also on internal error in ctx->verify_cb().
*/
static int check_purpose(X509_STORE_CTX *ctx, X509 *x, int purpose, int depth,
int must_be_ca)
@ -430,7 +438,7 @@ static int check_purpose(X509_STORE_CTX *ctx, X509 *x, int purpose, int depth,
return 1;
case X509_TRUST_REJECTED:
break;
default:
default: /* can only be X509_TRUST_UNTRUSTED */
switch (X509_check_purpose(x, purpose, must_be_ca > 0)) {
case 1:
return 1;
@ -446,9 +454,9 @@ static int check_purpose(X509_STORE_CTX *ctx, X509 *x, int purpose, int depth,
return verify_cb_cert(ctx, x, depth, X509_V_ERR_INVALID_PURPOSE);
}
/*
/*-
* Check extensions of a cert chain for consistency with the supplied purpose.
* Sadly, returns 0 also on internal error.
* Sadly, returns 0 also on internal error in ctx->verify_cb().
*/
static int check_extensions(X509_STORE_CTX *ctx)
{
@ -644,7 +652,10 @@ static int has_san_id(X509 *x, int gtype)
return ret;
}
/* Returns -1 on internal error */
/*-
* Returns -1 on internal error.
* Sadly, returns 0 also on internal error in ctx->verify_cb().
*/
static int check_name_constraints(X509_STORE_CTX *ctx)
{
int i;
@ -917,7 +928,7 @@ static int check_revocation(X509_STORE_CTX *ctx)
last = sk_X509_num(ctx->chain) - 1;
} else {
/* If checking CRL paths this isn't the EE certificate */
if (ctx->parent)
if (ctx->parent != NULL)
return 1;
last = 0;
}
@ -1628,6 +1639,7 @@ static int cert_crl(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x)
return 1;
}
/* Sadly, returns 0 also on internal error in ctx->verify_cb(). */
static int check_policy(X509_STORE_CTX *ctx)
{
int ret;
@ -1703,6 +1715,7 @@ static int check_policy(X509_STORE_CTX *ctx)
* the validation status.
*
* Return 1 on success, 0 otherwise.
* Sadly, returns 0 also on internal error in ctx->verify_cb().
*/
int ossl_x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth)
{
@ -1732,7 +1745,7 @@ int ossl_x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth)
/*
* Verify the issuer signatures and cert times of ctx->chain.
* Sadly, returns 0 also on internal error.
* Sadly, returns 0 also on internal error in ctx->verify_cb().
*/
static int internal_verify(X509_STORE_CTX *ctx)
{
@ -2897,6 +2910,7 @@ static void dane_reset(SSL_DANE *dane)
dane->pdpth = -1;
}
/* Sadly, returns 0 also on internal error in ctx->verify_cb(). */
static int check_leaf_suiteb(X509_STORE_CTX *ctx, X509 *cert)
{
int err = X509_chain_check_suiteb(NULL, cert, NULL, ctx->param->flags);
@ -2984,7 +2998,10 @@ static int get1_trusted_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *cert)
return ok;
}
/* Returns -1 on internal error */
/*-
* Returns -1 on internal error.
* Sadly, returns 0 also on internal error in ctx->verify_cb().
*/
static int build_chain(X509_STORE_CTX *ctx)
{
SSL_DANE *dane = ctx->dane;

6
doc/build.info
View File

@ -2671,6 +2671,10 @@ DEPEND[html/man3/X509_SIG_get0.html]=man3/X509_SIG_get0.pod
GENERATE[html/man3/X509_SIG_get0.html]=man3/X509_SIG_get0.pod
DEPEND[man/man3/X509_SIG_get0.3]=man3/X509_SIG_get0.pod
GENERATE[man/man3/X509_SIG_get0.3]=man3/X509_SIG_get0.pod
DEPEND[html/man3/X509_STORE_CTX_get_by_subject.html]=man3/X509_STORE_CTX_get_by_subject.pod
GENERATE[html/man3/X509_STORE_CTX_get_by_subject.html]=man3/X509_STORE_CTX_get_by_subject.pod
DEPEND[man/man3/X509_STORE_CTX_get_by_subject.3]=man3/X509_STORE_CTX_get_by_subject.pod
GENERATE[man/man3/X509_STORE_CTX_get_by_subject.3]=man3/X509_STORE_CTX_get_by_subject.pod
DEPEND[html/man3/X509_STORE_CTX_get_error.html]=man3/X509_STORE_CTX_get_error.pod
GENERATE[html/man3/X509_STORE_CTX_get_error.html]=man3/X509_STORE_CTX_get_error.pod
DEPEND[man/man3/X509_STORE_CTX_get_error.3]=man3/X509_STORE_CTX_get_error.pod
@ -3399,6 +3403,7 @@ html/man3/X509_NAME_get_index_by_NID.html \
html/man3/X509_NAME_print_ex.html \
html/man3/X509_PUBKEY_new.html \
html/man3/X509_SIG_get0.html \
html/man3/X509_STORE_CTX_get_by_subject.html \
html/man3/X509_STORE_CTX_get_error.html \
html/man3/X509_STORE_CTX_new.html \
html/man3/X509_STORE_CTX_set_verify_cb.html \
@ -3994,6 +3999,7 @@ man/man3/X509_NAME_get_index_by_NID.3 \
man/man3/X509_NAME_print_ex.3 \
man/man3/X509_PUBKEY_new.3 \
man/man3/X509_SIG_get0.3 \
man/man3/X509_STORE_CTX_get_by_subject.3 \
man/man3/X509_STORE_CTX_get_error.3 \
man/man3/X509_STORE_CTX_new.3 \
man/man3/X509_STORE_CTX_set_verify_cb.3 \

21
doc/man3/X509_LOOKUP.pod
View File

@ -91,7 +91,8 @@ associates and retrieves a pointer to application data to and from the
given B<X509_LOOKUP>, respectively.
X509_LOOKUP_ctrl_ex() is used to set or get additional data to or from
a B<X509_LOOKUP> structure or its associated L<X509_LOOKUP_METHOD(3)>.
a B<X509_LOOKUP> structure using any control function in the
associated L<X509_LOOKUP_METHOD(3)>.
The arguments of the control command are passed via I<argc> and I<argl>,
its return value via I<*ret>. The library context I<libctx> and property
query I<propq> are used when fetching algorithms from providers.
@ -195,21 +196,29 @@ or NULL on error.
X509_LOOKUP_init() and X509_LOOKUP_shutdown() return 1 on success, or
0 on error.
X509_LOOKUP_ctrl() returns -1 if the B<X509_LOOKUP> doesn't have an
X509_LOOKUP_ctrl_ex() and X509_LOOKUP_ctrl()
return -1 if the B<X509_LOOKUP> doesn't have an
associated B<X509_LOOKUP_METHOD>, or 1 if the X<509_LOOKUP_METHOD>
doesn't have a control function.
Otherwise, it returns what the control function in the
B<X509_LOOKUP_METHOD> returns, which is usually 1 on success and 0 in
error.
B<X509_LOOKUP_METHOD> returns, which is usually 1 on success and 0 on error
but could also be -1 on failure.
X509_LOOKUP_get_store() returns a B<X509_STORE> pointer if there is
one, otherwise NULL.
X509_LOOKUP_by_subject_ex(), X509_LOOKUP_by_subject(),
X509_LOOKUP_by_subject_ex() returns 0 if there is no B<X509_LOOKUP_METHOD>
that implements any of the get_by_subject_ex() or get_by_subject() functions.
It calls get_by_subject_ex() if present, otherwise get_by_subject(), and returns
the result of the function, which is usually 1 on success and 0 on error.
X509_LOOKUP_by_subject() is similar to X509_LOOKUP_by_subject_ex()
but passes NULL for both the libctx and propq.
X509_LOOKUP_by_issuer_serial(), X509_LOOKUP_by_fingerprint(), and
X509_LOOKUP_by_alias() all return 0 if there is no B<X509_LOOKUP_METHOD> or that
method doesn't implement the corresponding function.
Otherwise, it returns what the corresponding function in the
Otherwise, they return what the corresponding function in the
B<X509_LOOKUP_METHOD> returns, which is usually 1 on success and 0 in
error.

3
doc/man3/X509_LOOKUP_meth_new.pod
View File

@ -149,7 +149,7 @@ object.
Implementations must add objects they find to the B<X509_STORE> object
using X509_STORE_add_cert() or X509_STORE_add_crl(). This increments
its reference count. However, the X509_STORE_CTX_get_by_subject()
its reference count. However, the L<X509_STORE_CTX_get_by_subject(3)>
function also increases the reference count which leads to one too
many references being held. Therefore, applications should
additionally call X509_free() or X509_CRL_free() to decrement the
@ -178,6 +178,7 @@ pointers.
=head1 SEE ALSO
L<X509_STORE_CTX_get_by_subject(3)>,
L<X509_STORE_new(3)>, L<SSL_CTX_set_cert_store(3)>
=head1 HISTORY

51
doc/man3/X509_STORE_CTX_get_by_subject.pod Normal file
View File

@ -0,0 +1,51 @@
=pod
=head1 NAME
X509_STORE_CTX_get_by_subject,
X509_STORE_CTX_get_obj_by_subject
- X509 and X509_CRL lookup functions
=head1 SYNOPSIS
#include <openssl/x509_vfy.h>
int X509_STORE_CTX_get_by_subject(const X509_STORE_CTX *vs,
X509_LOOKUP_TYPE type,
const X509_NAME *name, X509_OBJECT *ret);
X509_OBJECT *X509_STORE_CTX_get_obj_by_subject(X509_STORE_CTX *vs,
X509_LOOKUP_TYPE type,
const X509_NAME *name);
=head1 DESCRIPTION
X509_STORE_CTX_get_by_subject() tries to find an object
of given I<type>, which may be B<X509_LU_X509> or B<X509_LU_CRL>,
and subject I<name> from the store in the provided store context I<vs>.
If found and I<ret> is not NULL, it increments the reference count and
stores the looked up object in I<ret>.
X509_STORE_CTX_get_obj_by_subject() is like X509_STORE_CTX_get_by_subject()
but returns the found object on success, else NULL.
=head1 RETURN VALUES
X509_STORE_CTX_get_by_subject() returns 1 if the lookup was successful, else 0.
X509_STORE_CTX_get_obj_by_subject() returns an object on success, else NULL.
=head1 SEE ALSO
L<X509_LOOKUP_meth_set_get_by_subject(3)>,
L<X509_LOOKUP_by_subject(3)>
=head1 COPYRIGHT
Copyright 2022 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
in the file LICENSE in the source distribution or at
L<https://www.openssl.org/source/license.html>.
=cut

2
util/missingcrypto.txt
View File

@ -1273,9 +1273,7 @@ X509_STORE_CTX_get0_policy_tree(3)
X509_STORE_CTX_get0_store(3)
X509_STORE_CTX_get1_certs(3)
X509_STORE_CTX_get1_crls(3)
X509_STORE_CTX_get_by_subject(3)
X509_STORE_CTX_get_explicit_policy(3)
X509_STORE_CTX_get_obj_by_subject(3)
X509_STORE_CTX_set0_dane(3)
X509_STORE_CTX_set_depth(3)
X509_STORE_CTX_set_flags(3)