Hi Eric, Coiby, On Fri, 2022-12-09 at 15:44 +0000, Eric Snowberg wrote: > > On Dec 9, 2022, at 3:26 AM, Coiby Xu <coxu@xxxxxxxxxx> wrote: > > > > Thanks for your work! The patch set looks good to me except for the > > requirement of an intermediate CA certificate should be vouched for by a > > root CA certificate before it can vouch for other certificates. What if > > users only want to enroll an intermediate CA certificate into the MOK? > > This question would need to be answered by the maintainers. The intermediate > requirement was based on my understanding of previous discussions requiring > there be a way to validate root of trust all the way back to the root CA. That definitely did not come from me. My requirement all along has been to support a single self-signed CA certificate for the end user/customer use case, so that they could create and load their own public key, signed by that CA, onto the trusted IMA/EVM keyrings. > > > If this requirement could be dropped, the code could be simplified and > > some issues could be resolved automatically, > > Agreed. I will make sure the issue below is resolved one way or the other, > once we have an agreement on the requirements. I totally agree with Coiby that there is no need for intermediate CA certificates be vouched for by a root CA certificate. In fact the closer the CA certificate is to the leaf code signing certificate, the better. As much as possible we want to limit the CA keys being loaded onto the machine keyring to those that are absolutely required. thanks, Mimi > > > 1. "[PATCH v2 03/10] KEYS: X.509: Parse Basic > > a root_ca filed to a certificate to indicate the subject of the > > certificate is a CA. The name root_ca implies it's also a root CA. But > > according to [1], both an intermediate and root CA will have > > root_ca=True. For example, the intermediate certificate of > > https://www.kernel.org/ has "Certificate Authority=Yes" in the basic > > constraints. Btw, a root CA certificate by definition is self-signed, > > so the following code in "[PATCH v2 05/10] KEYS: Introduce a CA > > endorsed flag" looks a bit strange to me, > > if (cert->kcs_set && cert->self_signed && cert->root_ca) > > prep->payload_flags |= KEY_ALLOC_PECA; > > > > 2. Since an intermediate CA certificate also has root_ca=True, > > "[PATCH v2 07/10] KEYS: X.509: Flag Intermediate CA certs as > > endorsed" won't work as intended i.e. this following else branch > > will never be reached, > > else if (!cert->self_signed && !cert->root_ca) > > prep->payload_flags |= KEY_MAYBE_PECA; > > > > 3. I see nowhere public_key->key_is_ca is set to true for an intermediate > > CA certificate after it gains the KEY_ALLOC_PECA flag. So it will fail > > restrict_link_by_ca even if the KEY_MAYBE_PECA flag is added. > > > > [1] https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.9 > > Thanks for reviewing the series. >
