Re: [kbd] [PATCH] vlock: allow sudo user to unlock his session

[Mikhail Novosyolov <m.novosyolov@rosalinux.ru>]

9 августа 2020 г. 19:08:47 GMT+03:00, Alexey Gladkov <gladkov.alexey@gmail.com> пишет:
>On Sat, Aug 01, 2020 at 04:19:59PM +0300, Mikhail Novosyolov wrote:
>> 
>> https://github.com/legionus/kbd/pull/45
>> 
>> 
>> If a non-root user ran sth like "sudo -i" and vlock'ed from inside
>it,
>> then that user himself should be able to unlock his console.
>> 
>> [user@HP-Elite-7300 tmp]$ echo $LOGNAME
>> user
>> [user@HP-Elite-7300 tmp]$ sudo -i
>> root@HP-Elite-7300:~# echo $LOGNAME
>> root
>> root@HP-Elite-7300:~# echo $SUDO_USER
>> user
>> root@HP-Elite-7300:~#
>> 
>> Tested on rosa2019.1 + kbd 2.2.0 + this patch:
>> [root@rosa-2019 kbd]# su - user
>> [user@rosa-2019 ~]$ sudo -i
>> [sudo] password for user:
>> [root@rosa-2019 ~]# vlock
>> Данное устройство tty (console) не является виртуальной консолью.
>> Блокировка console установлена user.
>> Пароль:
>> [root@rosa-2019 ~]#
>> sudo root session was successfully unlocked with user's password.
>> [root@rosa-2019 ~]# unset SUDO_USER
>> [root@rosa-2019 ~]# vlock
>> Данное устройство tty (console) не является виртуальной консолью.
>> Блокировка console установлена root.
>> Пароль:
>> root password is requested without $SUDO_ENV.
>
>I don't like the idea of implicitly changing the user through
>environment
>variables.

I also don't like it, but don't see much difference with setting LOGNAME=vasya before running vlock and then being unable to unlock the console without root due to fallback to uid=0...

> SUDO_USER can be exposed accidentally or leak into the
>environment due to an error. In this case, you will lock the console
>without being able to unlock.
>
>Also, your patch will not allow you to block the console by another
>user
>or by root.

What do you mean?

>
>> Another vlock implementation [1, 2] does not check that UIDs match,
>> I do not see sense in this check, removing it to make what I want
>work.
>> 
>> [1] Another vlock implementation: https://github.com/WorMzy/vlock
>> [2] My similar patch for it:
>https://github.com/mikhailnov/vlock/commit/ba38d5d563cdfaad3b2f260248b3434c235a7afd
>> ---
>>  src/vlock/username.c | 17 +++++++++--------
>>  1 file changed, 9 insertions(+), 8 deletions(-)
>> 
>> diff --git a/src/vlock/username.c b/src/vlock/username.c
>> index a26a148..4c6d295 100644
>> --- a/src/vlock/username.c
>> +++ b/src/vlock/username.c
>> @@ -40,17 +40,18 @@ get_username(void)
>>  {
>>      const char *name;
>>      struct passwd *pw = 0;
>> +    char *logname = NULL;
>>      uid_t uid         = getuid();
>>  
>> -    char *logname = getenv("LOGNAME");
>> +    /* If a non-root runs a sudo session, ask for user's
>> +     * password to unlock it, not root's password */
>> +    logname = getenv("SUDO_USER");
>> +    if (logname == NULL)
>> +        logname = getenv("LOGNAME");
>>  
>> -    if (logname) {
>> -        pw = getpwnam(logname);
>> -        /* Ensure uid is same as current. */
>> -        if (pw && pw->pw_uid != uid)
>> -            pw = 0;
>> -    }
>> -    if (!pw)
>> +    pw = getpwnam(logname);
>> +
>> +    if (!pw && uid)
>>          pw = getpwuid(uid);
>>  
>>      if (!pw)
>> -- 
>> 
>> Please CC me when replying, I am not subscribed to
>kbd@lists.altlinux.org
>> The same patch was submited as a pull request on Github:
>https://github.com/legionus/kbd/pull/45
>> 
>> _______________________________________________
>> kbd mailing list
>> kbd@lists.altlinux.org
>> https://lists.altlinux.org/mailman/listinfo/kbd

-- 
Простите за краткость, создано в K-9 Mail.