[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: RFC: obexapp - virtual root folder for each device



2009/4/14 Mikhail T. <mi+thun_(_at_)_aldan_(_dot_)_algebra_(_dot_)_com>:
> Maksim Yevmenkin написав(ла):
>> hopefully the latest patch will work for everyone.
>>
> Little nits this time...
>
> 1. Even when the service is not running as root to begin with (and thus
> chroot is impossible), a user owning multiple devices may wish to have
> separate directories for each one. Yet, the new -R switch, as proposed,
> requires successful chroot into a matching entry... It does not need to.
> How about:
>
>    +           case 'R': /* virtualize root for each device */
>    +                   context.vroot = 1;
>    +                   if (getuid() == 0)
>    +                           context.secure = 1;
>    +                   break;
>    +
>
> chdir should be used instead of chroot in this case (-R was given, and a
> matching entry is found, but we aren't running as root).

well, a couple of things. for now, we always have to start obexapp as
root because it needs to talk to sdpd(8) to register services. sdpd(8)
is checking credentials (passed via unix sockets) and makes sure that
the process, that is trying to register the service, has uid of 'root'
user. so, strictly speaking this change is a no-op because getuid()
will always be 0 or else obexapp will not start in server mode.

also, i'd _really_ like to keep clients "jailed" under their virtual
root folders. at least for now. as far as keeping and sharing files
under the same root folder, i just thought of a way to "break" the
latest patch: set up symlink that points to '.' under default root
folder. obviously chdir() and chroot() conditions will be satisfied
and you get your files dumped in the same directory.

> 2. When going through the list of possible subdirectories-candidates,
> hardcoding the number 3 is a bit dangerous. Using either
>
>    sizeof(root)/sizeof(root[0])
>
> or simply going, until hitting NULL:
>
>    char const *root[] = { NULL, NULL, NULL, NULL }, **r;
>    ...
>    foreach (r -> root; *r; r++) {
>
>        log_debug("%s(): Checking for %s/%s subdirectory",
>            __func__, context->root, *r);
>        ...
>
> would be a bit safer going forward -- what if some other parameter (such
> as an environment variable) may some day be added to the list of
> considerations?

that's fine. i will fix it.

> 3. After starting up, with the -R (or the -r) option, should/does not
> the daemon chdir into the specified top-level directory? And if so,
> there is no need to assemble the context->root with strlcat -- just
> perform chdir into the relative root[n] (or *r in my example). The
> chroot can then happen to ".". After chdir-ing, you can populate the
> actual contet->root with getcwd(context->root, PATH_MAX) -- a faster
> equivalent to using realpath(3).

it does, chdir(), i.e. obexapp_server_set_initial_root() does it.
strlcat() is not that expensive, imo. it can be changed, i guess.

> This is not material, but the fewer cases, where a hard-coded PATH_MAX
> is used instead of the POSIX-approved pathconf(2), the better...

PATH_MAX comes from sys/syslimits.h, so, i thought it would be ok to
use. another alternative was MAXPATHLEN which was the same.

thanks,
max
_______________________________________________
freebsd-bluetooth_(_at_)_freebsd_(_dot_)_org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bluetooth
To unsubscribe, send any mail to "freebsd-bluetooth-unsubscribe_(_at_)_freebsd_(_dot_)_org"