Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting extended attributes in postprocess-script? #412

Closed
copumpkin opened this issue Jul 27, 2016 · 10 comments
Closed

Setting extended attributes in postprocess-script? #412

copumpkin opened this issue Jul 27, 2016 · 10 comments
Labels

Comments

@copumpkin
Copy link
Contributor

I'm trying to set some extended attributes in my postprocess-script with setfattr and am noticing that they don't seem to make it into the final tree. In my postprocess-script I both setfattr and immediately afterwards getfattr, so I can see that the extended attributes get set properly, but somehow they disappear later in the process.

Is that expected to work? I see in the ostree documentation here that the repository format preserves extended attributes, but I'm not sure if rpm-ostree is doing something to remove my extended attributes.

If this is unexpected behavior, I can put together a minimal repro.

@jlebon
Copy link
Member

jlebon commented Jul 27, 2016

We only pick up security.capability in rpm-ostree, though ostree does support recording all the attributes. See: https://github.com/projectatomic/rpm-ostree/blob/89f7e481b4025cbfac8c68a3b0dcd0a0f5c30ebe/src/libpriv/rpmostree-postprocess.c#L1601

I'll have to look into what the rationale was behind only allowing that (or maybe @cgwalters knows offhand). Might be reasonable to expand it at least for user attrs (would that cover your use case?).

@copumpkin
Copy link
Contributor Author

Thanks for the quick reply! Yeah, the one I need is in user.. Even if you decide to maintain a restriction, it seems worthwhile printing out a warning if you encounter extended attributes that you discard, to avoid future confusion.

Also, I'm a little confused because another extended attribute seems to have made its way into my target tree, but it's security.selinux, not security.capability. Is that something that rpm-ostree is adding itself?

@copumpkin
Copy link
Contributor Author

Just to confirm, I patched my attribute into that array and my issue went away. Would be nice to have a less hacky solution to this upstream though 😄

@cgwalters
Copy link
Member

Yeah, we do selinux labeling natively via ostree_repo_commit_modifier_set_sepolicy().

So one thing to note here is what you're trying to do conflicts with the plan I have in ostreedev/ostree#369 (comment) - basically O_OBJECT or equivalent would seal the system. xattr space but not user..

Can you say what the user. attr is and what consumes it? user.xdg.origin.url like that curl --xattr writes?

@copumpkin
Copy link
Contributor Author

It's just user.pax.flags, from https://grsecurity.net/. I could use paxctl to modify the ELF files in place, but this seemed cleaner and PaX supports both.

@cgwalters
Copy link
Member

Sure, would take a patch to add that. Although...IMO they should really change to at least also honor the system. namespace, since that would blend best with my plans for immutability of the system. namespace with ostree.

@copumpkin
Copy link
Contributor Author

I'll raise it to the PaX/grsecurity folks, thanks 😄

By patch, you mean something that whitelists exactly that attribute? I can do that. Or are you talking more broadly about whitelisting the namespace?

What's the goal of that check/restriction, out of curiosity? I'm not really grasping your xattrs plan from that ostree ticket link you posted above.

@cgwalters
Copy link
Member

Something like this, if you want to test it:

diff --git a/src/libpriv/rpmostree-postprocess.c b/src/libpriv/rpmostree-postprocess.c
index 0f1b82e..913bbab 100644
--- a/src/libpriv/rpmostree-postprocess.c
+++ b/src/libpriv/rpmostree-postprocess.c
@@ -1711,8 +1711,10 @@ read_xattrs_cb (OstreeRepo     *repo,
                 gpointer        user_data)
 {
   int rootfs_fd = GPOINTER_TO_INT (user_data);
-  /* Hardcoded at the moment, we're only taking file caps */
-  static const char *accepted_xattrs[] = { "security.capability" };
+  static const char *accepted_xattrs[] =
+    { "security.capability", /* https://lwn.net/Articles/211883/ */
+      "user.pax.flags" /* https://github.com/projectatomic/rpm-ostree/issues/412 */
+    };
   guint i;
   g_autoptr(GVariant) existing_xattrs = NULL;
   gs_free_variant_iter GVariantIter *viter = NULL;

@copumpkin
Copy link
Contributor Author

Ah, that's exactly what I'm using today, minus the comments 😄 so it's already tested and works fine!

@cgwalters
Copy link
Member

The TL;DR on my thoughts on xattrs is I want OSTree to "seal" files such that:

  • The contents are immutable
  • But it's possible to make hardlinks
  • At least the security. namespace is immutable
  • But it's possible to change the user. and trusted. xattr space...this might be used by backup tools or mime types, see how curl injects user.xdg.origin.url
    (Alternatively we could define sealing to mean any attrs set at seal time are immutable, in which case grsecurity wouldn't need to change anything)

cgwalters added a commit to cgwalters/rpm-ostree that referenced this issue Jul 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants