| Message ID | 20221214003401.4086781-4-eric.snowberg@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add CA enforcement keyring restrictions | expand |
> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h > index a299c9c56f40..7c5c0ad1c22e 100644 > --- a/crypto/asymmetric_keys/x509_parser.h > +++ b/crypto/asymmetric_keys/x509_parser.h > @@ -38,6 +38,7 @@ struct x509_certificate { > bool self_signed; /* T if self-signed (check unsupported_sig too) */ > bool unsupported_sig; /* T if signature uses unsupported crypto */ > bool blacklisted; > + bool root_ca; /* T if basic constraints CA is set */ > }; The variable "root_ca" should probably be renamed to just "ca", right?
> On Jan 4, 2023, at 5:29 AM, Jarkko Sakkinen <jarkko@kernel.org> wrote: > > On Thu, Dec 15, 2022 at 06:10:04AM -0500, Mimi Zohar wrote: >>> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h >>> index a299c9c56f40..7c5c0ad1c22e 100644 >>> --- a/crypto/asymmetric_keys/x509_parser.h >>> +++ b/crypto/asymmetric_keys/x509_parser.h >>> @@ -38,6 +38,7 @@ struct x509_certificate { >>> bool self_signed; /* T if self-signed (check unsupported_sig too) */ >>> bool unsupported_sig; /* T if signature uses unsupported crypto */ >>> bool blacklisted; >>> + bool root_ca; /* T if basic constraints CA is set */ >>> }; >> >> The variable "root_ca" should probably be renamed to just "ca", right? > > Perhaps is_ca? I am open to renaming this, but need an agreement on whether the “is_” should be used or not: https://lore.kernel.org/lkml/b28ea211d88e968a5487b20477236e9b507755f4.camel@linux.ibm.com/
On Wed, 2023-01-04 at 20:14 +0000, Eric Snowberg wrote: > > > On Jan 4, 2023, at 5:29 AM, Jarkko Sakkinen <jarkko@kernel.org> wrote: > > > > On Thu, Dec 15, 2022 at 06:10:04AM -0500, Mimi Zohar wrote: > >>> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h > >>> index a299c9c56f40..7c5c0ad1c22e 100644 > >>> --- a/crypto/asymmetric_keys/x509_parser.h > >>> +++ b/crypto/asymmetric_keys/x509_parser.h > >>> @@ -38,6 +38,7 @@ struct x509_certificate { > >>> bool self_signed; /* T if self-signed (check unsupported_sig too) */ > >>> bool unsupported_sig; /* T if signature uses unsupported crypto */ > >>> bool blacklisted; > >>> + bool root_ca; /* T if basic constraints CA is set */ > >>> }; > >> > >> The variable "root_ca" should probably be renamed to just "ca", right? > > > > Perhaps is_ca? > > I am open to renaming this, but need an agreement on whether the “is_” should be used or not: > > https://lore.kernel.org/lkml/b28ea211d88e968a5487b20477236e9b507755f4.camel@linux.ibm.com/ Examples of both functions and variables exist that are prefixed with "is_". One is a question; the other a statement. Naming the variable "is_ca" and using it like "if (cert->is_ca)" does make sense.
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 7a9b084e2043..b4443e507153 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -586,6 +586,15 @@ int x509_process_extension(void *context, size_t hdrlen, return 0; } + if (ctx->last_oid == OID_basicConstraints) { + if (vlen < 2 || v[0] != (ASN1_CONS_BIT | ASN1_SEQ)) + return -EBADMSG; + if (v[1] != vlen - 2) + return -EBADMSG; + if (vlen >= 4 && v[1] != 0 && v[2] == ASN1_BOOL && v[3] == 1) + ctx->cert->root_ca = true; + } + return 0; } diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h index a299c9c56f40..7c5c0ad1c22e 100644 --- a/crypto/asymmetric_keys/x509_parser.h +++ b/crypto/asymmetric_keys/x509_parser.h @@ -38,6 +38,7 @@ struct x509_certificate { bool self_signed; /* T if self-signed (check unsupported_sig too) */ bool unsupported_sig; /* T if signature uses unsupported crypto */ bool blacklisted; + bool root_ca; /* T if basic constraints CA is set */ }; /*
Parse the X.509 Basic Constraints. The basic constraints extension identifies whether the subject of the certificate is a CA. BasicConstraints ::= SEQUENCE { cA BOOLEAN DEFAULT FALSE, pathLenConstraint INTEGER (0..MAX) OPTIONAL } If the CA is true, store it in the x509_certificate. This will be used in a follow on patch that requires knowing if the public key is a CA. Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com> --- crypto/asymmetric_keys/x509_cert_parser.c | 9 +++++++++ crypto/asymmetric_keys/x509_parser.h | 1 + 2 files changed, 10 insertions(+)