commit: Optionally translate ima xattr

[puiterwijk]

The security.ima extended attribute can only be written by root, but
there is the user.ima attribute that can be set by e.g. ima-evm-utils.
Allowing translation from user.ima to security.ima means that the tree
can be prepared with user.ima attributes, and after translation, stored
as security.ima in the tree.

This would result in a deployed security.ima attribute without needing
to run the tree compose as root.

Signed-off-by: Patrick Uiterwijk [email protected]

[openshift-ci-robot]

Hi @puiterwijk. Thanks for your PR.

I'm waiting for a ostreedev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[wmanley]

/ok-to-test

[jlebon]

This would result in a deployed security.ima attribute without needing
to run the tree compose as root.

Note you can also do this also via the xattr callback. For example this is how rpm-ostree sets file caps when importing RPMs: https://github.com/coreos/rpm-ostree/blob/e88a736e55eeb91ced5619c4ee9119d4cf705068/src/libpriv/rpmostree-importer.cxx#L818-L833.

Though if user.ima is an established convention, it might make sense indeed to have support for it in libostree.

[cgwalters]

Though if user.ima is an established convention, it might make sense indeed to have support for it in libostree.

Yeah I am OK with this, but I'd also like to see high level support in this in rpm-ostree i.e. declarative in the manifest something like:

ima:
  key: /path/to/rsa.priv

(and support for passing the key in the kernel keyring or via file descriptor too)

It looks like this would require today forking off evmctl for each unsigned file; a bit like the state of fsverity before it gained a shared library, but eh.

Anyways WDYT about just generalizing this so that e.g. all user.security xattrs are translated to security.? Or maybe it's:
ostree commit --xattr-map=user.ima=security.ima? That wouldn't require a new commit modifier flag (i.e. new ostree API) and could simply be done in the CLI commit modifier path, and easily replicated in rpm-ostree.

[cgwalters]

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, puiterwijk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [cgwalters]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[puiterwijk]

@cgwalters There's actually a libimaevm, which has sign_hash. I have a draft patch for ostree to support that, but then with arguments to ostree commit.

[ashcrow]

The travis tests failures look to show a lot of infra flakes (can't reach a gnome repo).

For jenkins CI I see:

--- FAIL: ext.ostree.destructive-rs.transactionality (134.80s)

        harness.go:709: kolet failed: : kolet run-test-unit failed: Error: Unit kola-runext.service exited with code 1

2021-01-29T21:41:39Z cli: Unit kola-runext.service exited with code 1: Process exited with status 1

I do see the above jenkins failure in a few other PRs such as:

• README.md: Also link apt2ostree #2273
• Bootloaders: Support for U-boot in extlinux.conf compatibility mode #2256
• prepare-root: Set up sysroot readonly in initramfs #2187

which may indicate this is a known failing test and not directly related to this change.

@cgwalters @lucab @travier @kelvinfan001 would one of you mind taking a look and verifying?

[ashcrow]

Is there a way to force a retest?

[cgwalters]

For Jenkins it's logging in and clicking retry; done now.
Travis requires a force push AFAIK; I hope to drop Travis soon probably in favor of using Prow, or maybe GH actions.

But let's reach a conclusion on the high level interface; I think I'd lean most towards

ostree commit --xattr-map=user.ima=security.ima

and we could also just hardcode doing that in rpm-ostree by default probably. (ostree is intended to be a flexible shared library, opinionated where necessary; rpm-ostree is more opinionated)

[cgwalters]

As for why the transactionality test is failing...well it's almost certainly not related to this PR but it's concerning. It's possible it's related to a newer rpm-ostree or something else.

I still really want to implement "reverse dependency testing" i.e. have FCOS CI run the ostree tests, including this.

[lucab]

CI failure seems to be a race/regression in rpm-ostree, tracked at coreos/rpm-ostree#2531.

For the short term, I managed to get a ✔️ by adding some retries around the racing logic in #2276.

[ashcrow]

Bump for continued conversation 😃

[puiterwijk]

Thanks, @ashcrow for the ping. Given that @cgwalters wants the generic translation, I'm now instead adding this to rpm-ostree as something that's read from the treefile instead.
I'll open that a PR against rpm-ostree later today.

[puiterwijk]

I opened the rpm-ostree PR (as WIP since I still need to finish tests) at coreos/rpm-ostree#2563