Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 31 Aug 2022 15:31:07 +0800
From:   joeyli <jlee@suse.com>
To:     Jarkko Sakkinen <jarkko@kernel.org>
Cc:     "Lee, Chun-Yi" <joeyli.kernel@gmail.com>,
        David Howells <dhowells@redhat.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        "David S . Miller" <davem@davemloft.net>,
        Ben Boeckel <me@benboeckel.net>,
        Randy Dunlap <rdunlap@infradead.org>,
        Malte Gell <malte.gell@gmx.de>,
        Varad Gautam <varad.gautam@suse.com>,
        Mimi Zohar <zohar@linux.ibm.com>, keyrings@vger.kernel.org,
        linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9,1/4] X.509: Add CodeSigning extended key usage parsing
Message-ID: <20220831073107.GT5247@linux-l9pv.suse>
References: <20220825142314.8406-1-jlee@suse.com>
 <20220825142314.8406-2-jlee@suse.com>
 <YwkrpLPyU0huPduE@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YwkrpLPyU0huPduE@kernel.org>
User-Agent: Mutt/1.11.4 (2019-03-13)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?/CghK1t0LX2tV0kPqSPjUbAfjNvCi+WUcL0zej/I8vi1ZFMw4FvTFOoSuMCW?=
 =?us-ascii?Q?2ukvUdopdPm1MRV1dDSi/rh4NLdV/B2dAx2/pRYtr/bBabj2J/FUOP/2RzTV?=
 =?us-ascii?Q?L048Rbg/tlCVqVLhwKvtRR0Cb3H57nX3oR1Bxn9aBIMWWwOXL46UJ1jTGJIP?=
 =?us-ascii?Q?z5OGkUdLTnBMy2Rb6C2Q8gsMGEgQ/+nwxcr9AmoUCVwjpNRXwcd5hiSCud1v?=
 =?us-ascii?Q?bVZ4byrOcWCs1WMR677HYN9WFWrERv8n4Z4v1iIQeLJ1I/aKYiV+K7X+ktWi?=
 =?us-ascii?Q?HX5MIsMX8FzeBbVR/d5gAtUoQsM9kElp2sktpF4QitrN+mFUAaWmchukXYOc?=
 =?us-ascii?Q?oLQXGe/UGCvms1EtDuXPX2aRq5oyrwS21qU4o7qefxtd/KZhvjw8LyguhnQb?=
 =?us-ascii?Q?+O6WAHbArPj3mpZmsO78K1ltpelI8ytR/bgU83Zy21DKlyJNzbLaHBd7ZypD?=
 =?us-ascii?Q?84Uu6+BvFJso8oItmVq+oZ/Vp6UTUaHpiz3P+tkn+l+s29mvcY3bgC2R/X21?=
 =?us-ascii?Q?0KX+QECx7cQeTx1W6WG+8MzoyphmnPYGEzq6lXgW0gs4E69gEpHgW+shJ5Xa?=
 =?us-ascii?Q?0o0Nn2RdCuTe8Rg8hKjonweZQriuGnx5oVjkiHi4EaM5RtPKMdmElfxnaE6h?=
 =?us-ascii?Q?rKz3MKT+Vn8RLgWY874SbNGGjNUEF0eSKpml7WP218HptM6crX3yKNEUQbfP?=
 =?us-ascii?Q?RbGeIikRtGTaBTDcTYeQkKmrqkQ2o+F9EJ8J+ptHirsH5B4mhWvijgjb+A3o?=
 =?us-ascii?Q?Kaox7MFyga0zBkX/0YB7SjBbBHV4wWDKXbHpp4Y+4cKZ6c86X5kJkdkDEHcB?=
 =?us-ascii?Q?nQUgXKaYHrQQ7XQGwalw5cwVs9Sp4wqU4MffEuB1DJ6y7Z5gN6qRjClxsplM?=
 =?us-ascii?Q?PgI14UQt/R5oVvmgp4y/9Emj7AdQ8EQGzm+NEUlqOm7w1biyT27SX3Kjqly4?=
 =?us-ascii?Q?MT719M2A9xvcRelK13VLtfRiSn8SglJAgN3G7tkf7SMi4Q2QdP8Ex3wqV9zJ?=
 =?us-ascii?Q?ChDfXEfxiaFh8xkpCrWThDWH5xuUrtXS/cPfyNKGuVlZCRhhewtpvd8yNqb2?=
 =?us-ascii?Q?SazsMG/QsCWUyXwlr8v9yuo1qbTqwQr72u4ptHPmwuR/vzfxbhnSHj2dsGii?=
 =?us-ascii?Q?k7oq3hWPFxGHmeeOs+vfV13v6QjPIkIj/j9dnMskSBdRd+OKOZGP4x1YInfT?=
 =?us-ascii?Q?6Jja+SErpXV6gZ8aQrM+FORj2k5t3kybSACdTwoaVz8snN8qfqKKCIwMparK?=
 =?us-ascii?Q?O8mpiAwrwb35vqRDjUHpY6iOMgohPk83ROOiU8ThxLgbALZENSpbbBjNI4RS?=
 =?us-ascii?Q?tChQtklnSgPiQOIXDAUbWhp27cjBXXipSVZyOYE7VgN5SYTcT56fp5utk/75?=
 =?us-ascii?Q?AM3evdmoDHptOkB81Yn5Bzhy9NrLGB/4Vq6AieCRjreNt8PSZ4cUylOKImld?=
 =?us-ascii?Q?LZN5GMz8FGinEwT6VYgG/WfUemZ7m8i3OuBQ1kat5io835LuC+N0VcspDeKJ?=
 =?us-ascii?Q?NAPboyQw2AhpwZtkEK6iZMPkAhgkoGsUml5MDkao6BHGTBOVGbCaOASg/6RV?=
 =?us-ascii?Q?vH8O2HOhxxBSG4l8DsA=3D?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cc9998ef-d084-4341-acb0-08da8b22ccfc
X-MS-Exchange-CrossTenant-AuthSource: DB8PR04MB7164.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Aug 2022 07:31:21.4980
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: F5apQp+PlAvQwmdaJhugMlVqD4aSIwSezwE2Il68zE7E6vSUpn2qEd+yegt3Ypf/
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0401MB2328
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

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@suse.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