* [kbd] [PATCH] vlock: allow sudo user to unlock his session
@ 2020-08-01 13:19 Mikhail Novosyolov
2020-08-09 16:08 ` Alexey Gladkov
0 siblings, 1 reply; 5+ messages in thread
From: Mikhail Novosyolov @ 2020-08-01 13:19 UTC (permalink / raw)
To: kbd
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.
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [kbd] [PATCH] vlock: allow sudo user to unlock his session
2020-08-01 13:19 [kbd] [PATCH] vlock: allow sudo user to unlock his session Mikhail Novosyolov
@ 2020-08-09 16:08 ` Alexey Gladkov
2020-08-09 20:50 ` Mikhail Novosyolov
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Gladkov @ 2020-08-09 16:08 UTC (permalink / raw)
To: Linux console tools development discussion; +Cc: Mikhail Novosyolov
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. 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.
> 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
--
Rgrds, legion
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [kbd] [PATCH] vlock: allow sudo user to unlock his session
2020-08-09 16:08 ` Alexey Gladkov
@ 2020-08-09 20:50 ` Mikhail Novosyolov
2020-08-10 11:16 ` Alexey Gladkov
0 siblings, 1 reply; 5+ messages in thread
From: Mikhail Novosyolov @ 2020-08-09 20:50 UTC (permalink / raw)
To: Alexey Gladkov, Linux console tools development discussion
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [kbd] [PATCH] vlock: allow sudo user to unlock his session
2020-08-09 20:50 ` Mikhail Novosyolov
@ 2020-08-10 11:16 ` Alexey Gladkov
2020-08-23 17:47 ` Михаил Новоселов
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Gladkov @ 2020-08-10 11:16 UTC (permalink / raw)
To: Mikhail Novosyolov; +Cc: Linux console tools development discussion
On Sun, Aug 09, 2020 at 11:50:07PM +0300, Mikhail Novosyolov wrote:
>
>
> 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...
Now the LOGNAME is essentially not used. The vlock calls getpwnam and if
the pw_uid does not match with current uid, vlock calls getpwuid.
Checking the uid protects against incorrect LOGNAME.
Your patch removes uid check and forces vlock to always use environment
variables. Now an incorrect LOGNAME cannot change the behavior of vlock,
but with your patch it will.
I think your patch is completely wrong.
I rather think that the LOGNAME check is unnecessary here. Dmitry can
probably remember why this check is here.
> > 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?
If I want to block the console with a root password, then I can do:
$ sudo vlock
> >
> >> 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.
>
--
Rgrds, legion
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [kbd] [PATCH] vlock: allow sudo user to unlock his session
2020-08-10 11:16 ` Alexey Gladkov
@ 2020-08-23 17:47 ` Михаил Новоселов
0 siblings, 0 replies; 5+ messages in thread
From: Михаил Новоселов @ 2020-08-23 17:47 UTC (permalink / raw)
To: Alexey Gladkov; +Cc: Linux console tools development discussion
----- Исходное сообщение -----
> От: "Alexey Gladkov" <gladkov.alexey@gmail.com>
> Кому: "Михаил Новоселов" <m.novosyolov@rosalinux.ru>
> Копия: "Linux console tools development discussion" <kbd@lists.altlinux.org>, "Dmitry V. Levin" <ldv@altlinux.org>
> Отправленные: Понедельник, 10 Август 2020 г 14:16:21
> Тема: Re: [kbd] [PATCH] vlock: allow sudo user to unlock his session
> On Sun, Aug 09, 2020 at 11:50:07PM +0300, Mikhail Novosyolov wrote:
>> >
>> >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...
>
> Now the LOGNAME is essentially not used. The vlock calls getpwnam and if
> the pw_uid does not match with current uid, vlock calls getpwuid.
> Checking the uid protects against incorrect LOGNAME.
>
> Your patch removes uid check and forces vlock to always use environment
> variables. Now an incorrect LOGNAME cannot change the behavior of vlock,
> but with your patch it will.
I probably confused something and thought that vlock fallbacks to root user, not the current user.
Fallback to the current user is good behavior.
>
>> > 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?
>
> If I want to block the console with a root password, then I can do:
>
> $ sudo vlock
Sounds reasonable, I don't know how to find out if vlock was run like this or not.
Actually I do not have much interest in implementing this, because neither me,
nor any people that I know ever used vlock, so let's leave this problem for future.
Thanks for review!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-23 17:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-01 13:19 [kbd] [PATCH] vlock: allow sudo user to unlock his session Mikhail Novosyolov
2020-08-09 16:08 ` Alexey Gladkov
2020-08-09 20:50 ` Mikhail Novosyolov
2020-08-10 11:16 ` Alexey Gladkov
2020-08-23 17:47 ` Михаил Новоселов
Linux console tools development discussion
This inbox may be cloned and mirrored by anyone:
git clone --mirror http://lore.altlinux.org/kbd/0 kbd/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 kbd kbd/ http://lore.altlinux.org/kbd \
kbd@lists.altlinux.org kbd@lists.altlinux.ru kbd@lists.altlinux.com
public-inbox-index kbd
Example config snippet for mirrors.
Newsgroup available over NNTP:
nntp://lore.altlinux.org/org.altlinux.lists.kbd
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git