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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to