IMA policy enhancement proposal

[THS-on]

Accompanies #70

[lukehinds]

Thanks for getting this started, I think we need to do a fair amount of planning around this as if we need to extend at some point in the future we can do so without being fixed to a certain manifest layout. I am imagining having manifest agility so allowlists can be consumed from different providers. We did something like this in rekor with what we call custom types, this way we can support multiple crypt algs and different manifest layouts and even formats.

I will sketch some ideas out in a doc and share them.

We also need to think how this marries with the signature verification and the multi allowlist work.

[THS-on]

@lukehinds this proposal only is the unification of the settings into a single policy and does not contain anything more, but I agree if we want to consume other formats directly we should plan ahead to make your life easier in the future.

[THS-on]

@mpeters @lukehinds @maugustosilva I think this proposal is ready.

For the actual signature format should be either part of #65 or a new enhancement.

[aplanas]

Incidentally I am trying to push this in createrepo_c: rpm-software-management/createrepo_c#316

One problem that I found is that when we try to generate the list of IMA hashes we need to: (1) access the monitored machine to calculate the hashes or (2) download all the RPMs / Debs from the repositories and fetch from there (maybe in the header) the hashed data.

The second option has the advantage that we know for sure that the hash is correct (it is signed and we can validate it), but is very slow and hard synchronize when there is an update.

For that, the patch in createrepo_c will add a new metadata file that contains all the hashes of the files that can be installed. This method is again secure (signed), and is also friendly to updates (http etag).

[lukehinds]

sorry for radio silence, am reviewing now

[lukehinds]

This looks good to me, with a caveat we can evolve the schema over time (things liked attached sigs which we discussed in this issue).

[THS-on]

@lukehinds thanks for the review.

This looks good to me, with a caveat we can evolve the schema over time (things liked attached sigs which we discussed in this issue).

Yes. As already mentioned this is just pulling all the IMA options into one single JSON object. The signature in this object is for IMA (ima-sig) validation and not for validating the JSON object. The proposal for adding signature validation for the IMA policy will be finalized once keylime/keylime#994 is merged and this proposal is implemented.

[mpeters]

Would it be possible to have backwards compatability, at least for a couple of versions? It's fine if it's deprecated and issues a warning.

[THS-on]

Kinda, but not really. Ideally we want to make the tenant simpler to use and not more complicated. Removing and providing a tool is easier than trying to build backwards compatability into the tenant long term.

When I asked there was still the need for the flat file format (I think it came from @maugustosilva) and with that approach we can support as long as needed, but no longer as a main part of the tenant.

[mpeters]

My main concern is that keylime_tenant for most people is THE interface for keylime and making backwards incompatible changes without a deprecation cycle is going to cause more and more problems as adoption increases. Especially after it starts landing in major OS distros soon. Maybe that's not an issue with this change, but I'm concerned about the precedent it sets.

Maybe a compromise is to have the tenant accept all of the old arguments and do one of the following:

1. Issue a deprecation warning and use the new tool in the background to do the conversion.
2. Issue an error message and helpfully point the user at the new conversion tool

In my mind the 1st is a superior user experience, but I can probably be convinced that the 2nd is better to keep the code cleaner.

[THS-on]

I think we should think about how many people are actually using the IMA feature today and make our decision based on that. I think that for new users the current tenant commandline interface is not really usable. It has way too many options, which are not really used. I'm fine with implementing the second idea.

When Keylime ships in e.g. RHEL it will be probably frozen on one mayor version right? So then it will be always an upgrade issue between the last shipped one and the current master.

[lukehinds]

Just some json schema validation notes:

"verification-keys": {
            "type": "array",
            "title": "Public keys to verify IMA attached signatures",
            "items": "string"
        },

Expected property to contain an array or schema (Object or Boolean), but found a String.

"verification-keys": {
            "type": "array",
            "title": "Public keys to verify IMA attached signatures",
            "items": { 
              "key1": ["abcd"],
              "key2": ["abcd"]
            }
        },

[lukehinds]

I would like to offer up the following for consideration. Understand this may need another enhancement, but would like any code to have in mind an extendable interface. This would allow signing of the body / envelope for attached sigs:

