Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: pam_set_item(,PAM_TTY, tty) requires 'tty' starts with '/dev/' #1242

Closed
kreijack opened this issue Feb 2, 2021 · 1 comment
Closed

Comments

@kreijack
Copy link
Contributor

kreijack commented Feb 2, 2021

Short version:

In 'su-common.c', the tty_name is computed without '/dev/' (e.g. 'tty3' or 'pts/3' instead of '/dev/tty3' or '/dev/pts/3'). When this name is passed to pam, it causes the failure of the module pam_timestamp.so. Looking at the man page of pam_set_item(3), it is stated that TTY_NAME requires the device starts with '/dev/'.

Long version:

I am referring to the following line

https://github.com/karelzak/util-linux/blob/82643a9be507bd20df98d38c8248299d1084ac74/login-utils/su-common.c#L370

where pam_set_item(,PAM_TTY, su->tty_name) is invoked with tty_name stripped by '/dev/'.
In fact su->tty_name is computed in init_tty using
https://github.com/karelzak/util-linux/blob/82643a9be507bd20df98d38c8248299d1084ac74/login-utils/su-common.c#L181

which strips "/dev/" in the second argument (see https://github.com/karelzak/util-linux/blob/82643a9be507bd20df98d38c8248299d1084ac74/lib/ttyutils.c#L111)

The pam_timestamp.so module is confused by a tty name without the '/dev/' prefix, but with a '/' in the name (like 'pts/3').
See https://github.com/linux-pam/linux-pam/blob/95b464f8417da81d8c495fb424e24de5e196ab12/modules/pam_timestamp/pam_timestamp.c#L151 for further details.

I think that the check performed by pam_timestamp is wrong. However as stated in pam_set_item(3), PAM_TTY requires that the device starts with '/dev/'. So even su-common.c is wrong.

karelzak added a commit that referenced this issue Feb 3, 2021
pam_set_item() man page:
 PAM_TTY
   The terminal name: prefixed by /dev/ if it is a device file;
   for graphical, X-based, applications the value for this item
   should be the $DISPLAY variable.

It seems for example pam_timestamp module is not robust enough to
differentiate between /dev/ and pty/0 and it assumes that '/' in the
path always means '/dev/' prefix ...

Fixes: #1242
Signed-off-by: Karel Zak <kzak@redhat.com>
@karelzak
Copy link
Collaborator

karelzak commented Feb 3, 2021

Good catch. Fixed in login(1) and su(1).

Frankly, from my point of view, the code in pam_timestamp should be more robust...

karelzak added a commit that referenced this issue Feb 10, 2021
pam_set_item() man page:
 PAM_TTY
   The terminal name: prefixed by /dev/ if it is a device file;
   for graphical, X-based, applications the value for this item
   should be the $DISPLAY variable.

It seems for example pam_timestamp module is not robust enough to
differentiate between /dev/ and pty/0 and it assumes that '/' in the
path always means '/dev/' prefix ...

Fixes: #1242
Signed-off-by: Karel Zak <kzak@redhat.com>
karelzak added a commit that referenced this issue Feb 10, 2021
pam_set_item() man page:
 PAM_TTY
   The terminal name: prefixed by /dev/ if it is a device file;
   for graphical, X-based, applications the value for this item
   should be the $DISPLAY variable.

It seems for example pam_timestamp module is not robust enough to
differentiate between /dev/ and pty/0 and it assumes that '/' in the
path always means '/dev/' prefix ...

Fixes: #1242
Signed-off-by: Karel Zak <kzak@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants