VT_PROCESS, VT_LOCKSWITCH capabilities

VT_PROCESS, VT_LOCKSWITCH capabilities

[Frank Benkstein]

Hi,

I wonder why there are different permissions needed for VT_PROCESS
(access to the current virtual console) and VT_LOCKSWITCH
(CAP_SYS_TTY_CONFIG).

The first one lets the calling process decide if console switching is
allowed, the second one simply disables it.  If a program wants to
forbid console switching the only technical difference I can see is that
switching is automatically reenabled when the program exits when using
VT_PROCESS.  When using VT_LOCKSWITCH it must be manually reenabled.
When the program uses the first method and disables terminal signals and
SysRQ is disabled, too, I see no practical difference between the two.

Please CC me on replies, I am not on the list.

Best regards
Frank Benkstein.

Re: VT_PROCESS, VT_LOCKSWITCH capabilities

[Frank Benkstein]

[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

Frank Benkstein wrote:
> I wonder why there are different permissions needed for VT_PROCESS
> (access to the current virtual console) and VT_LOCKSWITCH
> (CAP_SYS_TTY_CONFIG).

To be more direct:

require CAP_SYS_TTY_CONFIG for VT_SETMODE as its essentially the same as
VT_LOCKSWITCH and said capability is already required there

diff --git a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
index c6f6f42..7034a68 100644
--- a/drivers/char/vt_ioctl.c
+++ b/drivers/char/vt_ioctl.c
@@ -662,7 +662,7 @@ int vt_ioctl(struct tty_struct *tty, struct file * file,
        {
                struct vt_mode tmp;

-               if (!perm)
+               if (!perm || !capable(CAP_SYS_TTY_CONFIG))
                        return -EPERM;
                if (copy_from_user(&tmp, up, sizeof(struct vt_mode)))
                        return -EFAULT;

-- 
GPG (Mail): 7093 7A43 CC40 463A 5564  599B 88F6 D625 BE63 866F
GPG (XMPP): 2243 DBBA F234 7C5A 6D71  3983 9F28 4D03 7110 6D51


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: VT_PROCESS, VT_LOCKSWITCH capabilities

[Andrew Morton]

On Wed, 01 Aug 2007 04:44:32 +0200
Frank Benkstein <frank@benkstein.net> wrote:

> Frank Benkstein wrote:
> > I wonder why there are different permissions needed for VT_PROCESS
> > (access to the current virtual console) and VT_LOCKSWITCH
> > (CAP_SYS_TTY_CONFIG).
> 
> To be more direct:
> 
> require CAP_SYS_TTY_CONFIG for VT_SETMODE as its essentially the same as
> VT_LOCKSWITCH and said capability is already required there
> 
> diff --git a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
> index c6f6f42..7034a68 100644
> --- a/drivers/char/vt_ioctl.c
> +++ b/drivers/char/vt_ioctl.c
> @@ -662,7 +662,7 @@ int vt_ioctl(struct tty_struct *tty, struct file * file,
>         {
>                 struct vt_mode tmp;
> 
> -               if (!perm)
> +               if (!perm || !capable(CAP_SYS_TTY_CONFIG))
>                         return -EPERM;
>                 if (copy_from_user(&tmp, up, sizeof(struct vt_mode)))
>                         return -EFAULT;
> 

There's a good risk of breaking stuff with this change.  A quick peek
through http://www.google.com/codesearch shows that.

We need good reasons for making that change, and for handling the
subsequent fallout, getting shouted at by aggrieved users, etc.

It's tricky.

Re: VT_PROCESS, VT_LOCKSWITCH capabilities

[Frank Benkstein]

[-- Attachment #1: Type: text/plain, Size: 2362 bytes --]

Andrew Morton wrote:
> On Wed, 01 Aug 2007 04:44:32 +0200
> Frank Benkstein <frank@benkstein.net> wrote:
> 
>> Frank Benkstein wrote:
>>> I wonder why there are different permissions needed for VT_PROCESS
>>> (access to the current virtual console) and VT_LOCKSWITCH
>>> (CAP_SYS_TTY_CONFIG).
>> To be more direct:
>>
>> require CAP_SYS_TTY_CONFIG for VT_SETMODE as its essentially the same as
>> VT_LOCKSWITCH and said capability is already required there
>>
>> diff --git a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
>> index c6f6f42..7034a68 100644
>> --- a/drivers/char/vt_ioctl.c
>> +++ b/drivers/char/vt_ioctl.c
>> @@ -662,7 +662,7 @@ int vt_ioctl(struct tty_struct *tty, struct file * file,
>>         {
>>                 struct vt_mode tmp;
>>
>> -               if (!perm)
>> +               if (!perm || !capable(CAP_SYS_TTY_CONFIG))
>>                         return -EPERM;
>>                 if (copy_from_user(&tmp, up, sizeof(struct vt_mode)))
>>                         return -EFAULT;
>>
> 
> There's a good risk of breaking stuff with this change.  A quick peek
> through http://www.google.com/codesearch shows that.
> 
> We need good reasons for making that change, and for handling the
> subsequent fallout, getting shouted at by aggrieved users, etc.
> 
> It's tricky.

I had a quick look through codesearch, too.  Another solution may be to
allow VT_SETMODE but deny VT_RELDISP if CAP_SYS_TTY_CONFIG is not
present. This way graphics tools still get notified when the console is
switched but can't prevent it.

It probably isn't worth the hassle.  I was just looking though the
kernel source to find out which ioctl would be better for my own console
locking tool to use.  I'm happy that it doesn't have to be setuid-root
but at the same time it nags me a little that denying service (and
potentially making users lose data because they cannot save) is so easy.
 And there is no way to switch it off.  Other than carrying the patch
myself, that is.

Regarding your earlier remark of VT_LOCKSWITCH potentially affecting the
session of the next user:  this is also possible with VT_PROCESS by
starting the locking process in the background.

-- 
GPG (Mail): 7093 7A43 CC40 463A 5564  599B 88F6 D625 BE63 866F
GPG (XMPP): 2243 DBBA F234 7C5A 6D71  3983 9F28 4D03 7110 6D51




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: VT_PROCESS, VT_LOCKSWITCH capabilities

[Andrew Morton]

On Wed, 01 Aug 2007 00:22:38 +0200 Frank Benkstein <frank-lkml@benkstein.net> wrote:

> I wonder why there are different permissions needed for VT_PROCESS
> (access to the current virtual console) and VT_LOCKSWITCH
> (CAP_SYS_TTY_CONFIG).
> 
> The first one lets the calling process decide if console switching is
> allowed, the second one simply disables it.  If a program wants to
> forbid console switching the only technical difference I can see is that
> switching is automatically reenabled when the program exits when using
> VT_PROCESS.  When using VT_LOCKSWITCH it must be manually reenabled.
> When the program uses the first method and disables terminal signals and
> SysRQ is disabled, too, I see no practical difference between the two.

It'd take some kernel archaeology to work out how things got the way they
are.

Perhaps the issue with VT_LOCKSWITCH is that its effects will persist after
the user has logged out?  So user A is effectively altering user B's
console, hence suitable capabilities are needed?

Is the current code actually causing any observable problem?

Re: VT_PROCESS, VT_LOCKSWITCH capabilities

[Frank Benkstein]

Andrew Morton wrote:
> On Wed, 01 Aug 2007 00:22:38 +0200 Frank Benkstein <frank-lkml@benkstein.net> wrote:
> 
>> I wonder why there are different permissions needed for VT_PROCESS
>> (access to the current virtual console) and VT_LOCKSWITCH
>> (CAP_SYS_TTY_CONFIG).
>>
> Perhaps the issue with VT_LOCKSWITCH is that its effects will persist after
> the user has logged out?  So user A is effectively altering user B's
> console, hence suitable capabilities are needed?
> 
> Is the current code actually causing any observable problem?

Both controls can be used to deny service to other users.  For example:
user B locks his X session or current console and walks off to lunch.
User A walks up to user B's machine, switches to another console, logs
in and execs program_that_does_vt_process.  User B will not be able to
continue work unless he/she can get user A or someone with CAP_KILL to
kill the program.  If remote logins aren't allowed, the only way I see
to use the machine again is to reboot.

I think VT_PROCESS (or VT_SETMODE respectively) should be protected with
the same level of security as VT_LOCKSWITCH, i.e. CAP_SYS_TTY_CONFIG.