Re: [PATCH][RESEND#15] keys: Handle missing Authority Key Identifier X509 extension

[Jarkko Sakkinen <jarkko@kernel.org>]

On Mon, Jan 11, 2021 at 03:27:57PM +0100, Andrew Zaborowski wrote:
> Hi,
> 
> On Sun, 10 Jan 2021 at 05:45, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > On Mon, Jan 04, 2021 at 05:40:47PM +0100, Andrew Zaborowski wrote:
> > > In a self-signed certificate the subject and issuer are the same and so
> > > the Authority Key Identifier X.509 v3 extension is explicitly made
> > > optional in RFC5280 section 4.2.1.1.
> > > crypto/asymmetric_keys/x509_cert_parser.c can't handle this and makes
> > > (at least) the restrict.c functions refuse to work with certificates
> > > that don't include the AKID.  Fix this by filling in the missing
> > > cert->sig->auth_ids with the certificate's own IDs after parsing and
> > > determinig the certificate is self-signed.
> > >
> > > The asymmetric_key_generate_id return value is not checked because it's
> > > already succeeded once at this point.
> > >
> > > There are root X.509 v3 certificates in use where this is the case,
> > > mostly oldish ones.
> > >
> > > Signed-off-by: Andrew Zaborowski <andrew.zaborowski@intel.com>
> > > Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> >
> > I'll take active role with these.
> 
> Great!
> 
> >
> > I don't think that adding field data that does not exist is a great
> > idea.
> >
> > I think that a more sane way to fix this issue, would be to add field
> > 'self_signed' to 'struct public_key_signature', and remove the
> > equivalent field from 'struct x509_certificate'.
> 
> Ok, that's also an option.  I went for directly adding the
> cert->sig->auth_ids because the name ".auth_ids" only implies that it
> contains the signing authority's key IDs, which is something that we
> have and does exist in the input file.  The name doesn't imply that it
> should specifically be in the AKID extension.  Similarly
> x509_key_preparse even generates a key description even though a
> description is not part of the x509 format.
> 
> >
> > Minor updates are required to:
> >
> > - x509_check_for_self_signed()
> > - pkcs7_verify_sig_chain()
> >
> > Then you can use then new field to refine the check in
> > restrict_link_by_signature().
> 
> Ok.

I mean in common sense: kernel stores the information needed.
It just needs a bit of relocation.

/Jarkko