IMA: Data included in the key measurement

IMA: Data included in the key measurement

[Lakshmi Ramasubramanian]

Hi Mimi,

 >>> everything needed for verifying a signature is included in
 >>> the key measurement.

Regarding the requirement you had stated above, I would like some 
clarification.

When I started this change to measure keys through IMA, the use case we 
had in mind was enabling an attestation service, for instance, to verify 
if the client has only known good (trusted) keys - for example, in 
keyrings such as ".builtin_trusted_keys", ".ima", etc.

On the client IMA verifies the signature of system binaries using keys 
in the IMA keyring. And, if the config namely 
CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY is enabled, 
only keys signed by a built-in trusted key can be added to the IMA keyring.

An attestation service can keep a list of public keys of "known good 
(trusted)" keys for various keyrings, and verify against the measurement 
data provided by the client.

To achieve the above we decided to include only the public key in the 
key measurement buffer.

I would like to know what benefit we'd get by including "everything 
needed for verifying a signature in the key measurement"?

 From testing point of view, if we have the certificate (like the .DER 
file), we can validate the key measurement data in the IMA log.

Do you see a need to include more data or the entire cert for the 
product code?

thanks,
  -lakshmi

Re: IMA: Data included in the key measurement

[James Bottomley]

On Thu, 2019-11-21 at 08:17 -0800, Lakshmi Ramasubramanian wrote:
> Hi Mimi,
> 
>  >>> everything needed for verifying a signature is included in
>  >>> the key measurement.
> 
> Regarding the requirement you had stated above, I would like some 
> clarification.
> 
> When I started this change to measure keys through IMA, the use case
> we had in mind was enabling an attestation service, for instance, to
> verify if the client has only known good (trusted) keys - for
> example, in keyrings such as ".builtin_trusted_keys", ".ima", etc.
> 
> On the client IMA verifies the signature of system binaries using
> keys in the IMA keyring. And, if the config namely 
> CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY is
> enabled, only keys signed by a built-in trusted key can be added to
> the IMA keyring.
> 
> An attestation service can keep a list of public keys of "known good 
> (trusted)" keys for various keyrings, and verify against the
> measurement data provided by the client.
> 
> To achieve the above we decided to include only the public key in
> the key measurement buffer.
> 
> I would like to know what benefit we'd get by including "everything 
> needed for verifying a signature in the key measurement"?  X.509
> itself doesn't buy this isomorphism property, which is why the
> subject key id 
> 
>  From testing point of view, if we have the certificate (like the
> .DER file), we can validate the key measurement data in the IMA log.
> 
> Do you see a need to include more data or the entire cert for the 
> product code?

You're making the assumption that the public key and the certificate
are isomorphic.  That's only true if you trust the issuer (which you
obviously do, since it's you [microsoft]) but nothing in X.509 prevents
the issuer from issuing multiple certificates with the same public key
and different properties.  Even in your use case, I would think
attesting to whether the certificate had expired or not would be
useful.

James

Re: IMA: Data included in the key measurement

[Lakshmi Ramasubramanian]

On 11/21/19 8:38 AM, James Bottomley wrote:

Hi James,

> You're making the assumption that the public key and the certificate
> are isomorphic.  That's only true if you trust the issuer (which you
> obviously do, since it's you [microsoft]) but nothing in X.509 prevents
> the issuer from issuing multiple certificates with the same public key
> and different properties.  Even in your use case, I would think
> attesting to whether the certificate had expired or not would be
> useful.

I agree attesting the x509 certificate and not just the public key is 
the right approach. But in the kernel the certificate is not stored - 
only the key extracted from the certificate is stored (please correct me 
if I am wrong).

When IMA, for instance, verifies the file signature only the public key 
(the data in "struct public_key") of the corresponding IMA key is used - 
the certificate is not used.

In my key measurement implementation, IMA hook is called when 
key_create_or_update() function has successfully added\updated a key in 
a keyring. At that point, I do not have the corresponding x509 certificate.

Please let me know if the kernel indeed stores the x509 certificate also 
and it can be queried when the corresponding key is added to a keyring. 
I'll make the changes to measure the x509 cert instead of just the 
public key.

Thank you.

  -lakshmi

