From 5d20c4fb3582a0e6cbf8513c94c60e4cd326716d Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Sun, 17 Sep 2006 17:16:28 +0000 Subject: [PATCH] Overhaul of by_dir code to handle dynamic loading of CRLs. --- CHANGES | 11 +++ apps/s_cb.c | 4 +- apps/s_client.c | 3 + apps/s_server.c | 4 +- crypto/stack/safestack.h | 44 +++++++++ crypto/x509/by_dir.c | 198 +++++++++++++++++++++++++++++---------- crypto/x509/x509_lu.c | 37 ++++---- crypto/x509/x509_vfy.c | 4 +- crypto/x509/x509_vfy.h | 2 + 9 files changed, 231 insertions(+), 76 deletions(-) diff --git a/CHANGES b/CHANGES index 506e8fa4aa..8f9c9a1ae3 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,17 @@ Changes between 0.9.8d and 0.9.9 [xx XXX xxxx] + *) Overhaul of by_dir code. Add support for dynamic loading of CRLs so + new CRLs added to a directory can be used. New command line option + -verify_return_error to s_client and s_server. This causes real errors + to be returned by the verify callback instead of carrying on no matter + what. This reflects the way a "real world" verify callback would behave. + [Steve Henson] + + *) GOST engine, supporting several GOST algorithms and public key formats. + Kindly donated by Cryptocom. + [Cryptocom] + *) Partial support for Issuing Distribution Point CRL extension. CRLs partitioned by DP are handled but no indirect CRL or reason partitioning (yet). Complete overhaul of CRL handling: now the most suitable CRL is diff --git a/apps/s_cb.c b/apps/s_cb.c index 573f98cea6..6d322d4f40 100644 --- a/apps/s_cb.c +++ b/apps/s_cb.c @@ -123,6 +123,7 @@ int verify_depth=0; int verify_error=X509_V_OK; +int verify_return_error=0; int MS_CALLBACK verify_callback(int ok, X509_STORE_CTX *ctx) { @@ -142,7 +143,8 @@ int MS_CALLBACK verify_callback(int ok, X509_STORE_CTX *ctx) X509_verify_cert_error_string(err)); if (verify_depth >= depth) { - ok=1; + if (!verify_return_error) + ok=1; verify_error=X509_V_OK; } else diff --git a/apps/s_client.c b/apps/s_client.c index d105a7413e..3515070489 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -187,6 +187,7 @@ typedef unsigned int u_int; extern int verify_depth; extern int verify_error; +extern int verify_return_error; #ifdef FIONBIO static int c_nbio=0; @@ -478,6 +479,8 @@ int MAIN(int argc, char **argv) vflags |= X509_V_FLAG_CRL_CHECK; else if (strcmp(*argv,"-crl_check_all") == 0) vflags |= X509_V_FLAG_CRL_CHECK|X509_V_FLAG_CRL_CHECK_ALL; + else if (strcmp(*argv,"-verify_return_error") == 0) + verify_return_error = 1; else if (strcmp(*argv,"-prexit") == 0) prexit=1; else if (strcmp(*argv,"-crlf") == 0) diff --git a/apps/s_server.c b/apps/s_server.c index a294ed343d..ac43e5aac1 100644 --- a/apps/s_server.c +++ b/apps/s_server.c @@ -258,7 +258,7 @@ static int accept_socket= -1; #undef PROG #define PROG s_server_main -extern int verify_depth; +extern int verify_depth, verify_return_error; static char *cipher=NULL; static int s_server_verify=SSL_VERIFY_NONE; @@ -842,6 +842,8 @@ int MAIN(int argc, char *argv[]) { vflags |= X509_V_FLAG_CRL_CHECK|X509_V_FLAG_CRL_CHECK_ALL; } + else if (strcmp(*argv,"-verify_return_error") == 0) + verify_return_error = 1; else if (strcmp(*argv,"-serverpref") == 0) { off|=SSL_OP_CIPHER_SERVER_PREFERENCE; } else if (strcmp(*argv,"-cipher") == 0) diff --git a/crypto/stack/safestack.h b/crypto/stack/safestack.h index 118f7b100b..5e55d15866 100644 --- a/crypto/stack/safestack.h +++ b/crypto/stack/safestack.h @@ -410,6 +410,50 @@ STACK_OF(type) \ #define sk_BIO_sort(st) SKM_sk_sort(BIO, (st)) #define sk_BIO_is_sorted(st) SKM_sk_is_sorted(BIO, (st)) +#define sk_BY_DIR_ENTRY_new(st) SKM_sk_new(BY_DIR_ENTRY, (st)) +#define sk_BY_DIR_ENTRY_new_null() SKM_sk_new_null(BY_DIR_ENTRY) +#define sk_BY_DIR_ENTRY_free(st) SKM_sk_free(BY_DIR_ENTRY, (st)) +#define sk_BY_DIR_ENTRY_num(st) SKM_sk_num(BY_DIR_ENTRY, (st)) +#define sk_BY_DIR_ENTRY_value(st, i) SKM_sk_value(BY_DIR_ENTRY, (st), (i)) +#define sk_BY_DIR_ENTRY_set(st, i, val) SKM_sk_set(BY_DIR_ENTRY, (st), (i), (val)) +#define sk_BY_DIR_ENTRY_zero(st) SKM_sk_zero(BY_DIR_ENTRY, (st)) +#define sk_BY_DIR_ENTRY_push(st, val) SKM_sk_push(BY_DIR_ENTRY, (st), (val)) +#define sk_BY_DIR_ENTRY_unshift(st, val) SKM_sk_unshift(BY_DIR_ENTRY, (st), (val)) +#define sk_BY_DIR_ENTRY_find(st, val) SKM_sk_find(BY_DIR_ENTRY, (st), (val)) +#define sk_BY_DIR_ENTRY_find_ex(st, val) SKM_sk_find_ex(BY_DIR_ENTRY, (st), (val)) +#define sk_BY_DIR_ENTRY_delete(st, i) SKM_sk_delete(BY_DIR_ENTRY, (st), (i)) +#define sk_BY_DIR_ENTRY_delete_ptr(st, ptr) SKM_sk_delete_ptr(BY_DIR_ENTRY, (st), (ptr)) +#define sk_BY_DIR_ENTRY_insert(st, val, i) SKM_sk_insert(BY_DIR_ENTRY, (st), (val), (i)) +#define sk_BY_DIR_ENTRY_set_cmp_func(st, cmp) SKM_sk_set_cmp_func(BY_DIR_ENTRY, (st), (cmp)) +#define sk_BY_DIR_ENTRY_dup(st) SKM_sk_dup(BY_DIR_ENTRY, st) +#define sk_BY_DIR_ENTRY_pop_free(st, free_func) SKM_sk_pop_free(BY_DIR_ENTRY, (st), (free_func)) +#define sk_BY_DIR_ENTRY_shift(st) SKM_sk_shift(BY_DIR_ENTRY, (st)) +#define sk_BY_DIR_ENTRY_pop(st) SKM_sk_pop(BY_DIR_ENTRY, (st)) +#define sk_BY_DIR_ENTRY_sort(st) SKM_sk_sort(BY_DIR_ENTRY, (st)) +#define sk_BY_DIR_ENTRY_is_sorted(st) SKM_sk_is_sorted(BY_DIR_ENTRY, (st)) + +#define sk_BY_DIR_HASH_new(st) SKM_sk_new(BY_DIR_HASH, (st)) +#define sk_BY_DIR_HASH_new_null() SKM_sk_new_null(BY_DIR_HASH) +#define sk_BY_DIR_HASH_free(st) SKM_sk_free(BY_DIR_HASH, (st)) +#define sk_BY_DIR_HASH_num(st) SKM_sk_num(BY_DIR_HASH, (st)) +#define sk_BY_DIR_HASH_value(st, i) SKM_sk_value(BY_DIR_HASH, (st), (i)) +#define sk_BY_DIR_HASH_set(st, i, val) SKM_sk_set(BY_DIR_HASH, (st), (i), (val)) +#define sk_BY_DIR_HASH_zero(st) SKM_sk_zero(BY_DIR_HASH, (st)) +#define sk_BY_DIR_HASH_push(st, val) SKM_sk_push(BY_DIR_HASH, (st), (val)) +#define sk_BY_DIR_HASH_unshift(st, val) SKM_sk_unshift(BY_DIR_HASH, (st), (val)) +#define sk_BY_DIR_HASH_find(st, val) SKM_sk_find(BY_DIR_HASH, (st), (val)) +#define sk_BY_DIR_HASH_find_ex(st, val) SKM_sk_find_ex(BY_DIR_HASH, (st), (val)) +#define sk_BY_DIR_HASH_delete(st, i) SKM_sk_delete(BY_DIR_HASH, (st), (i)) +#define sk_BY_DIR_HASH_delete_ptr(st, ptr) SKM_sk_delete_ptr(BY_DIR_HASH, (st), (ptr)) +#define sk_BY_DIR_HASH_insert(st, val, i) SKM_sk_insert(BY_DIR_HASH, (st), (val), (i)) +#define sk_BY_DIR_HASH_set_cmp_func(st, cmp) SKM_sk_set_cmp_func(BY_DIR_HASH, (st), (cmp)) +#define sk_BY_DIR_HASH_dup(st) SKM_sk_dup(BY_DIR_HASH, st) +#define sk_BY_DIR_HASH_pop_free(st, free_func) SKM_sk_pop_free(BY_DIR_HASH, (st), (free_func)) +#define sk_BY_DIR_HASH_shift(st) SKM_sk_shift(BY_DIR_HASH, (st)) +#define sk_BY_DIR_HASH_pop(st) SKM_sk_pop(BY_DIR_HASH, (st)) +#define sk_BY_DIR_HASH_sort(st) SKM_sk_sort(BY_DIR_HASH, (st)) +#define sk_BY_DIR_HASH_is_sorted(st) SKM_sk_is_sorted(BY_DIR_HASH, (st)) + #define sk_CONF_IMODULE_new(st) SKM_sk_new(CONF_IMODULE, (st)) #define sk_CONF_IMODULE_new_null() SKM_sk_new_null(CONF_IMODULE) #define sk_CONF_IMODULE_free(st) SKM_sk_free(CONF_IMODULE, (st)) diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c index 6398d35a0a..11e9c1ec0a 100644 --- a/crypto/x509/by_dir.c +++ b/crypto/x509/by_dir.c @@ -72,15 +72,29 @@ #include #include +DECLARE_STACK_OF(BY_DIR_HASH) +DECLARE_STACK_OF(BY_DIR_ENTRY) + +typedef struct lookup_dir_hashes_st + { + unsigned long hash; + int suffix; + } BY_DIR_HASH; + +typedef struct lookup_dir_entry_st + { + char *dir; + int dir_type; + STACK_OF(BY_DIR_HASH) *hashes; + } BY_DIR_ENTRY; + typedef struct lookup_dir_st { BUF_MEM *buffer; - int num_dirs; - char **dirs; - int *dirs_type; - int num_dirs_alloced; + STACK_OF(BY_DIR_ENTRY) *dirs; } BY_DIR; + static int dir_ctrl(X509_LOOKUP *ctx, int cmd, const char *argp, long argl, char **ret); static int new_dir(X509_LOOKUP *lu); @@ -150,34 +164,51 @@ static int new_dir(X509_LOOKUP *lu) OPENSSL_free(a); return(0); } - a->num_dirs=0; a->dirs=NULL; - a->dirs_type=NULL; - a->num_dirs_alloced=0; lu->method_data=(char *)a; return(1); } +static void by_dir_hash_free(BY_DIR_HASH *hash) + { + OPENSSL_free(hash); + } + +static int by_dir_hash_cmp(const BY_DIR_HASH * const *a, + const BY_DIR_HASH * const *b) + { + if ((*a)->hash > (*b)->hash) + return 1; + if ((*a)->hash < (*b)->hash) + return -1; + return 0; + } + +static void by_dir_entry_free(BY_DIR_ENTRY *ent) + { + if (ent->dir) + OPENSSL_free(ent->dir); + if (ent->hashes) + sk_BY_DIR_HASH_pop_free(ent->hashes, by_dir_hash_free); + OPENSSL_free(ent); + } + static void free_dir(X509_LOOKUP *lu) { BY_DIR *a; - int i; a=(BY_DIR *)lu->method_data; - for (i=0; inum_dirs; i++) - if (a->dirs[i] != NULL) OPENSSL_free(a->dirs[i]); - if (a->dirs != NULL) OPENSSL_free(a->dirs); - if (a->dirs_type != NULL) OPENSSL_free(a->dirs_type); - if (a->buffer != NULL) BUF_MEM_free(a->buffer); + if (a->dirs != NULL) + sk_BY_DIR_ENTRY_pop_free(a->dirs, by_dir_entry_free); + if (a->buffer != NULL) + BUF_MEM_free(a->buffer); OPENSSL_free(a); } static int add_cert_dir(BY_DIR *ctx, const char *dir, int type) { int j,len; - int *ip; const char *s,*ss,*p; - char **pp; if (dir == NULL || !*dir) { @@ -191,47 +222,51 @@ static int add_cert_dir(BY_DIR *ctx, const char *dir, int type) { if ((*p == LIST_SEPARATOR_CHAR) || (*p == '\0')) { + BY_DIR_ENTRY *ent; ss=s; s=p+1; len=(int)(p-ss); if (len == 0) continue; - for (j=0; jnum_dirs; j++) - if (strncmp(ctx->dirs[j],ss,(unsigned int)len) == 0) + for (j=0; j < sk_BY_DIR_ENTRY_num(ctx->dirs); j++) + { + ent = sk_BY_DIR_ENTRY_value(ctx->dirs, j); + if (strncmp(ent->dir,ss,(unsigned int)len) == 0) continue; - if (ctx->num_dirs_alloced < (ctx->num_dirs+1)) + } + + if (ctx->dirs == NULL) { - ctx->num_dirs_alloced+=10; - pp=(char **)OPENSSL_malloc(ctx->num_dirs_alloced* - sizeof(char *)); - ip=(int *)OPENSSL_malloc(ctx->num_dirs_alloced* - sizeof(int)); - if ((pp == NULL) || (ip == NULL)) + ctx->dirs = sk_BY_DIR_ENTRY_new_null(); + if (!ctx->dirs) { X509err(X509_F_ADD_CERT_DIR,ERR_R_MALLOC_FAILURE); - return(0); + return 0; } - memcpy(pp,ctx->dirs,(ctx->num_dirs_alloced-10)* - sizeof(char *)); - memcpy(ip,ctx->dirs_type,(ctx->num_dirs_alloced-10)* - sizeof(int)); - if (ctx->dirs != NULL) - OPENSSL_free(ctx->dirs); - if (ctx->dirs_type != NULL) - OPENSSL_free(ctx->dirs_type); - ctx->dirs=pp; - ctx->dirs_type=ip; } - ctx->dirs_type[ctx->num_dirs]=type; - ctx->dirs[ctx->num_dirs]=(char *)OPENSSL_malloc((unsigned int)len+1); - if (ctx->dirs[ctx->num_dirs] == NULL) return(0); - strncpy(ctx->dirs[ctx->num_dirs],ss,(unsigned int)len); - ctx->dirs[ctx->num_dirs][len]='\0'; - ctx->num_dirs++; + ent = OPENSSL_malloc(sizeof(BY_DIR_ENTRY)); + if (!ent) + return 0; + ent->dir_type = type; + ent->hashes = sk_BY_DIR_HASH_new(by_dir_hash_cmp); + ent->dir = OPENSSL_malloc((unsigned int)len+1); + if (!ent->dir || !ent->hashes) + { + by_dir_entry_free(ent); + return 0; + } + strncpy(ent->dir,ss,(unsigned int)len); + ent->dir[len] = '\0'; + if (!sk_BY_DIR_ENTRY_push(ctx->dirs, ent)) + { + by_dir_entry_free(ent); + return 0; + } } - if (*p == '\0') break; + if (*p == '\0') + break; p++; } - return(1); + return 1; } static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, @@ -287,20 +322,42 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, ctx=(BY_DIR *)xl->method_data; h=X509_NAME_hash(name); - for (i=0; inum_dirs; i++) + for (i=0; i < sk_BY_DIR_ENTRY_num(ctx->dirs); i++) { - j=strlen(ctx->dirs[i])+1+8+6+1+1; + BY_DIR_ENTRY *ent; + int idx; + BY_DIR_HASH htmp, *hent; + ent = sk_BY_DIR_ENTRY_value(ctx->dirs, i); + j=strlen(ent->dir)+1+8+6+1+1; if (!BUF_MEM_grow(b,j)) { X509err(X509_F_GET_CERT_BY_SUBJECT,ERR_R_MALLOC_FAILURE); goto finish; } - k=0; + if (type == X509_LU_CRL && ent->hashes) + { + htmp.hash = h; + CRYPTO_r_lock(CRYPTO_LOCK_X509_STORE); + idx = sk_BY_DIR_HASH_find(ent->hashes, &htmp); + if (idx >= 0) + { + hent = sk_BY_DIR_HASH_value(ent->hashes, idx); + k = hent->suffix; + } + else + { + hent = NULL; + k=0; + } + CRYPTO_r_unlock(CRYPTO_LOCK_X509_STORE); + } + else + k = 0; for (;;) { char c = '/'; #ifdef OPENSSL_SYS_VMS - c = ctx->dirs[i][strlen(ctx->dirs[i])-1]; + c = ent->dir[strlen(ctx->dirs[i])-1]; if (c != ':' && c != '>' && c != ']') { /* If no separator is present, we assume the @@ -321,16 +378,15 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, /* This is special. When c == '\0', no directory separator should be added. */ BIO_snprintf(b->data,b->max, - "%s%08lx.%s%d",ctx->dirs[i],h, + "%s%08lx.%s%d",ent->dir,h, postfix,k); } else { BIO_snprintf(b->data,b->max, - "%s%c%08lx.%s%d",ctx->dirs[i],c,h, + "%s%c%08lx.%s%d",ent->dir,c,h, postfix,k); } - k++; #ifndef OPENSSL_NO_POSIX_IO { struct stat st; @@ -342,16 +398,17 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, if (type == X509_LU_X509) { if ((X509_load_cert_file(xl,b->data, - ctx->dirs_type[i])) == 0) + ent->dir_type)) == 0) break; } else if (type == X509_LU_CRL) { if ((X509_load_crl_file(xl,b->data, - ctx->dirs_type[i])) == 0) + ent->dir_type)) == 0) break; } /* else case will caught higher up */ + k++; } /* we have added it to the cache so now pull @@ -362,6 +419,43 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, else tmp = NULL; CRYPTO_r_unlock(CRYPTO_LOCK_X509_STORE); + + /* If a CRL, update the last file suffix added for this */ + + if (type == X509_LU_CRL) + { + CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + /* Look for entry again in case another thread added + * an entry first. + */ + if (!hent) + { + htmp.hash = h; + idx = sk_BY_DIR_HASH_find(ent->hashes, &htmp); + if (idx >= 0) + hent = + sk_BY_DIR_HASH_value(ent->hashes, idx); + } + if (!hent) + { + hent = OPENSSL_malloc(sizeof(BY_DIR_HASH)); + hent->hash = h; + hent->suffix = k; + if (!sk_BY_DIR_HASH_push(ent->hashes, hent)) + { + CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + OPENSSL_free(hent); + ok = 0; + goto finish; + } + } + else if (hent->suffix < k) + hent->suffix = k; + + CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + + } + if (tmp != NULL) { ok=1; diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index fad7ea0689..ead1e0b811 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -288,7 +288,7 @@ int X509_STORE_get_by_subject(X509_STORE_CTX *vs, int type, X509_NAME *name, tmp=X509_OBJECT_retrieve_by_subject(ctx->objs,type,name); - if (tmp == NULL) + if (tmp == NULL || type == X509_LU_CRL) { for (i=vs->current_method; iget_cert_methods); i++) { @@ -530,33 +530,30 @@ STACK_OF(X509_CRL)* X509_STORE_get1_crls(X509_STORE_CTX *ctx, X509_NAME *nm) int i, idx, cnt; STACK_OF(X509_CRL) *sk; X509_CRL *x; - X509_OBJECT *obj; + X509_OBJECT *obj, xobj; sk = sk_X509_CRL_new_null(); CRYPTO_r_lock(CRYPTO_LOCK_X509_STORE); /* Check cache first */ idx = x509_object_idx_cnt(ctx->ctx->objs, X509_LU_CRL, nm, &cnt); + + /* Always do lookup to possibly add new CRLs to cache + */ + CRYPTO_r_unlock(CRYPTO_LOCK_X509_STORE); + if (!X509_STORE_get_by_subject(ctx, X509_LU_CRL, nm, &xobj)) + { + sk_X509_CRL_free(sk); + return NULL; + } + X509_OBJECT_free_contents(&xobj); + CRYPTO_r_lock(CRYPTO_LOCK_X509_STORE); + idx = x509_object_idx_cnt(ctx->ctx->objs,X509_LU_CRL, nm, &cnt); if (idx < 0) { - /* Nothing found in cache: do lookup to possibly add new - * objects to cache - */ - X509_OBJECT xobj; CRYPTO_r_unlock(CRYPTO_LOCK_X509_STORE); - if (!X509_STORE_get_by_subject(ctx, X509_LU_CRL, nm, &xobj)) - { - sk_X509_CRL_free(sk); - return NULL; - } - X509_OBJECT_free_contents(&xobj); - CRYPTO_r_lock(CRYPTO_LOCK_X509_STORE); - idx = x509_object_idx_cnt(ctx->ctx->objs,X509_LU_CRL, nm, &cnt); - if (idx < 0) - { - CRYPTO_r_unlock(CRYPTO_LOCK_X509_STORE); - sk_X509_CRL_free(sk); - return NULL; - } + sk_X509_CRL_free(sk); + return NULL; } + for (i = 0; i < cnt; i++, idx++) { obj = sk_X509_OBJECT_value(ctx->ctx->objs, idx); diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 2292b27ff7..1252439f1e 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -967,8 +967,8 @@ static int cert_crl(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x) return 1; /* See if we have any critical CRL extensions: since we - * currently don't handle any CRL extensions the CRL must be - * rejected. + * currently only handle IDP the CRL must be rejected if any others + * are present. * This code accesses the X509_CRL structure directly: applications * shouldn't do this. */ diff --git a/crypto/x509/x509_vfy.h b/crypto/x509/x509_vfy.h index 322637a6e4..bfca7c7453 100644 --- a/crypto/x509/x509_vfy.h +++ b/crypto/x509/x509_vfy.h @@ -77,6 +77,7 @@ extern "C" { #endif +#if 0 /* Outer object */ typedef struct x509_hash_dir_st { @@ -85,6 +86,7 @@ typedef struct x509_hash_dir_st int *dirs_type; int num_dirs_alloced; } X509_HASH_DIR_CTX; +#endif typedef struct x509_file_st { -- 2.25.1