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

[joeyli]

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@xxxxxxxx>
> > ---
> >  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