{
    "signatures": [
        {
         "keyid": "{hex-encoded-digest-of-pubkey-or-x509-pem-certificate}",
         "sig": "{hex-encoded-signature}"
        }
    ],
    "signed": {
        "meta": {
        "version": 872
        },
        "release": 774.75,
        "hashes": {
        ",'Sr": []
        },
        "excludes": [],
        "keyrings": {
        "}": "ABCDEFGHIJK"
        },
        "ima": {
        "ignored_keyrings": [],
        "log_hash_alg": "ABCDEFGHIJKLMNOPQRS"
        },
        "ima-buf": {
        "": "ABCDEFGH"
        },
        "verification-keys": []
    }
}

hex, could also be b64

[THS-on]

@lukehinds the schema had an error. This should now be fixed.

For the signatures: I would move this discussion to a new proposal, because this is outside the scope of this one.

[lukehinds]

@lukehinds the schema had an error. This should now be fixed.

For the signatures: I would move this discussion to a new proposal, because this is outside the scope of this one.

understood, but I want to make sure any work we do here (as a result of this enhancement) is not going to make it problematic (api breaking) to implement an attached signature envelope as a follow up.

I will implement the enhancement asap, and we can then look to introduce both at the same time, rather than hit a non-backwards compat changen twice over.

[THS-on]

@mbestavros anything to add from your side? If not I would merge it.

[mbestavros]

This looks great! Will definitely be a significant usability improvement, and I agree that it will be best to introduce all these improvements as one large breaking change to simplify how things are handled on tenant and verifier.

A few notes I had once I'd read through it:

• This new format presents an opportunity to strip out a bunch of the allowlist processing steps on the tenant and verifier - ideally this becomes something we can store unmodified on the verifier (would make signature validation and the like possible). Does the current proposal have a 1:1 mapping in terms of fields used in the latest versions of Keylime allowlists? (i.e. will we need to do any post-processing? I suspect no, but just to confirm.) I notice a version field, which is good.
• The IMA policy migration tool will be necessary, and a lot of the ideas are good. Legacy signature validation caught my eye - one potential expansion on that idea could be a --sign option or similar, where the migration tool would also handle keygen and signing of the converted IMA policy and spit out associated signing materials for use with Keylime.
• This proposal assumes IMA policies will still be stored alongside individual agent data, which will likely not be the case for much longer - when the Allowlists API is complete, IMA policies will live in their own separate table. It's a technical detail, but one worth keeping in mind.

Most importantly, though, I agree with @lukehinds that we need to think carefully about how we want to approach signing before implementing the new policy format. We have an opportunity to future-proof that mechanism, and we should if we're making a breaking API change anyway. I don't see why that concern should block this enhancement from being merged, but I also think #79 should be merged before attempting implementation.

[THS-on]

Does the current proposal have a 1:1 mapping in terms of fields used in the latest versions of Keylime allowlists?

Yes, there is no conversion required.

The IMA policy migration tool will be necessary, and a lot of the ideas are good. Legacy signature validation caught my eye - one potential expansion on that idea could be a --sign option or similar, where the migration tool would also handle keygen and signing of the converted IMA policy and spit out associated signing materials for use with Keylime.

Maybe, but signing using something like openssl is pretty straightforward. We should add documentation on how to do that, but I would like to keep the conversion tool as simple as possible because we are going to remove it at some point.

This proposal assumes IMA policies will still be stored alongside individual agent data, which will likely not be the case for much longer - when the Allowlists API is complete, IMA policies will live in their own separate table. It's a technical detail, but one worth keeping in mind.

Yes, thanks for pointing that out. Fortunately this change does not really affect this proposal because the IMA policy format does not require knowledge about the agent. The only thing is probably the some DB column names might change.

Most importantly, though, I agree with @lukehinds that we need to think carefully about how we want to approach signing before implementing the new policy format. We have an opportunity to future-proof that mechanism, and we should if we're making a breaking API change anyway. I don't see why that concern should block this enhancement from being merged, but I also think #79 should be merged before attempting implementation.

Fully agree. It makes sense to implement both proposals in one go.

[mbestavros]

I didn't realize I hadn't explicitly approved it, but I think this is ready to go!