linux-kernel - Re: [PATCH v9,1/4] X.509: Add CodeSigning extended key usage parsing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 31 Aug 2022 15:31:07 +0800
From:   joeyli <jlee@...e.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>
Cc:     "Lee, Chun-Yi" <joeyli.kernel@...il.com>,
        David Howells <dhowells@...hat.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S . Miller" <davem@...emloft.net>,
        Ben Boeckel <me@...boeckel.net>,
        Randy Dunlap <rdunlap@...radead.org>,
        Malte Gell <malte.gell@....de>,
        Varad Gautam <varad.gautam@...e.com>,
        Mimi Zohar <zohar@...ux.ibm.com>, keyrings@...r.kernel.org,
        linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9,1/4] X.509: Add CodeSigning extended key usage parsing

Hi Jarkko,

Thanks for your review!

On Fri, Aug 26, 2022 at 11:23:00PM +0300, Jarkko Sakkinen wrote:
> On Thu, Aug 25, 2022 at 10:23:11PM +0800, Lee, Chun-Yi wrote:
> > This patch adds the logic for parsing the CodeSign extended key usage
> 
> It's *not* a patch once it is applied.
> 
> And isn't the identifier actually "codeSign", not "CodeSign"? Please,
> format identifier correctly in order not to cause confusion.
> 
> So, how I would rewrite the first sentence, would be:
> 
>   Add the logic for parsing codeSign extended key usage field, as
>   described in RFC2459, section "4.2.1.13  Extended key usage
>   field.
> 
> E.g. it took me 15 minutes to review the commit message alone
> because I could not remember the RFC number off top of my head.
>

Thanks for your suggestion. As your rewrited sentence, Adding RFC2459
and section is better for reader of patch descriptor. 

I will change the patch descriptor.
 
> > extension in X.509. The parsing result will be set to the
> > ext_key_usage
> > flag which is carried by public key. It can be used in the PKCS#7
> > verification.
> > 
> > Signed-off-by: "Lee, Chun-Yi" <jlee@...e.com>
> > ---
> >  crypto/asymmetric_keys/x509_cert_parser.c | 25 +++++++++++++++++++++++
> >  include/crypto/public_key.h               |  1 +
> >  include/linux/oid_registry.h              |  5 +++++
> >  3 files changed, 31 insertions(+)
> > 
> > diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> > index 2899ed80bb18..1f67e0adef65 100644
> > --- a/crypto/asymmetric_keys/x509_cert_parser.c
> > +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> > @@ -554,6 +554,8 @@ int x509_process_extension(void *context, size_t hdrlen,
> >  	struct x509_parse_context *ctx = context;
> >  	struct asymmetric_key_id *kid;
> >  	const unsigned char *v = value;
> > +	int i = 0;
> > +	enum OID oid;
> 
> I'd reorder the declarations (suggestion).
>

I will change the order.

Thanks a lot!
Joey Lee