[Toybox] Integration of SMACK
enh
enh at google.com
Wed Apr 29 22:26:25 PDT 2015
you should be able to do ls -Z and ls -lZ on any Ubuntu box (albeit
with missing labels):
~$ ls -lZ /proc/self/fd
total 0
lrwx------ 1 enh eng ? 64 Apr 29 22:22 0 -> /dev/pts/8
lrwx------ 1 enh eng ? 64 Apr 29 22:22 1 -> /dev/pts/8
lrwx------ 1 enh eng ? 64 Apr 29 22:22 2 -> /dev/pts/8
lr-x------ 1 enh eng ? 64 Apr 29 22:22 3 -> /proc/9563/fd
~$ ls -Z /proc/self/fd
? 0 ? 1 ? 2 ? 3
a real SELinux label on Android looks something like "u:object_r:init_exec:s0".
On Wed, Apr 29, 2015 at 10:15 PM, Rob Landley <rob at landley.net> wrote:
> 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".
>
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
1430371585.0
More information about the Toybox
mailing list