[Toybox] Integration of SMACK
Rob Landley
rob at landley.net
Wed Apr 29 22:15:46 PDT 2015
On 04/29/2015 01:58 PM, Rob Landley wrote:
>>> I don't see a getxattr() variant that lets us feed it a directory
>>> descriptor instead of using curdir() (apparently not many kernel
>>> developers ever actually use this syscall), but it does at least have
>>> fgetxattr() which can presumably be stacked on openat(). Or at least
>>> it could in a sane world, but this is security theatre code we're
>>> talking about here so "sane" is a big assumption: how do I know it
>>> isn't going to veto an O_RDONLY open of the file even before anybody
>>> tries to read data from it, so we can't actually get an inactive
>>> filehandle to this thing we can use to query extended attributes but
>>> not read file contents from...?
>
> In 2.6.39 the kernel guys added the O_PATH flag:
>
> http://man7.org/linux/man-pages/man2/open.2.html
>
> Which allows you to open a file without read permissions. (O_RDONLY is
> zero so you can't _not_ specify "I would like read permission on this
> file". So they had to add O_PATH to say "switch off the O_RDONLY bit
> even though there isn't one".)
>
> The point of this would be to let you open a chmod 000 file via openat()
> and then use functions like fgetxattr() on it. (And this eliminates some
> races because you can fstat() the filehandle and go "yup, this is a
> directory" before proceeding.)
>
> I'm leaning towards rewriting your ls patch to use openat() with O_PATH
> and use that to avoid allocating, traversing, and freeing full paths
> from cwd for every file.
>
> Of course I dunno if your security infrastructure is going to veto the
> open anyway. Still can't test it...
>
> Rob
I mentioned the redundant path creation and switching it out for openat(O_PATH)
and fgetxattr(). (If smack vetoes this open in ls, smack is probably going to
have problems with other programs using modern apis.)
The first -Z hunk used lgetxattr() but the second used getxattr(). So we
measured the length of the symlink attribute, then printed the file attribute?
I'm consistently using openat(O_PATH|O_NOFOLLOW) which should give me a
filehandle to the symlink...
We're testing whether getxattr() returns a length > SMACK_LABEL_LEN. If that
can happen, did it stomp stack data outside the buffer? If it can't happen,
why are we testing for it?
Query: we feed sizeof(zlen) to getxattr(), I.E. SMACK_LABEL_LEN+1. But we
write a nul terminator, so the getxattr() call isn't terminating the buffer,
so why are we feeding it a buffer size _including_ space for a null terminator?
The third -Z hunk is identical to the second -Z hunk except it's left
justified instead of right justified. Also your length is zlen-1 in hunk 3
and zlen without the -1 in hunk 2? Let's see... what you want is always one
space on the right, only a space on the left for -long, and yes the left/right
justification changes between -C and -l for no apparent reason but that's
what you expect so... (Note: sprintf %*s will left justify if fed a negative
length. Posix 2008 says so.)
Why are you typecasting zlen to (int)? Doesn't subtracting a ssize_t work?
The query is setting a length of 0 if getxattr() doesn't find any data,
but you're printing "?" and a space, meaning the column sizing logic circa
line 550 needs adjusting too. (totpad += totals[7]+!!totals[7])
Alright, here's a wild guess at a patch, which I can't test because
the tizen image I installed following the instructions on
https://wiki.tizen.org/wiki/Tizen_Standalone_Emulator hasn't got a native
toolchain.
index 740a4ee..b25048f 100644
--- a/lib/portability.h
+++ b/lib/portability.h
@@ -226,6 +226,10 @@ ssize_t getline(char **lineptr, size_t *n, FILE *stream);
#define O_CLOEXEC 02000000
#endif
+#ifndef O_PATH
+#define O_PATH 010000000
+#endif
+
#if defined(__SIZEOF_DOUBLE__) && defined(__SIZEOF_LONG__) \
&& __SIZEOF_DOUBLE__ <= __SIZEOF_LONG__
typedef double FLOAT;
@@ -257,5 +261,7 @@ int getcon(void* con);
#define smack_set_label_for_self(...) (-1)
#define XATTR_NAME_SMACK ""
#define SMACK_LABEL_LEN (1) /* for just ? */
+
+ssize_t fgetxattr (int fd, char *name, void *value, size_t size);
#endif
diff --git a/toys/posix/ls.c b/toys/posix/ls.c
index 892e826..fcce121 100644
--- a/toys/posix/ls.c
+++ b/toys/posix/ls.c
@@ -5,7 +5,7 @@
*
* See http://opengroup.org/onlinepubs/9699919799/utilities/ls.html
-USE_LS(NEWTOY(ls, USE_LS_COLOR("(color):;")"goACFHLR"USE_LS_SMACK("Z")"Sacdfiklmnpqrstux1[-1Cglmnox][-cu][-ftS][-HL]", TOYFLAG_BIN|TOYFLAG_LOCALE))
+USE_LS(NEWTOY(ls, USE_LS_COLOR("(color):;")USE_LS_SMACK("Z")"goACFHLRSacdfiklmnpqrstux1[-1Cglmnox][-cu][-ftS][-HL]", TOYFLAG_BIN|TOYFLAG_LOCALE))
config LS
bool "ls"
@@ -33,7 +33,7 @@ config LS
-f unsorted -r reverse -t timestamp -S size
config LS_SMACK
- bool
+ bool
default y
depends on LS && TOYBOX_SMACK
help
@@ -142,6 +142,27 @@ static int numlen(long long ll)
return snprintf(0, 0, "%llu", ll);
}
+// measure/print smack attributes
+// lr = 0 just measure, 1 left justified, 2 right justified
+static unsigned zmack(struct dirtree *dt, int pad, int lr)
+{
+ char buf[SMACK_LABEL_LEN+1];
+ int fd = openat(dirtree_parentfd(dt), dt->name, O_PATH|O_NOFOLLOW);
+ ssize_t len = 1;
+
+ strcpy(buf, "?");
+ if (fd != -1) {
+ len = fgetxattr(fd, XATTR_NAME_SMACK, lr?buf:0, lr?SMACK_LABEL_LEN:0);
+ close(fd);
+
+ if (len<1 || len>SMACK_LABEL_LEN) len = 0;
+ else buf[len] = 0;
+ }
+ if (lr) printf(" %*s "+(lr==2), pad*((2*(lr==1))-1), buf);
+
+ return len;
+}
+
// Figure out size of printable entry fields for display indent/wrap
static void entrylen(struct dirtree *dt, unsigned *len)
@@ -168,13 +189,7 @@ static void entrylen(struct dirtree *dt, unsigned *len)
len[6] = (flags & FLAG_s) ? numlen(st->st_blocks) : 0;
- if (CFG_LS_SMACK && (flags & FLAG_Z)) {
- char *zpath = dirtree_path(dt, 0);
- ssize_t zlen = lgetxattr(zpath, XATTR_NAME_SMACK, 0, 0);
- free(zpath);
- len[7] = (zlen <= 0) || (zlen > SMACK_LABEL_LEN) ? 0 : (int)zlen;
- } else
- len[7] = 0;
+ if (CFG_LS_SMACK && (flags & FLAG_Z)) len[7] = zmack(dt, 0, 0);
}
static int compare(void *a, void *b)
@@ -427,17 +442,7 @@ static void listfiles(int dirfd, struct dirtree *indir)
printf("%s% *ld %s%s%s%s", perm, totals[2]+1, (long)st->st_nlink,
usr, upad, grp, grpad);
- if (CFG_LS_SMACK) {
- if (flags & FLAG_Z) {
- char zbuf[SMACK_LABEL_LEN + 1];
- char *zpath = dirtree_path(sort[next], 0);
- ssize_t zlen = getxattr(zpath, XATTR_NAME_SMACK, zbuf, sizeof(zbuf));
- free(zpath);
- if ((zlen <= 0) || (zlen > SMACK_LABEL_LEN)) zbuf[0] = '?', zlen = 1;
- zbuf[zlen] = '\0';
- xprintf(" %s%s", zbuf, toybuf+256-(totals[7]-(int)zlen));
- }
- }
+ if (CFG_LS_SMACK && (flags & FLAG_Z)) zmack(sort[next], totals[7], 1);
if (S_ISCHR(st->st_mode) || S_ISBLK(st->st_mode))
printf("% *d,% 4d", totals[5]-4, major(st->st_rdev),minor(st->st_rdev));
@@ -446,17 +451,7 @@ static void listfiles(int dirfd, struct dirtree *indir)
tm = localtime(&(st->st_mtime));
strftime(thyme, sizeof(thyme), "%F %H:%M", tm);
xprintf(" %s ", thyme);
- } else if (CFG_LS_SMACK) {
- if (flags & FLAG_Z) {
- char zbuf[SMACK_LABEL_LEN + 1];
- char *zpath = dirtree_path(sort[next], 0);
- ssize_t zlen = getxattr(zpath, XATTR_NAME_SMACK, zbuf, sizeof(zbuf));
- free(zpath);
- if ((zlen <= 0) || (zlen > SMACK_LABEL_LEN)) zbuf[0] = '?', zlen = 1;
- zbuf[zlen] = '\0';
- xprintf("%s%s ", toybuf+256-(totals[7]-(int)(zlen-1)), zbuf);
- }
- }
+ } else if (CFG_LS_SMACK && (flags & FLAG_Z)) zmack(sort[next], totals[7],2);
if (flags & FLAG_color) {
color = color_from_mode(st->st_mode);
Rob
P.S. I note that pops up a useless gui which is taller than my netbook's
screen. Although said gui lets you launch a terminal window with a really
tiny font, it completely ignores the keyboard so you can't _type_ anything
into it. However, if you edit run.sh to change -serial to point to /dev/tty,
then you get a console on stdin/stdout of the qemu process. Well, your bespoke
forked qemu, anyway. At least it lets me see what "ls -Z /" and "ls -lZ /" are
supposed to output...
P.P.S. Ah, I see that if you right click you can switch the keyboard on and
change the scale, but at 1/4 size the font in the terminal is _hilariously_
tiny (my vision isn't that great) so I'm sticking with the serial console
on stdio thing. Luckily google found I can login as "root/tizen".
1430370946.0
More information about the Toybox
mailing list