On Thu, Oct 22, 2020 at 08:44:29PM -0700, Jeremy Evans wrote: > I was trying to diagnose a certificate validation failure in Ruby's > openssl extension tests with LibreSSL 3.2.2, and it was made more > difficult because the verification error type was dropped, resulting > in a SSL_R_CERTIFICATE_VERIFY_FAILED error where > SSL_get_verify_result returned X509_V_OK.
Could you perhaps isolate a test case or explain the reproducer in a bit more detail? I tried running ruby 2.7's regress from ports but that always resulted in a SIGABRT (usually while running test_ftp.rb with or without your diff). With my diff below I once got past this abort and saw this: OpenSSL::TestSSL#test_verify_result [/usr/ports/pobj/ruby-2.7.2/ruby-2.7.2/test/openssl/utils.rb:279]: exceptions on 1 threads: <19> expected but was <20>. Did you see <0> here instead? > I think this patch will fix it. Compile tested only. OKs, or is there > a better way to fix it? This will probably also address the issue with the haproxy test reported on the libressl list: https://marc.info/?l=libressl&m=160339671313132&w=2 https://github.com/haproxy/haproxy/issues/916 Regarding your diff, I think setting the error on the store context should not be the responsibility of x509_verify()'s caller. After all, x509_verify() will likely end up being public API. The below diff should have the same effect as yours. > Thanks, > Jeremy > > Index: x509_vfy.c > =================================================================== > RCS file: /cvs/src/lib/libcrypto/x509/x509_vfy.c,v > retrieving revision 1.81 > diff -u -p -r1.81 x509_vfy.c > --- x509_vfy.c 26 Sep 2020 02:06:28 -0000 1.81 > +++ x509_vfy.c 23 Oct 2020 03:34:10 -0000 > @@ -680,6 +680,9 @@ X509_verify_cert(X509_STORE_CTX *ctx) > if ((vctx = x509_verify_ctx_new_from_xsc(ctx, roots)) != NULL) { > ctx->error = X509_V_OK; /* Initialize to OK */ > chain_count = x509_verify(vctx, NULL, NULL); > + if (vctx->error) { > + ctx->error = vctx->error; > + } > } > > sk_X509_pop_free(roots, X509_free); > Index: x509/x509_verify.c =================================================================== RCS file: /var/cvs/src/lib/libcrypto/x509/x509_verify.c,v retrieving revision 1.13 diff -u -p -r1.13 x509_verify.c --- x509/x509_verify.c 26 Sep 2020 15:44:06 -0000 1.13 +++ x509/x509_verify.c 23 Oct 2020 05:04:05 -0000 @@ -853,13 +853,13 @@ x509_verify(struct x509_verify_ctx *ctx, if (ctx->roots == NULL || ctx->max_depth == 0) { ctx->error = X509_V_ERR_INVALID_CALL; - return 0; + goto err; } if (ctx->xsc != NULL) { if (leaf != NULL || name != NULL) { ctx->error = X509_V_ERR_INVALID_CALL; - return 0; + goto err; } leaf = ctx->xsc->cert; @@ -872,34 +872,34 @@ x509_verify(struct x509_verify_ctx *ctx, */ if ((ctx->xsc->chain = sk_X509_new_null()) == NULL) { ctx->error = X509_V_ERR_OUT_OF_MEM; - return 0; + goto err; } if (!X509_up_ref(leaf)) { ctx->error = X509_V_ERR_OUT_OF_MEM; - return 0; + goto err; } if (!sk_X509_push(ctx->xsc->chain, leaf)) { X509_free(leaf); ctx->error = X509_V_ERR_OUT_OF_MEM; - return 0; + goto err; } ctx->xsc->error_depth = 0; ctx->xsc->current_cert = leaf; } if (!x509_verify_cert_valid(ctx, leaf, NULL)) - return 0; + goto err; if (!x509_verify_cert_hostname(ctx, leaf, name)) - return 0; + goto err; if ((current_chain = x509_verify_chain_new()) == NULL) { ctx->error = X509_V_ERR_OUT_OF_MEM; - return 0; + goto err; } if (!x509_verify_chain_append(current_chain, leaf, &ctx->error)) { x509_verify_chain_free(current_chain); - return 0; + goto err; } if (x509_verify_ctx_cert_is_root(ctx, leaf)) x509_verify_ctx_add_chain(ctx, current_chain); @@ -925,4 +925,9 @@ x509_verify(struct x509_verify_ctx *ctx, return ctx->xsc->verify_cb(ctx->chains_count, ctx->xsc); } return (ctx->chains_count); + + err: + if (ctx->xsc != NULL) + ctx->xsc->error = ctx->error; + return 0; }