Re: IMA: Data included in the key measurement

[James Bottomley]

On Thu, 2019-11-21 at 17:15 -0800, Lakshmi Ramasubramanian wrote:
> On 11/21/19 8:38 AM, James Bottomley wrote:
> 
> Hi James,
> 
> > You're making the assumption that the public key and the
> > certificate are isomorphic.  That's only true if you trust the
> > issuer (which you obviously do, since it's you [microsoft]) but
> > nothing in X.509 prevents the issuer from issuing multiple
> > certificates with the same public key and different
> > properties.  Even in your use case, I would think
> > attesting to whether the certificate had expired or not would be
> > useful.
> 
> I agree attesting the x509 certificate and not just the public key
> is the right approach. But in the kernel the certificate is not
> stored - only the key extracted from the certificate is stored
> (please correct me  if I am wrong).

It depends what you mean by "store".  The built in certificates are
available in the system_certificate_list array; IMA tends to expect
public keys as X.509 certificates.  In the actual keyring key, we get
the subject + serial/skid, the subtype the keyids and the signature. 
It does seem we don't set the key expiry to the expiry date of the
cert, though.  And before loading we do verify the key chain (depending
on how we were loaded).

> When IMA, for instance, verifies the file signature only the public
> key (the data in "struct public_key") of the corresponding IMA key is
> used - the certificate is not used.

Once you've performed the certificate verification, we only need
certain parameters for signature verification, so we store them. 
However, what we verify depends on the signature algorithm. For pkcs1,
obviously we can only check the signature and nothing else, so the cert
and the public key become isomorphic here.  For pkcs7, which is what
IMA mostly uses, we do check some of the parameters.

> In my key measurement implementation, IMA hook is called when 
> key_create_or_update() function has successfully added\updated a key
> in a keyring. At that point, I do not have the corresponding x509
> certificate.

It sounds like you might be hooking into the wrong place ... we
definitely pass in the whole certificate for public key load.

> Please let me know if the kernel indeed stores the x509 certificate
> also and it can be queried when the corresponding key is added to a
> keyring. I'll make the changes to measure the x509 cert instead of
> just the public key.

If you measure at time of insertion you should be able to measure the
entire key because it's inserted as a complete certificate.  If there's
additional data you need to retrieve after the load, we might be able
to store it in addition to the data we already save from the
certificate.

James

Re: IMA: Data included in the key measurement

[Lakshmi Ramasubramanian]

On 11/22/19 8:17 AM, James Bottomley wrote:

Thanks for the info James. I'll investigate further.

  -lakshmi

> On Thu, 2019-11-21 at 17:15 -0800, Lakshmi Ramasubramanian wrote:
>> On 11/21/19 8:38 AM, James Bottomley wrote:
>>
>> Hi James,
>>
>>> You're making the assumption that the public key and the
>>> certificate are isomorphic.  That's only true if you trust the
>>> issuer (which you obviously do, since it's you [microsoft]) but
>>> nothing in X.509 prevents the issuer from issuing multiple
>>> certificates with the same public key and different
>>> properties.  Even in your use case, I would think
>>> attesting to whether the certificate had expired or not would be
>>> useful.
>>
>> I agree attesting the x509 certificate and not just the public key
>> is the right approach. But in the kernel the certificate is not
>> stored - only the key extracted from the certificate is stored
>> (please correct me  if I am wrong).
> 
> It depends what you mean by "store".  The built in certificates are
> available in the system_certificate_list array; IMA tends to expect
> public keys as X.509 certificates.  In the actual keyring key, we get
> the subject + serial/skid, the subtype the keyids and the signature.
> It does seem we don't set the key expiry to the expiry date of the
> cert, though.  And before loading we do verify the key chain (depending
> on how we were loaded).
> 
>> When IMA, for instance, verifies the file signature only the public
>> key (the data in "struct public_key") of the corresponding IMA key is
>> used - the certificate is not used.
> 
> Once you've performed the certificate verification, we only need
> certain parameters for signature verification, so we store them.
> However, what we verify depends on the signature algorithm. For pkcs1,
> obviously we can only check the signature and nothing else, so the cert
> and the public key become isomorphic here.  For pkcs7, which is what
> IMA mostly uses, we do check some of the parameters.
> 
>> In my key measurement implementation, IMA hook is called when
>> key_create_or_update() function has successfully added\updated a key
>> in a keyring. At that point, I do not have the corresponding x509
>> certificate.
> 
> It sounds like you might be hooking into the wrong place ... we
> definitely pass in the whole certificate for public key load.
> 
>> Please let me know if the kernel indeed stores the x509 certificate
>> also and it can be queried when the corresponding key is added to a
>> keyring. I'll make the changes to measure the x509 cert instead of
>> just the public key.
> 
> If you measure at time of insertion you should be able to measure the
> entire key because it's inserted as a complete certificate.  If there's
> additional data you need to retrieve after the load, we might be able
> to store it in addition to the data we already save from the
> certificate.
> 
> James

Re: IMA: Data included in the key measurement

[James Bottomley]

On Fri, 2019-11-22 at 09:39 -0800, Lakshmi Ramasubramanian wrote:
> On 11/22/19 8:17 AM, James Bottomley wrote:
> 
> Thanks for the info James. I'll investigate further.

It strikes me that for attestation purposes, the hash of the
TBSCertificate, which is the thing that the issuer signs so we have it
anyway (well modulo us wanting a different hash algorithm), is a
complete and unique identifier for the certificate; can't we just use
that ... and perhaps we should add it to the ids stored in the key
payload[2]?

James

Re: IMA: Data included in the key measurement

[Lakshmi Ramasubramanian]

On 11/22/19 8:17 AM, James Bottomley wrote:

> If you measure at time of insertion you should be able to measure the
> entire key because it's inserted as a complete certificate.  If there's
> additional data you need to retrieve after the load, we might be able
> to store it in addition to the data we already save from the
> certificate.
> 
> James

You are right James - at the time of insertion the complete certificate 
can be measured. Thanks for the information.

I will update my patch set to include the certificate data in key 
measurement. Please let me know if you have any comments\concerns.

Please see below for details:

In the file "security/keys/key.c" =>
key_ref_t key_create_or_update(key_ref_t keyring_ref,
			       const char *type,
			       const char *description,
			       const void *payload,
			       size_t plen,
			       key_perm_t perm,
			       unsigned long flags)

In the key measurement, instead of just the "public key", I included the 
buffer pointed to by the "payload" parameter (buffer of size "plen" 
bytes) in the call to key_create_or_update(). It is the entire certificate.

thanks,
  -lakshmi

Please see the sequence of commands below to import a certificate (in 
DER format) to ".ima" keyring and regenerate the certificate from the 
IMA measurement log.

****** Import a DER certificate to .ima keyring ******

root@nramas:/home/nramas# keyctl show %:.ima
Keyring
   75295183 ---lswrv      0     0  keyring: .ima

root@nramas:/home/nramas# evmctl import x509_ima.der 75295183
118886017

root@nramas:/home/nramas# keyctl show %:.ima
Keyring
   75295183 ---lswrv      0     0  keyring: .ima
  118886017 --als--v      0     0   \_ asymmetric: hostname: whoami 
signing key: 052dd247dc3c36d6d60675fe7ae869790be56171

****** View the IMA measurement log ******

root@nramas:/home/nramas# cat 
/sys/kernel/security/integrity/ima/ascii_runtime_measurements
10 faf3dd532114feed8b8215eb7b5d8c3107d5e702 ima-buf 
sha256:ac8bd67bdaded63be9231c495585fd88edce0812d9b677e1e1e219e2dd3bcd60 
.ima 
30820286308201efa00302010202145be0234ff3adf050349b33b89465a6aab6e339f7300d06092a864886f70d01010b050030503111300f060355040a0c08686f73746e616d65311b301906035504030c1277686f616d69207369676e696e67206b6579311e301c06092a864886f70d010901160f77686f616d6940686f73746e616d65301e170d3139303832323032323930325a170d3230303832313032323930325a30503111300f060355040a0c08686f73746e616d65311b301906035504030c1277686f616d69207369676e696e67206b6579311e301c06092a864886f70d010901160f77686f616d6940686f73746e616d6530819f300d06092a864886f70d010101050003818d0030818902818100ee96b264072a42888f78a2f9b8198467a3ad97d126f3d1cc1c24d23e7185cc743b04d4a54254ca16e1e11ed4450deb98b1f7bb4288424570fabcfc6d5aa93a2a14fa2b7835ac877cfea761e5ff414c6ee274eff26f8bd6c484312e56619299acf0dbd224b87c3883b66a9393d21af8962458663b0ac1706c63773cd50e8236270203010001a35d305b300c0603551d130101ff04023000300b0603551d0f040403020780301d0603551d0e04160414052dd247dc3c36d6d60675fe7ae869790be56171301f0603551d23041830168014e36710f0834c973ed94a186fbcd22375b45e2454300d06092a864886f70d01010b050003818100b12faeff1e0e390cfd5eb7140af3b7a653cb49c6ab0a23be24c035331d7600c8f758f9df7fdfc5eeb6fec35859203eca0e4f01f9a79a58be630947cb959a52d3f2de96f210d49247c33a6226dc2a52ee541069ed3c621f8767fd36a061e9a61adb5d1dd34499d99a1ce6baa496b4f5e2268bfc52c3eea4a6b7b5181f08524aee

****** Regenerate the certificate from IMA measurement log ******

root@nramas:/home/nramas# cat 
/sys/kernel/security/ima/ascii_runtime_measurements | grep " .ima" | cut 
-d' ' -f 6 | xxd -r -p > ima-cert.der

root@nramas:/home/nramas# openssl x509 -in ima-cert.der -inform DER 
-text -noout
Certificate:
     Data:
         Version: 3 (0x2)
         Serial Number:
             5b:e0:23:4f:f3:ad:f0:50:34:9b:33:b8:94:65:a6:aa:b6:e3:39:f7
         Signature Algorithm: sha256WithRSAEncryption
         Issuer: O = hostname, CN = whoami signing key, emailAddress = 
whoami@hostname
         Validity
             Not Before: Aug 22 02:29:02 2019 GMT
             Not After : Aug 21 02:29:02 2020 GMT
         Subject: O = hostname, CN = whoami signing key, emailAddress = 
whoami@hostname
         Subject Public Key Info:
             Public Key Algorithm: rsaEncryption
                 RSA Public-Key: (1024 bit)
                 Modulus:
                     00:ee:96:b2:64:07:2a:42:88:8f:78:a2:f9:b8:19:
                     84:67:a3:ad:97:d1:26:f3:d1:cc:1c:24:d2:3e:71:
                     85:cc:74:3b:04:d4:a5:42:54:ca:16:e1:e1:1e:d4:
                     45:0d:eb:98:b1:f7:bb:42:88:42:45:70:fa:bc:fc:
                     6d:5a:a9:3a:2a:14:fa:2b:78:35:ac:87:7c:fe:a7:
                     61:e5:ff:41:4c:6e:e2:74:ef:f2:6f:8b:d6:c4:84:
                     31:2e:56:61:92:99:ac:f0:db:d2:24:b8:7c:38:83:
                     b6:6a:93:93:d2:1a:f8:96:24:58:66:3b:0a:c1:70:
                     6c:63:77:3c:d5:0e:82:36:27
                 Exponent: 65537 (0x10001)
         X509v3 extensions:
             X509v3 Basic Constraints: critical
                 CA:FALSE
             X509v3 Key Usage:
                 Digital Signature
             X509v3 Subject Key Identifier:
                 05:2D:D2:47:DC:3C:36:D6:D6:06:75:FE:7A:E8:69:79:0B:E5:61:71
             X509v3 Authority Key Identifier:
 
keyid:E3:67:10:F0:83:4C:97:3E:D9:4A:18:6F:BC:D2:23:75:B4:5E:24:54

     Signature Algorithm: sha256WithRSAEncryption
          b1:2f:ae:ff:1e:0e:39:0c:fd:5e:b7:14:0a:f3:b7:a6:53:cb:
          49:c6:ab:0a:23:be:24:c0:35:33:1d:76:00:c8:f7:58:f9:df:
          7f:df:c5:ee:b6:fe:c3:58:59:20:3e:ca:0e:4f:01:f9:a7:9a:
          58:be:63:09:47:cb:95:9a:52:d3:f2:de:96:f2:10:d4:92:47:
          c3:3a:62:26:dc:2a:52:ee:54:10:69:ed:3c:62:1f:87:67:fd:
          36:a0:61:e9:a6:1a:db:5d:1d:d3:44:99:d9:9a:1c:e6:ba:a4:
          96:b4:f5:e2:26:8b:fc:52:c3:ee:a4:a6:b7:b5:18:1f:08:52:
          4a:ee

Re: IMA: Data included in the key measurement

[Mimi Zohar]

On Mon, 2019-11-25 at 09:33 -0800, Lakshmi Ramasubramanian wrote:
> On 11/22/19 8:17 AM, James Bottomley wrote:
> 
> > If you measure at time of insertion you should be able to measure the
> > entire key because it's inserted as a complete certificate.  If there's
> > additional data you need to retrieve after the load, we might be able
> > to store it in addition to the data we already save from the
> > certificate.
> > 
> > James
> 
> You are right James - at the time of insertion the complete certificate 
> can be measured. Thanks for the information.
> 
> I will update my patch set to include the certificate data in key 
> measurement. Please let me know if you have any comments\concerns.
> 
> Please see below for details:
> 
> In the file "security/keys/key.c" =>
> key_ref_t key_create_or_update(key_ref_t keyring_ref,
> 			       const char *type,
> 			       const char *description,
> 			       const void *payload,
> 			       size_t plen,
> 			       key_perm_t perm,
> 			       unsigned long flags)
> 
> In the key measurement, instead of just the "public key", I included the 
> buffer pointed to by the "payload" parameter (buffer of size "plen" 
> bytes) in the call to key_create_or_update(). It is the entire certificate.
> 
> thanks,
>   -lakshmi
> 
> Please see the sequence of commands below to import a certificate (in 
> DER format) to ".ima" keyring and regenerate the certificate from the 
> IMA measurement log.
> 
> ****** Import a DER certificate to .ima keyring ******
> 
> root@nramas:/home/nramas# keyctl show %:.ima
> Keyring
>    75295183 ---lswrv      0     0  keyring: .ima
> 
> root@nramas:/home/nramas# evmctl import x509_ima.der 75295183
> 118886017
> 
> root@nramas:/home/nramas# keyctl show %:.ima
> Keyring
>    75295183 ---lswrv      0     0  keyring: .ima
>   118886017 --als--v      0     0   \_ asymmetric: hostname: whoami 
> signing key: 052dd247dc3c36d6d60675fe7ae869790be56171
> 
> ****** View the IMA measurement log ******
> 
> root@nramas:/home/nramas# cat 
> /sys/kernel/security/integrity/ima/ascii_runtime_measurements
> 10 faf3dd532114feed8b8215eb7b5d8c3107d5e702 ima-buf 
> sha256:ac8bd67bdaded63be9231c495585fd88edce0812d9b677e1e1e219e2dd3bcd60 
> .ima 
> 30820286308201efa00302010202145be0234ff3adf050349b33b89465a6aab6e339f7300d06092a864886f70d01010b050030503111300f060355040a0c08686f73746e616d65311b301906035504030c1277686f616d69207369676e696e67206b6579311e301c06092a864886f70d010901160f77686f616d6940686f73746e616d65301e170d3139303832323032323930325a170d3230303832313032323930325a30503111300f060355040a0c08686f73746e616d65311b301906035504030c1277686f616d69207369676e696e67206b6579311e301c06092a864886f70d010901160f77686f616d6940686f73746e616d6530819f300d06092a864886f70d010101050003818d0030818902818100ee96b264072a42888f78a2f9b8198467a3ad97d126f3d1cc1c24d23e7185cc743b04d4a54254ca16e1e11ed4450deb98b1f7bb4288424570fabcfc6d5aa93a2a14fa2b7835ac877cfea761e5ff414c6ee274eff26f8bd6c484312e56619299acf0dbd224b87c3883b66a9393d21af8962458663b0ac1706c63773cd50e8236270203010001a35d305b300c0603551d130101ff04023000300b0603551d0f040403020780301d0603551d0e04160414052dd247dc3c36d6d60675fe7ae869790be56171301f0603551d23041830168014e36710f0834c973ed94a186fbcd223
 75b45e2454300d06092a864886f70d01010b050003818100b12faeff1e0e390cfd5eb7140af3b7a653cb49c6ab0a23be24c035331d7600c8f758f9df7fdfc5eeb6fec35859203eca0e4f01f9a79a58be630947cb959a52d3f2de96f210d49247c33a6226dc2a52ee541069ed3c621f8767fd36a061e9a61adb5d1dd34499d99a1ce6baa496b4f5e2268bfc52c3eea4a6b7b5181f08524aee
> 
> ****** Regenerate the certificate from IMA measurement log ******
> 
> root@nramas:/home/nramas# cat 
> /sys/kernel/security/ima/ascii_runtime_measurements | grep " .ima" | cut 
> -d' ' -f 6 | xxd -r -p > ima-cert.der
> 
> root@nramas:/home/nramas# openssl x509 -in ima-cert.der -inform DER 
> -text -noout

Only the commands to validate the "key" measurement and extract the
certificate need to be included in the patch description.  You could
combine these two commands using "tee".  One would save the
certificate, while the other would calculate the template data hash.
 Providing the openssl command to display the saved certificate is
optional.

Mimi

Re: IMA: Data included in the key measurement

[Lakshmi Ramasubramanian]

On 11/25/19 10:14 AM, Mimi Zohar wrote:

> 
> Only the commands to validate the "key" measurement and extract the
> certificate need to be included in the patch description.  You could
> combine these two commands using "tee".  One would save the
> certificate, while the other would calculate the template data hash.
>   Providing the openssl command to display the saved certificate is
> optional.
> 
> Mimi
> 

Sure - I just wanted to provide more details in this mail to get 
feedback\comments on the measurement data included.

Will include only the info you've listed above when I send the patches.

thanks,
  -lakshmi

Re: IMA: Data included in the key measurement

[Mimi Zohar]

[Cc'ing David Howells, James Bottomley]

On Thu, 2019-11-21 at 08:17 -0800, Lakshmi Ramasubramanian wrote:
> Hi Mimi,
> 
>  >>> everything needed for verifying a signature is included in
>  >>> the key measurement.
> 
> Regarding the requirement you had stated above, I would like some 
> clarification.
> 
> When I started this change to measure keys through IMA, the use case we 
> had in mind was enabling an attestation service, for instance, to verify 
> if the client has only known good (trusted) keys - for example, in 
> keyrings such as ".builtin_trusted_keys", ".ima", etc.
> 
> On the client IMA verifies the signature of system binaries using keys 
> in the IMA keyring. And, if the config namely 
> CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY is enabled, 
> only keys signed by a built-in trusted key can be added to the IMA keyring.
> 
> An attestation service can keep a list of public keys of "known good 
> (trusted)" keys for various keyrings, and verify against the measurement 
> data provided by the client.

Right, that's true, assuming the attestation server is aware of all
possible public keys, which was the original reason you provided for
measuring the keys on the builtin trusted keyring, not the IMA
keyring.

> 
> To achieve the above we decided to include only the public key in the 
> key measurement buffer.
> 
> I would like to know what benefit we'd get by including "everything 
> needed for verifying a signature in the key measurement"?
> 
>  From testing point of view, if we have the certificate (like the .DER 
> file), we can validate the key measurement data in the IMA log.
> 
> Do you see a need to include more data or the entire cert for the 
> product code?

Your code has come a long way, since you first started.  Please don't
go back to what you originally intended/wanted to do.  Measuring keys
just on the builtin trusted keyring, as you did, was a non starter.
 It would never have been accepted upstream by me and doubtfully by
David.  The current measuring of keys is more generic and properly
isolated to IMA.  This is what I would have expected from the very
beginning.  Only now are the patches at the point, where there is
something to even discuss.

Now that you understand what patches should look like in order to be upstreamed, please look outside of your use-case scenario and think about the bigger picture.  Remember changing the "key" measurement in the future would cause a userspace regression.  So we need to fully understand what is needed, *before* it is upstreamed.  Yes, changing what is measured might change the IMA hook placement.

James Bottomley already started the discussion in this thread...

Mimi