Date:   Fri, 23 Dec 2022 16:13:49 +0000
From:   Eric Snowberg <eric.snowberg@...cle.com>
To:     Mimi Zohar <zohar@...ux.ibm.com>
CC:     Jarkko Sakkinen <jarkko@...nel.org>, Coiby Xu <coxu@...hat.com>,
        David Howells <dhowells@...hat.com>,
        David Woodhouse <dwmw2@...radead.org>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "dmitry.kasatkin@...il.com" <dmitry.kasatkin@...il.com>,
        "paul@...l-moore.com" <paul@...l-moore.com>,
        "jmorris@...ei.org" <jmorris@...ei.org>,
        "serge@...lyn.com" <serge@...lyn.com>,
        "pvorel@...e.cz" <pvorel@...e.cz>,
        "noodles@...com" <noodles@...com>, "tiwai@...e.de" <tiwai@...e.de>,
        Kanth Ghatraju <kanth.ghatraju@...cle.com>,
        Konrad Wilk <konrad.wilk@...cle.com>,
        Elaine Palmer <erpalmer@...ux.vnet.ibm.com>,
        "keyrings@...r.kernel.org" <keyrings@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>
Subject: Re: [PATCH v3 00/10] Add CA enforcement keyring restrictions



> On Dec 22, 2022, at 8:41 AM, Mimi Zohar <zohar@...ux.ibm.com> wrote:
> 
> On Thu, 2022-12-22 at 15:15 +0000, Eric Snowberg wrote:
>> 
>>> On Dec 21, 2022, at 12:01 PM, Mimi Zohar <zohar@...ux.ibm.com> wrote:
>>> 
>>> On Wed, 2022-12-21 at 18:27 +0000, Eric Snowberg wrote:
>>>> 
>>>>> On Dec 18, 2022, at 5:21 AM, Mimi Zohar <zohar@...ux.ibm.com> wrote:
>>>>> 
>>>>> On Fri, 2022-12-16 at 22:06 +0800, Coiby Xu wrote:
>>>>>> Hi Eric and Mimi,
>>>>>> 
>>>>>> On Thu, Dec 15, 2022 at 09:45:37PM +0000, Eric Snowberg wrote:
>>>>>>> 
>>>>>>> 
>>>>>>>>>>>>> A CA cert shall be defined as any X509 certificate that contains the
>>>>>>>>>>>>> keyCertSign key usage and has the CA bit set to true.
>>>>>>>>>>>> 
>>>>>>>>>>>> Hi Eric,
>>>>>>>>>>>> 
>>>>>>>>>>>> Allowing CA certificates with the digitalSignature key usage flag
>>>>>>>>>>>> enabled defeats the purpose of the new Kconfig.  Please update the
>>>>>>>>>>>> above definition to exclude the digitalSignature key usage flag and
>>>>>>>>>>>> modify the code accordingly.
>>>>>>>>>>> 
>>>>>>>>>>> Within v2, the request was made to allow Intermediate CA certificates to be
>>>>>>>>>>> loaded directly.  The Intermediate CA referenced was the one used by kernel.org.
>>>>>>>>>>> This Intermediate CA contains both digitalSignature and keyCertSign.  If the code
>>>>>>>>>>> is changed to exclude this certificate, now the root CA has to be loaded again.  Is that
>>>>>>>>>>> the intent?
>>>>>>>>>> 
>>>>>>>>>> That definitely was not the intent.  Nor would it address the issue of
>>>>>>>>>> a particular intermediate CA certificate having both keyCertSign and
>>>>>>>>>> digitalSignature.
>>>>>>>>> 
>>>>>>>>> Sorry, I’m not following.  Why is it an issue that an intermediate CA certificate contains
>>>>>>>>> both keyCertSign and digitalSignature? Why would we want to exclude an Intermediate
>>>>>>>>> CA cert like the one used on kernel.org?
>>>>>>>> 
>>>>>>>> I must be missing something.  Isn't the purpose of "keyUsage" to
>>>>>>>> minimize how a certificate may be used?   Why would we want the same
>>>>>>>> certificate to be used for both certificate signing and code signing?
>>>>>>> 
>>>>>>> Every 3rd party intermediate CA I have looked at so far contains both set. Most have CRLSign set.
>>>>>>> Typically the root CA contains keyCertSign and CRLSign, but some also have digitalSignature
>>>>>>> set.  Finding a 3rd party Intermediate CA without digitalSignature set is probably going to be
>>>>>>> challenging and will severely limit usage.
>>>>>> 
>>>>>> How about allowing both keyCertSign and digitalSignature asserted but
>>>>>> issuing a warning for this case?
>>>>>> 
>>>>>> Here's my rationale for this proposal.
>>>>>> 
>>>>>> I assume we should conform to some X.509 specifications. So I checked
>>>>>> "RFC 5280: Internet X.509 Public Key Infrastructure Certificate and
>>>>>> Certificate Revocation List (CRL) Profile" [1] and ITU-T X.509 (2012-10)
>>>>>> [2].
>>>>>> 
>>>>>> [1] states in 4.2.1.3. Key Usage,
>>>>>>  "If the keyUsage extension is present, then the subject public key
>>>>>>  MUST NOT be used to verify signatures on certificates or CRLs unless
>>>>>>  the corresponding keyCertSign or cRLSign bit is set.  If the subject
>>>>>>  public key is only to be used for verifying signatures on
>>>>>>  certificates and/or CRLs, then the digitalSignature and
>>>>>>  nonRepudiation bits SHOULD NOT be set.  However, the digitalSignature
>>>>>>  and/or nonRepudiation bits MAY be set in addition to the keyCertSign
>>>>>>  and/or cRLSign bits if the subject public key is to be used to verify
>>>>>>  signatures on certificates and/or CRLs as well as other objects."
>>>>>> 
>>>>>> and [2] states in 8.2.2.3 Key usage extension that,
>>>>>> "More than one bit may be set in an instance of the keyUsage extension.
>>>>>> The setting of multiple bits shall not change the meaning of each
>>>>>> individual bit but shall indicate that the certificate may be used for
>>>>>> all of the purposes indicated by the set bits. There may be risks
>>>>>> incurred when setting multiple bits. A review of those risks is
>>>>>> documented in Annex I."
>>>>>> 
>>>>>> I interpret the above texts as we should allow both keyCertSign and
>>>>>> digitalSignature. However [2] warns about the risks of setting multiple
>>>>>> bits. Quoting Annex I,
>>>>>> 
>>>>>> "Combining the contentCommitment bit in the keyUsage certificate
>>>>>> extension with other keyUsage bits may have security implications
>>>>>> depending on the security environment in which the certificate is to be
>>>>>> used. If the subject's environment can be fully controlled and trusted,
>>>>>> then there are no specific security implications. For example, in cases
>>>>>> where the subject is fully confident about exactly which data is signed
>>>>>> or cases where the subject is fully confident about the security
>>>>>> characteristics of the authentication protocol being used. If the
>>>>>> subject's environment is not fully controlled or not fully trusted, then
>>>>>> unintentional signing of commitments is possible. Examples include the
>>>>>> use of badly formed authentication exchanges and the use of a rogue
>>>>>> software component. If untrusted environments are used by a subject,
>>>>>> these security implications can be limited through use of the following
>>>>>> measures:   
>>>>>>  – to not combine the contentCommitment key usage setting in
>>>>>>    certificates with any other key usage setting and to use the
>>>>>>    corresponding private key only with this certificate;   
>>>>>> 
>>>>>>  – to limit the use of private keys associated with certificates that
>>>>>>    have the contentCommitment key usage bit set, to environments which
>>>>>>    are considered adequately controlled and trustworthy"
>>>>>> 
>>>>>> So maybe it's useful to add a warning if both keyCertSign and
>>>>>> digitalSignature are asserted.
>>>>> 
>>>>> Coiby, thank you for adding these details.  I was hoping others would
>>>>> chime in as well.  I agree at minimum there should be a warning.
>>>> 
>>>> A warning could be added.
>>>> 
>>>>> Perhaps instead of making INTEGRITY_CA_MACHINE_KEYRING dependent on
>>>>> INTEGRITY_MACHINE_KEYRING, make them a Kconfig "choice" to support the
>>>>> more restrictive certificate use case requirements:  all certificates,
>>>>> CA certificate signing and digital signature, only CA certificate
>>>>> signing.
>>>> 
>>>> As could support for additional restrictions.
>>>> 
>>>> Would these additions be required within this series? What is missing from this 
>>>> discussion is why would these additions be necessary?  Why should the kernel 
>>>> enforce a restriction that is beyond the scope of the X.509 spec?  If a warning was 
>>>> to be added, what would be the justification for adding this additional code?  From 
>>>> my research every single 3rd party code signing intermediate CA would be flagged 
>>>> with the warning.  Isn’t this just going to cause confusion?  Or is there a benefit that 
>>>> I am missing that needs to be stated?
>>> 
>>> You're focusing on third party kernel modules and forgetting about the
>>> simple use case of allowing an end user (or business) to sign their own
>>> code.  True they could use the less restrictive CA certificates, but it
>>> is unnecessary.
>> 
>> My focus is on signing user-space applications, as outlined in the cover letter.  This 
>> series has nothing to do with kernel modules.  Most end-users and businesses rely on 
>> a third party to deal with code signing.  All third party code signing services I have 
>> found use an intermediate CA containing more than just the keyCertSign usage set.  
>> It seems to be an industry accepted practice that does not violate the spec. Before writing
>> new code to either warn or exclude a third party intermediate CA,  I would like to understand 
>> the motivation behind this request.
> 
> In older discussions there are comments like, "Any CA certificate, no
> matter if it's a root or an intermediate, must have the keyCertSign
> extension. If you want to sign a revocation list (CRL) with the CA
> certificate as well (you usually do want that), than you have to add
> cRLSign as well. Any other keyUsages can and should be avoided for CA
> certificates."
> 
> The question as to "why" this changed to include "digitalSignature" was
> posed here [2] with the response being for "OCSP".   It also includes a
> link to Entrusts root and intermediate CAs with just keyCertSign and
> cRLSign keyUsages.
> 
> The matchine keyring is a means of establishing a new root of trust. 
> The motivation for further restricting CA certificates to just
> keyCertSign and cRLSign keyUsages is to limit how the CA certificates
> may be used.  They should not be used for code signing.

Fair enough.  If this will be viewed as justification for adding the additional 
code, I can work on adding it.  Above you mentioned a warning would be needed 
at a minimum and a restriction could be placed behind a Kconfig.  How about for 
the default case I add the warning and when compiling with 
INTEGRITY_CA_MACHINE_KEYRING the restriction will be enforced.

Powered by blists - more mailing lists