Skip to content

Commit

Permalink
deps: backport openssl patch of alt cert chains 1
Browse files Browse the repository at this point in the history 
This a backport of da084a5ec6cebd67ae27f2463ebe4a50bb840fa5 in
https://github.com/openssl/openssl by Matt Caswell <matt@openssl.org> as

In certain situations the server provided certificate chain may no
longer be valid. However the issuer of the leaf, or some intermediate
cert is in fact in the trust store.

When building a trust chain if the first attempt fails, then try to
see if alternate chains could be constructed that are trusted.

deps: backport openssl patch of alt cert chains 2

This a backport of 15dba5be6a4482a9ad7e5b846291f31e97e338ca in
https://github.com/openssl/openssl by Matt Caswell <matt@openssl.org> as

Add flag to inhibit checking for alternate certificate chains. Setting this
behaviour will force behaviour as per previous versions of OpenSSL

Fixes: #589
PR-URL: #1389
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
Shigeki Ohtsu committed Apr 14, 2015
1 parent 71316c4 commit ae8831f
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 70 deletions.
177 changes: 107 additions & 70 deletions deps/openssl/openssl/crypto/x509/x509_vfy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,11 @@ static X509 *lookup_cert_match(X509_STORE_CTX *ctx, X509 *x)

int X509_verify_cert(X509_STORE_CTX *ctx)
{
X509 *x, *xtmp, *chain_ss = NULL;
X509 *x, *xtmp, *xtmp2, *chain_ss = NULL;
int bad_chain = 0;
X509_VERIFY_PARAM *param = ctx->param;
int depth, i, ok = 0;
int num;
int num, j, retry;
int (*cb) (int xok, X509_STORE_CTX *xctx);
STACK_OF(X509) *sktmp = NULL;
if (ctx->cert == NULL) {
Expand Down Expand Up @@ -276,91 +276,128 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
break;
}

/* Remember how many untrusted certs we have */
j = num;
/*
* at this point, chain should contain a list of untrusted certificates.
* We now need to add at least one trusted one, if possible, otherwise we
* complain.
*/

/*
* Examine last certificate in chain and see if it is self signed.
*/

i = sk_X509_num(ctx->chain);
x = sk_X509_value(ctx->chain, i - 1);
if (cert_self_signed(x)) {
/* we have a self signed certificate */
if (sk_X509_num(ctx->chain) == 1) {
/*
* We have a single self signed certificate: see if we can find
* it in the store. We must have an exact match to avoid possible
* impersonation.
*/
ok = ctx->get_issuer(&xtmp, ctx, x);
if ((ok <= 0) || X509_cmp(x, xtmp)) {
ctx->error = X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT;
ctx->current_cert = x;
ctx->error_depth = i - 1;
if (ok == 1)
X509_free(xtmp);
bad_chain = 1;
ok = cb(0, ctx);
if (!ok)
goto end;
do {
/*
* Examine last certificate in chain and see if it is self signed.
*/
i = sk_X509_num(ctx->chain);
x = sk_X509_value(ctx->chain, i - 1);
if (cert_self_signed(x)) {
/* we have a self signed certificate */
if (sk_X509_num(ctx->chain) == 1) {
/*
* We have a single self signed certificate: see if we can
* find it in the store. We must have an exact match to avoid
* possible impersonation.
*/
ok = ctx->get_issuer(&xtmp, ctx, x);
if ((ok <= 0) || X509_cmp(x, xtmp)) {
ctx->error = X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT;
ctx->current_cert = x;
ctx->error_depth = i - 1;
if (ok == 1)
X509_free(xtmp);
bad_chain = 1;
ok = cb(0, ctx);
if (!ok)
goto end;
} else {
/*
* We have a match: replace certificate with store
* version so we get any trust settings.
*/
X509_free(x);
x = xtmp;
(void)sk_X509_set(ctx->chain, i - 1, x);
ctx->last_untrusted = 0;
}
} else {
/*
* We have a match: replace certificate with store version so
* we get any trust settings.
* extract and save self signed certificate for later use
*/
X509_free(x);
x = xtmp;
(void)sk_X509_set(ctx->chain, i - 1, x);
ctx->last_untrusted = 0;
chain_ss = sk_X509_pop(ctx->chain);
ctx->last_untrusted--;
num--;
j--;
x = sk_X509_value(ctx->chain, num - 1);
}
} else {
/*
* extract and save self signed certificate for later use
*/
chain_ss = sk_X509_pop(ctx->chain);
ctx->last_untrusted--;
num--;
x = sk_X509_value(ctx->chain, num - 1);
}
}

/* We now lookup certs from the certificate store */
for (;;) {
/* If we have enough, we break */
if (depth < num)
break;
/* We now lookup certs from the certificate store */
for (;;) {
/* If we have enough, we break */
if (depth < num)
break;
/* If we are self signed, we break */
if (cert_self_signed(x))
break;
ok = ctx->get_issuer(&xtmp, ctx, x);

/* If we are self signed, we break */
if (cert_self_signed(x))
break;
if (ok < 0)
return ok;
if (ok == 0)
break;
x = xtmp;
if (!sk_X509_push(ctx->chain, x)) {
X509_free(xtmp);
X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);
return 0;
}
num++;
}

ok = ctx->get_issuer(&xtmp, ctx, x);
/* we now have our chain, lets check it... */
i = check_trust(ctx);

if (ok < 0)
return ok;
if (ok == 0)
break;
/* If explicitly rejected error */
if (i == X509_TRUST_REJECTED)
goto end;
/*
* If it's not explicitly trusted then check if there is an alternative
* chain that could be used. We only do this if we haven't already
* checked via TRUSTED_FIRST and the user hasn't switched off alternate
* chain checking
*/
retry = 0;
if (i != X509_TRUST_TRUSTED
&& !(ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST)
&& !(ctx->param->flags & X509_V_FLAG_NO_ALT_CHAINS)) {
while (j-- > 1) {
xtmp2 = sk_X509_value(ctx->chain, j - 1);
ok = ctx->get_issuer(&xtmp, ctx, xtmp2);
if (ok < 0)
goto end;
/* Check if we found an alternate chain */
if (ok > 0) {
/*
* Free up the found cert we'll add it again later
*/
X509_free(xtmp);

x = xtmp;
if (!sk_X509_push(ctx->chain, x)) {
X509_free(xtmp);
X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);
return 0;
/*
* Dump all the certs above this point - we've found an
* alternate chain
*/
while (num > j) {
xtmp = sk_X509_pop(ctx->chain);
X509_free(xtmp);
num--;
ctx->last_untrusted--;
}
retry = 1;
break;
}
}
}
num++;
}
} while (retry);

/* we now have our chain, lets check it... */

i = check_trust(ctx);

/* If explicitly rejected error */
if (i == X509_TRUST_REJECTED)
goto end;
/*
* If not explicitly trusted then indicate error unless it's a single
* self signed certificate in which case we've indicated an error already
Expand Down
6 changes: 6 additions & 0 deletions deps/openssl/openssl/crypto/x509/x509_vfy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,12 @@ void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth);

/* Allow partial chains if at least one certificate is in trusted store */
# define X509_V_FLAG_PARTIAL_CHAIN 0x80000
/*
* If the initial chain is not trusted, do not attempt to build an alternative
* chain. Alternate chain checking was introduced in 1.1.0. Setting this flag
* will force the behaviour to match that of previous versions.
*/
# define X509_V_FLAG_NO_ALT_CHAINS 0x100000

# define X509_VP_FLAG_DEFAULT 0x1
# define X509_VP_FLAG_OVERWRITE 0x2
Expand Down

8 comments on commit ae8831f

@indutny
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good news @shigeki @bnoordhuis ! This patch is going to be backported to 1.0.2 branch!

@shigeki
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@indutny Yes, I've found them on git repository. We need not to apply private patches from 1.0.2b.
I also found an another backported patch in openssl/openssl@47daa15 . I wonder if this is needed in 1.0.2a since there is no loop for a cert chain in checking a issuer in 1.0.2a where there is one in master as https://github.com/openssl/openssl/blob/master/crypto/x509/x509_vfy.c#L538-L544 . But I might have missed something.

@indutny
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shigeki I'd stay close with the 1.0.2b anyway, but I do see your argument. :) Let's update to it once it'll be released, I think it should happen soon.

@shigeki
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@indutny Let's wait it until 1.0.2b. I'm just wondering if we need to apply the new backported patch to iojs immediately.

@indutny
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shigeki doesn't seem to be necessary. I'll ask Matt about the purpose of this patch.

@shigeki
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@indutny Thanks!

@indutny
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shigeki according to Matt it was backported by mistake! Good catch, man!

@shigeki
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@indutny It's my pleasure. Thanks for your feedback.

Please sign in to comment.