stage0/cas: apply xattr attributes #3305
Conversation
pkg/tar/tar.go
Outdated
|
|
||
| if typ == tar.TypeReg || typ == tar.TypeRegA { | ||
| for k, v := range hdr.Xattrs { | ||
| err := syscall.Setxattr(p, k, []byte(v), 0) |
There was a problem hiding this comment.
I am a little worried, that I am simply setting these xattr fields 1:1. Are there any security concerns?
/cc @lucab
There was a problem hiding this comment.
Not specific cases off the top of my head, but I can imagine several misuses. I'd say it would be safer to perform some prefix-whitelisting on k here, and just allow security.capability and user prefixes for the moment.
099d3f7 to
2e7819b
Compare
lucab
left a comment
There was a problem hiding this comment.
Just a simple naming nit, otherwise code and tests look fine. Feel free to self-merge once renaming is done.
pkg/tar/tar.go
Outdated
| return []syscall.Timespec{fileutil.TimeToTimespec(hdr.AccessTime), fileutil.TimeToTimespec(hdr.ModTime)} | ||
| } | ||
|
|
||
| func forbiddenXattr(xattr string) bool { |
There was a problem hiding this comment.
As we are doing whitelisting, this function name (and logic) is a bit misleading. Can we turn it into isXattrAllowed() and negate the returned bool?
There was a problem hiding this comment.
sounds good, I was back-and-forth on this one actually ;)
Currently the xattr field is ignored, when untarring files. This fixes it. Fixes rkt#3179
2e7819b to
4a1f187
Compare
Currently the xattr field is ignored, when untarring files. This fixes
it.
Fixes #3179