[Pkg-shadow-devel] Newuidmap works with usernames instead of uids

[Bostjan Skufca]

On 1 September 2014 22:55, Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> Thanks, looks good overall.  Note that you are not freeing
> buf after you malloc it.
/me slightly rusty in c ATM. But hey, on the bright side I managed to
scr.w up 100% of newly used and mallocated variables, nice score!

Tnx, fixed.


>         sprintf(owner_uid_string, "%d", owner_pwd.pw_uid
>
> should that be a %u ?

Yes, fixed.


> It is quite a shame as this will lengthen the failure case by
> quite a bit, especially the 'two usernames with same uid'
> concern.  But it seems necessary.


Hm, it seems we have three options:
1. single loop, as it is now
2. dual loop, as proposed by the patch: first loop is fast, without
getpwnam syscalls, comparing literal usernames; the second is slower.
Failure case lenghtens, as you have said.
3. single loop, where we convert and compare everything with UIDs and
ignore usernames completely.


What to do, which way to choose?

If we think about how actual login (or ssh login) process works, there
are actual usernames compared/matched and you can not use username
"uid100_user1" and then type in password from user "uid100_user2".
Therefore: usernames are compared literaly. It does not matter that
the uid is the same for both usernames.

But, once you are in the system, there the username used is of little
significance. All that matters is UID (same UID, file operation
allowed; different UID, it depends on file permissions).

I would recommend it the way the patch does it: first literal
comparison (so users are able to create preferences for different
usernames). If that fails, get the first UID that matches and use
that.


We may benchmark what actually uses more time: looping itself or
cumulated syscalls to getpwnam_r().

If it turns out that getpwnam is cheaper than looping itself, we might
as well getpwnam all entries in the first loop and save the first UID
entry that matches. If the rest of the loop does not find the exact
username, we would not have to run the loop again, as we already have
the entry of interest.


b.


>
> thanks,
> -serge
>
> Quoting Bostjan Skufca (bostjan at a2o.si):
>> Change preview:
>> https://github.com/bostjan/shadow/commit/1e2a7df18d12c5a4d58e47afa7e3087e009371ea
>>
>> (I am already using it, works fine.)
>>
>> b.
>>
>>
>>
>> On 22 August 2014 23:19, Bostjan Skufca <bostjan at a2o.si> wrote:
>>
>> > Tnx for reply.
>> >
>> > BTW: How do these two repo locations relate actually?
>> > 1. git://anonscm.debian.org/git/pkg-shadow/shadow.git
>> > 2. https://github.com/shadow-maint/shadow
>> >
>> > Second one seems stale (less activity in 2014, but it also does not seem
>> > to be *just* lagging behind).
>> >
>> > b.
>> >
>> >
>> >
>> > On 22 August 2014 16:12, Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
>> >
>> >> Quoting Bostjan Skufca (bostjan at a2o.si):
>> >> > Hi there,
>> >> > (below I only describe situation for UIDs, but content also applies to
>> >> GIDs)
>> >> >
>> >> > UID mapping feature only works if /etc/subuid contains exact username
>> >> > definition. Contrary to expectations* it does not work if instead exact
>> >> > username UID is used.
>> >> >
>> >> > Additionaly: It also does not work if mapping is requested by username,
>> >> for
>> >> > which there is no explicit configuration in /etc/subuid, but /etc/subuid
>> >> > contains mapping definition for an username which shares the same UID
>> >> with
>> >> > original requesting username.
>> >> >
>> >> > Is this by design (using names instead of UIDs)?
>> >> > If so, why?
>> >> > Would you accept patches to at least enable mapping definitions which
>> >> use
>> >>
>> >> Hi,
>> >>
>> >> nope doesn't seem to be by design, so patches appreciated.
>> >>
>> >> -serge
>> >>
>> >> > UIDs instead of usernames?
>> >> >
>> >> >
>> >> > Thank you for your answer,
>> >> > b.
>> >> >
>> >> > (*expectations: mine, not general, though this is debatable)
>> >>
>> >> > _______________________________________________
>> >> > Pkg-shadow-devel mailing list
>> >> > Pkg-shadow-devel at lists.alioth.debian.org
>> >> >
>> >> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-shadow-devel
>> >>
>> >>
>> >