lkml.org 
[lkml]   [2014]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier
On Thu, Nov 20, 2014 at 04:54:14PM +0000, David Howells wrote:

[..]
> @@ -215,21 +219,42 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> /* Look through the X.509 certificates in the PKCS#7 message's
> * list to see if the next one is there.
> */
> - pr_debug("- want %*phN\n",
> - x509->auth_skid->len, x509->auth_skid->data);
> - for (p = pkcs7->certs; p; p = p->next) {
> - if (!p->skid)
> - continue;
> - pr_debug("- cmp [%u] %*phN\n",
> - p->index, p->skid->len, p->skid->data);
> - if (asymmetric_key_id_same(p->skid, x509->auth_skid))
> - goto found_issuer;
> + auth = x509->auth_id;
> + if (auth) {
> + pr_debug("- want %*phN\n", auth->len, auth->data);
> + for (p = pkcs7->certs; p; p = p->next) {
> + pr_debug("- cmp [%u] %*phN\n",
> + p->index, p->id->len, p->id->data);
> + if (asymmetric_key_id_same(p->id, auth))
> + goto found_issuer_check_skid;
> + }
> + } else {
> + auth = x509->auth_skid;
> + pr_debug("- want %*phN\n", auth->len, auth->data);
> + for (p = pkcs7->certs; p; p = p->next) {
> + if (!p->skid)
> + continue;
> + pr_debug("- cmp [%u] %*phN\n",
> + p->index, p->skid->len, p->skid->data);
> + if (asymmetric_key_id_same(p->skid, auth))
> + goto found_issuer;
> + }
> }
>
> /* We didn't find the root of this chain */
> pr_debug("- top\n");
> return 0;
>
> + found_issuer_check_skid:
> + /* We matched issuer + serialNumber, but if there's an
> + * authKeyId.keyId, that must match the CA subjKeyId also.
> + */
> + if (x509->auth_skid &&
> + !asymmetric_key_id_same(p->skid, x509->auth_skid)) {
> + pr_warn("Sig %u: X.509 chain contains auth-skid nonmatch (%u->%u)\n",
> + sinfo->index, x509->index, p->index);
> + return -EKEYREJECTED;
> + }

Hi David,

A minor nit.

pkcs7_verify_sig_chain() is getting big with multiple goto labels. Will
it make sense to introduce a helper function to see if cert B is authority
cert of cert A or not. And then we should be able to get rid of labels
like found_issuer_check_skid() and some of the inline code also go away.

Thanks
Vivek


\
 
 \ /
  Last update: 2014-11-21 17:01    [W:0.242 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site