Re: keyring timestamps

[Mimi Zohar]

On Tue, 2015-12-01 at 21:58 +0200, Petko Manolov wrote:

> First off, this is not a real patch rather than my idea in a C form.  I feel 
> uncertain about a few points:
> 
>   0) does keyrings keep a timestamp when created or last updated?  David?
> 
>   1) is jiffies(_64) the best thing to use for timestamping?  
>  sched_clock() is known to stop at suspend/sleep.

"struct key" defines a couple of time stamps, but none of them are what
we need.

union {
time_t  expiry; /* time at which key
expires (or 0) */
time_t  revoked_at; /* time at which key was
revoked */
};
time_t  last_used_at;   /* last time used for
LRU keyring discard */

>   2) the code below is not optimal - it removes the node from the RB tree 
>  and then walks it again to find the right place.  Mimi, any 
>  objections to restructure integrity_inode_get() for speed when 
>  dealing with timestamps?

Instead of removing the iint's from the RB tree, how about just clearing
the cached appraised info in the iint.   Something like: 
iint->flags &= ~(IMA_APPRAISED | IMA_APPRAISED_SUBMASK);

> 0) is crucial.  If there is no such thing as "time of the last update" for 
> keyrings i guess we'll either have to implement it or use another mechanism 
> to 
> get similar result.

For this usecase, we need a timestamp for the last time a key was added
to the keyring.

Mimi


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: keyring timestamps

[Petko Manolov]

On December 1, 2015 11:03:32 PM GMT+02:00, David Howells  
wrote:
>Petko Manolov  wrote:
>
>>  0) does keyrings keep a timestamp when created or last updated? 
>David?
>
>No.
>
>> 0) is crucial.  If there is no such thing as "time of the last
>update" for
>> keyrings i guess we'll either have to implement it or use another
>mechanism
>> to get similar result.
>
>You haven't said why you want it?  Update what?
>
>David

Once a file has been hashed, IMA keeps this value in a cache in order to avoid 
doing the same thing in the future.  This used to work properly until we 
introduced a blacklist keyring.  It is now possible the key that signed a file 
with already calculated and cached hash to land in the said keyring.

It would be incorrect behavior to allow the requested operation.  One way to 
deal with this situation is to invalidate the whole iint cache when a key 
enters the blacklist keyring.  Far from optimal.

Another way is to extend the iint structure with a timestamp field that is 
updated when the hash is calculated.  We could then compare this time against 
the last time the blacklist keyring was updated.  If newer then all is well. If 
the hash was calculated earlier we invalidate this entry and do all checks anew.

The third option may be to compare the IDs of the blacklisted key(s) against 
the one that signed the file and base the decision on this.  This isn't a bad 
one either but i am not sure about key ID collisions.


Petko


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: keyring timestamps

[David Howells]

Petko Manolov  wrote:

>   0) does keyrings keep a timestamp when created or last updated?  David?

No.

> 0) is crucial.  If there is no such thing as "time of the last update" for
> keyrings i guess we'll either have to implement it or use another mechanism
> to get similar result.

You haven't said why you want it?  Update what?

David
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: keyring timestamps

[Mark D. Baushke]

Regarding keyID collisions...

IMA version 2 format uses the low-order 32-bits of a SHA1 hash of the
ASN.1 encoded public key and exponent.

Collisions of this keyID are possible in two ways:

  a) the public key + exponent manages to have a collision on the
 low-order 32-bits of the hash,

  b) someone has managed to generate the same public key material
 in a separate certificate.

I would guess that item a is not very likely, but is it certainly
possible in theory. We have seen OpenPGP keyids that collide but are
actually two separate public/private key pairs.

For item b, some users have been known to generate a single CSR and
submit it to multiple signing authorities (Intermediate Cross-Signed
Certificates), or re-use a public key when a certificate expires.

URLs of examples of the item b collisions:

http://security.stackexchange.com/questions/6926/multiple-cas-signing-a-single-cert-csr
https://en.wikipedia.org/wiki/X.509 (section 3 cross-certification)

http://social.technet.microsoft.com/wiki/contents/articles/1102.how-to-changeextend-the-expiration-date-of-certificates-that-are-issued-by-a-windows-server-2008-or-a-windows-server-2003-certificate-authority.aspx

Playing with a blacklist or expired certificates implies being able to
explicitly tie a given IMA keyID back to the certificate it uses. This
could be an issue in the case of a cross-signed certificate where one of
parent certifictes in the chain has been compromised and put in the
blacklist while the other cross-signed hierarchy remains intact.

-- Mark
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

keyring timestamps

[Petko Manolov]

On 15-12-01 13:40:05, Mimi Zohar wrote:
> On Tue, 2015-12-01 at 18:51 +0200, Petko Manolov wrote:
> > 
> > I'll also send you something resembling a patch about iint invalidation 
> > based on 
> > .ima_blacklist updates.  I've got a few questions.
> 
> Ok.  At some point, we really to take this back online.

Here we go.

First off, this is not a real patch rather than my idea in a C form.  I feel 
uncertain about a few points:

0) does keyrings keep a timestamp when created or last updated?  David?

1) is jiffies(_64) the best thing to use for timestamping?  
   sched_clock() is known to stop at suspend/sleep.

2) the code below is not optimal - it removes the node from the RB tree 
   and then walks it again to find the right place.  Mimi, any 
   objections to restructure integrity_inode_get() for speed when 
   dealing with timestamps?

0) is crucial.  If there is no such thing as "time of the last update" for 
keyrings i guess we'll either have to implement it or use another mechanism to 
get similar result.

---
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 2de9c82..a1c0062 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -96,15 +96,21 @@ struct integrity_iint_cache *integrity_inode_get(struct 
inode *inode)
struct integrity_iint_cache *iint, *test_iint;
 
iint = integrity_iint_find(inode);
-   if (iint)
+   if (iint && (iint->timestamp > blacklist_timestamp)) {
return iint;
+   } else {
+   write_lock(_iint_lock);
+   rb_erase(>rb_node, _iint_tree);
+   init_once(iint);
+   goto init;
+   }
 
iint = kmem_cache_alloc(iint_cache, GFP_NOFS);
if (!iint)
return NULL;
 
write_lock(_iint_lock);
-
+init:
p = _iint_tree.rb_node;
while (*p) {
parent = *p;
@@ -116,6 +122,7 @@ struct integrity_iint_cache *integrity_inode_get(struct 
inode *inode)
p = &(*p)->rb_right;
}
 
+   iint->timestamp = jiffies_64;
iint->inode = inode;
node = >rb_node;
inode->i_flags |= S_IMA;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 5efe2ec..2642bf8 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -105,6 +105,7 @@ struct integrity_iint_cache {
struct rb_node rb_node; /* rooted in integrity_iint_tree */
struct inode *inode;/* back pointer to inode in question */
u64 version;/* track inode changes */
+   u64 timestamp;  /* compare against blacklisted keys */
unsigned long flags;
enum integrity_status ima_file_status:4;
enum integrity_status ima_mmap_status:4;
---


Petko
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html