Re: A requirement for the current user to own ttys

[Key Offecka]

Hi,

> What do you mean by "the resource" in the lines above? There are at least
two pieces of resource in the game, the tty and the vcsa (maybe more, I
don't know). We'd need a much more precise description.
>> if a user doesn't own the tty device but is a member of a group owning
the tty should that user have the dumb terminal?
I was talking only about tty devices here. I didn't mention vcs* in any
manner.

> On your system where these vcsa devices cannot normally be accessed by a
member of the tty group _and_ the real user is not the same as the tty's
owner, I'm not convinced yet why permission should be granted.

Why do you think on my system I cannot access vcs* devices. As far as I
remember I never said that. I am sorry if didn't make it clear. On my
system cons.saver is owned by the vcsa user and has the setuid bit. vcs*
devices are owned by the vcsa user as well, and the user has read/write
permissions.

Once again, my questions were only about tty devices. I do not know why
you're answering questions I never asked.

I am not sure if the same security policy should be applied to vcs*
devices. All my questions were only about tty since the access to tty
devices causes the issue. There are no problems with vcs* on my system,
though some improvements of mc may also be needed here.

At this moment, as far as I can read the C code, vcs* and tty* devices
treated differently. There are no special checks on vcs* done at all. So I
believe access to the vcs* is relied only on the OS security. There are
special checks on tty and from my perspective they are too paranoid since
involve only the user owning devices.

|| fstat (console_fd, ) < 0 || st.st_uid != uid
|| read (vcsa_fd, buffer, buffer_size) != buffer_size

console_fd is a tty device here not vcsa

As I mentioned, I think groups also should be involved in the checks of tty
devices. That's it. Nothing more.

Anyway, thank you very much for the discussion. I believe the both sides
received all needed information. It's now up to you whether you want to
change something in the mc security or not, whether you want to make it
less paranoid or not.

--
Best regards,
Konstantín


On 11 March 2017 at 19:57, Egmont Koblinger <egm...@gmail.com> wrote:

> Hi,
>
> On Sun, Mar 12, 2017 at 12:54 AM, Key Offecka <key.offe...@gmail.com>
> wrote:
>
>>
>> if the user (the real user, not the effective one) is root then
>> permission check is successful
>> else
>>   if the user owns the resource then permission check is successful
>>   else
>> if the user belongs to the group owning the resource then permission
>> check is successful
>>
>
> I guess we should also add "and the resource is read-writable by its group"
>
>
>> else deny the access
>>
>
> What do you mean by "the resource" in the lines above? There are at least
> two pieces of resource in the game, the tty and the vcsa (maybe more, I
> don't know). We'd need a much more precise description.
>
> So, I think the main question here: isn't mc too paranoid?
>>
>
> It could easily be, dunno.
>
>
>> To answer this question we could try answering some more specific
>> questions:
>>
>> How do you think
>> 1) is it OK that in the descried example root has that dumb terminal?
>> 2) if a user doesn't own the tty device but is a member of a group owning
>> the tty, should that user have the dumb terminal?
>>
>
> There are two sides of the story: the user-expected behavior and the
> technical possibilities. Not sure if the two match here. Normally I'd
> always put the user-facing behavior first and do the technical bits to
> implement the expected user-facing behavior. Having a setuid/setgid bit
> changes the game quite a bit, compromises might become necessary, not
> having a security hole becomes the number one priority even if the
> user-facing behavior suffers from drawbacks.
>
> What I would say is: If the actual real user has the sufficient access to
> the tty and vcsa devices even without a setuid/setgid bit (in other words:
> they could compile and install a modified version of cons.saver for
> themselves which removes all the current verifications and just tries to
> operate on the devices and would succeed even without the setuid/setgid
> bits) then the checks we have in place should not introduce any obstacles.
> There's no point in denying something that a non-setuid/setgid appication
> could do.
>
> This definitely covers your 1st point. Root should always have access.
>
> I am not convinced about the 2nd. On my system (Ubuntu) the vcsa devices
> belong to the tty group and have read-write perms for this group, so on my
> system, yes. On your system where these vcsa devices cannot normally be
>

Re: A requirement for the current user to own ttys

[Key Offecka]

Hi,

> You did mention "sudo" a couple of times

Yes, I did. And maybe even more times, but I never told about extra rights
obtained by a user just because of sudoing.

> You keep talking about "first" and "second" user, in order to have these
you must switch user by some means

And I told you, in my test case I use sudo (please mind the *just*)

>> sudo here is just used to switch from echo to ghost

Once again, sudo is for switching users, access to echo's resources is a
different question.

> There's no sticky bit in the game, I assume you meant the setuid or
setgid bit.

Please excuse me, you are totally right, of course it's setuid bit in game
here. Sir, I have no explanations why I said 'stick bit'. Demon possession?

>> I said: the second user (`ghost` in this example) is authorised to act
on behalf of `echo`.
> This is an absolutely false expectation. It's not how things are working
(and I'm not talking about mc here). The second user does _not_ have access
to the first user's belongings. Try sudoing from a normal user to another
normal one, and then remove a file of the first user. You'll be denied
access.

Yes, but mc doesn't delete tty3. So the expectation is not necessarily
false. It depends on a particular case. In your example, it's false. But
there may be others when it's valid, for example I still can do the
following

sudo -u ghost bash -c 'echo hello echo >/dev/tty3'

just because ghost belongs to the tty group. So by giving the write
permission on tty3 to the tty group `echo` authorizes members of the group
to write to tty3. It may be not quite correct to say "echo authorizes to
act participants of the tty group on behave of them". Sorry for the unclear
wording.

But you were right I'd got a vague understanding of how it was supposed to
work. Thanks to explanations (I really appreciate your wasting time on me)
I think I now understand it's also up to the software to handle permissions.

> It's still unclear to me: What do you expect cons.saver to do differently?

And my suggestions are:

if the user (the real user, not the effective one) is root then permission
check is successful
else
  if the user owns the resource then permission check is successful
  else
if the user belongs to the group owning the resource then permission
check is successful
else deny the access

> if I take it as a setuid root cons.saver should allow any user to
save/restore the contents of any vcs (that's what root can do anyways) then
it's a big fat NO for security reasons

I agree.

> Could you please be more specific? I do not understand what you're
_exactly_ asking for. E.g. "respect root privileges" is way too generic

By saying respect root permissions I mean that if root runs mc, root should
see the background contents and shouldn't press any key after executing
commands even though if root doesn't own the tty.

> Maybe the checks it's doing are imperfect, maybe they are a bit too
strict or paranoid and could be loosened up, I don't know.

So, I think the main question here: isn't mc too paranoid? To answer this
question we could try answering some more specific questions:

How do you think
1) is it OK that in the descried example root has that dumb terminal?
2) if a user doesn't own the tty device but is a member of a group owning
the tty, should that user have the dumb terminal?

On the both questions personally I answer: no and no. It's not OK that root
has the dump terminal, in my opinion it's not OK for a member of the tty
group to have the dumb terminal.

Off topic:

> It could be a matter of taste, but I definitely prefer the graphical
emulators' behavior here

You are right, it's a matter of taste.

> although I'm not sure if I should mention this

You shouldn't have to. The same way you could say: to fix the issue don't
use mc, use Krusader.

--
Best regards,
Konstantín

On 11 March 2017 at 16:24, Egmont Koblinger  wrote:

> Hi,
>
> On Sat, Mar 11, 2017 at 7:50 PM, Konstantin I. 
> wrote:
>
>> Hi,
>>
>> > Nope. Via "sudo", the first user is allowed to execute certain commands
>> on behalf of the second, not the other way around.
>>
>> I didn't say "via sudo"
>>
> You did mention "sudo" a couple of times. You keep talking about "first"
> and "second" user, in order to have these you must switch user by some
> means. sudo, su, perhaps ssh localhost -- can you think of any other means?
>
>
>> I said: the second user (`ghost` in this example) is authorised to act on
>> behalf of `echo`.
>>
> This is an absolutely false expectation. It's not how things are working
> (and I'm not talking about mc here). The second user does _not_ have access
> to the first user's belongings. Try sudoing from a normal user to another
> normal one, and then remove a file of the first user. You'll be denied
> access.
>
>
>> How it's done is irrelevant. You mentioned PAM. Ok. Let it be PAM.
>>
> In that case mc-devel is probably not the right forum to discuss it ;) I
> admit 

Re: A requirement for the current user to own ttys

[Key Offecka]

Hi Egmont,

> What do you mean the tty owner is the _problem_? What kind of problem?

Please excuse me, I wasn't quite correct there.

Let's forget about permissions and TTYs. Let's look at the issue from the
user point of view. Please consider this case:

There is a user, say `echo` and there is another user, say `ghost`. It can
also be `root` but that is not necessary. The requirement here is that the
second user (`ghost` in this example) is authorised to act on behalf of
`echo`. User `echo` logs on a tty, say tty3. In terms of permissions (OK,
it's difficult to forget about them), ghost is a part of the tty group

echo:/etc/init.d$ ls -la /dev/tty3
crw-rw 1 echo tty 4, 3 mar 11 09:40 /dev/tty3

echo:/etc/init.d$ groups ghost
ghost : users root tty wheel

echo:/usr/lib/mc$ ls -la cons.saver
-rwsr-sr-x 1 vcsa root 10144 mar 11 09:49 cons.saver

echo:/usr/lib/mc$ ls -la /dev/vcs{,{,a}3}
crw--- 1 vcsa root 7,   0 mar 11 09:28 /dev/vcs
crw--- 1 vcsa root 7,   3 mar 11 09:28 /dev/vcs3
crw--- 1 vcsa root 7, 131 mar 11 09:28 /dev/vcsa3

The logged in user `echo` does sudo like so

sudo -u ghost mc

Now we execute a command in mc, mc runs it but before returning back it
shows that ugly message about any key. If we press something, we return to
the normal mc panels, but if we want to see the printed results of the
previously executed command, we see the blank screen.

Question: does mc work in this case as it was designed?
The behaviour I would expect: in the example above mc shouldn't stop after
executing commands and should show previous command output.

The same issue is valid for root, root is just more obvious example of the
misbehaviour.

And the reason of the misbehaviour in my opinion is that there is a
requirement for the tty to be owned by `ghost` in this example. And as I
mentioned several times before, this comes from this sort of comparison

fstat (console_fd, ) < 0 || st.st_uid != uid

That piece of information shouldn't be used like so. I think to achieve the
goals you mentioned above (to not allow others to see what they shouldn't
see) another solution should be found. Do you agree?

--
Konstantin


On 11 March 2017 at 04:37, Egmont Koblinger  wrote:

> Hi,
>
> > All you say about vcs* sounds reasonable, unfortunately according to the
> code, the tty owner is the problem.
>
> What do you mean the tty owner is the _problem_? What kind of problem?
>
> I believe it's not the _problem_, it's the piece of information we rely on
> to figure out if cons.saver is being run as a legitime user.
>
> > Disregarding of what was the intention,  disregarding of what you were
> trying to achieve and what security hole to close, do you think, that sort
> of comparison is valid here?
>
> I'm not aware of the details of the code and don't have time to dig into
> it, sorry.
>
> As far as I understand, your problem is: You expect that if the real user
> is root, cons.saver should dutifully perform its role rather than bail out
> due to some ownership mismatch. Am I right? If so, I believe it's a fair
> request, although possible security implications should be double checked.
> Could you please file a new bug for this?
>
> Thanks,
> egmont
>
>
___
mc-devel mailing list
https://mail.gnome.org/mailman/listinfo/mc-devel

Re: A requirement for the current user to own ttys

[Key Offecka]

Thank you, egmont, for your time answering my question. All you say about
vcs* sounds reasonable, unfortunately according to the code, the tty owner
is the problem.

fstat (console_fd, ) < 0 || st.st_uid != uid

As far as I understand the c code here, you compare the user id with the id
of the tty owner. This happens in the very beginning of the main function,
and then in

case CONSOLE_SAVE

if (seteuid (euid) < 0
|| lseek (vcsa_fd, 0, 0) != 0
|| fstat (console_fd, ) < 0 || st.st_uid != uid
|| read (vcsa_fd, buffer, buffer_size) != buffer_size
|| fstat (console_fd, ) < 0 || st.st_uid != uid)

and then in

case CONSOLE_RESTORE

if (seteuid (euid) >= 0
&& lseek (vcsa_fd, 0, 0) == 0 && fstat (console_fd, ) >=
0 && st.st_uid == uid)

Yes, you set the effective user id (in case when cons.server has that
sticky bit and is owned by vcsa, it is the vcsa user). You need it to be
able to

lseek (vcsa_fd, 0, 0) and  read (vcsa_fd, buffer, buffer_size)

But then you compare the  owner id of the tty with the real user id (in my
example it is the root user), the comparison fails and the root user sees
the blank screen in one case, and has to press the `anyKey` in the other as
described earlier.

Disregarding of what was the intention,  disregarding of what you were
trying to achieve and what security hole to close, do you think, that sort
of comparison is valid here?

Thank you.

--
Konstantin

On 10 March 2017 at 02:30, Egmont Koblinger <egm...@gmail.com> wrote:

> Hi,
>
> cons.saver, as you apparently know this, is the helper binary responsible
> for restoring the contents of the Linux console when you quit mc or press
> Ctrl+O. A helper is required since the Linux console does not have an
> "alternate screen" that graphical terminal emulators have.
>
> In order to be able to do this, it needs read/write access to /dev/vcsa*.
>
> When you log in on the console, the corresponding /dev/tty* becomes owned
> by you but /dev/vcsa* don't. I believe the reason behind it is that there
> is a way to revoke the tty from you, but there is no way to revoke the vcsa
> access. That is, when you log out, you might keep a background process
> running which still has access to it via a previously opened file
> descriptor, and subsequently as someone else logs in, you could spy on the
> console's contents.
>
> As such, since /dev/vcsa* is not owned by the desired user, cons.saver
> needs to be setgid tty (or setuid root).
>
> Setuid/setgid apps must have all these kinds of precautions that you're
> asking about, they need to duplicate the permission checks because they are
> not being run as the actual real user. It's crucial that someone not
> actually sitting in front of the tty cannot trick cons.saver into tampering
> with the tty's contents.
>
> Hope this explains the situation.
>
> I'm not sure why something is checked twice, but it can easily be in order
> to avoid a race condition (or could easily be a harmless bug as well).
>
> egmont
>
>
> On Fri, Mar 10, 2017 at 1:06 AM, Key Offecka <key.offe...@gmail.com>
> wrote:
>
>> Hi,
>>
>> I am looking at the
>>
>> main (int argc, char **argv)
>>
>> function in
>>
>> src/consaver/cons.saver.c
>>
>> There are calls like
>>
>> st.st_uid != uid
>>
>> fstat (console_fd, ) >= 0 && st.st_uid == uid
>>
>> fstat (console_fd, ) < 0 || st.st_uid != uid
>>
>> The last one is especially strange taking into account that it appears
>> twice
>>
>> if (seteuid (euid) < 0
>> || lseek (vcsa_fd, 0, 0) != 0
>> || fstat (console_fd, ) < 0 || st.st_uid != uid
>> || read (vcsa_fd, buffer, buffer_size) != buffer_size
>> || fstat (console_fd, ) < 0 || st.st_uid != uid)
>>
>> This all is taken from the commit e9fd11bfcd1dab97e3ba423bcfb8b6ca1088b11c
>> which is the latest at this moment
>>
>> It looks to me MC tries inventing its own permission scheme rather than
>> relying on the system set up.
>> Consider there is a user in the system who is allowed to read/write and
>> to do whatever they want with vcs, tty and with whatever files else you may
>> only wish. root is one obvious candidate but nothing restricts us to set up
>> another user taking advantaged of  all those system security facilities.
>> There is a traditional UNIX permission scheme, SeLinux may be involved if
>> needed. And now comes MC, and introduces a hardcoded/unconfigurable/solid
>> as a stone requirement for the current user to be the owner of the files.
>> Why so?

A requirement for the current user to own ttys

[Key Offecka]

Hi,

I am looking at the

main (int argc, char **argv)

function in

src/consaver/cons.saver.c

There are calls like

st.st_uid != uid

fstat (console_fd, ) >= 0 && st.st_uid == uid

fstat (console_fd, ) < 0 || st.st_uid != uid

The last one is especially strange taking into account that it appears twice

if (seteuid (euid) < 0
|| lseek (vcsa_fd, 0, 0) != 0
|| fstat (console_fd, ) < 0 || st.st_uid != uid
|| read (vcsa_fd, buffer, buffer_size) != buffer_size
|| fstat (console_fd, ) < 0 || st.st_uid != uid)

This all is taken from the commit e9fd11bfcd1dab97e3ba423bcfb8b6ca1088b11c
which is the latest at this moment

It looks to me MC tries inventing its own permission scheme rather than
relying on the system set up.
Consider there is a user in the system who is allowed to read/write and to
do whatever they want with vcs, tty and with whatever files else you may
only wish. root is one obvious candidate but nothing restricts us to set up
another user taking advantaged of  all those system security facilities.
There is a traditional UNIX permission scheme, SeLinux may be involved if
needed. And now comes MC, and introduces a hardcoded/unconfigurable/solid
as a stone requirement for the current user to be the owner of the files.
Why so?

I believe there is case and that code is called to cover it. But
unfortunately I do not see the reason. And this is my question, I would
appreciate if anybody could explain what security issue was addressed here?
In my particular case this code introduces an inconvenience, so I just
removed it and feel total happy without it. But still am a little bit
concerned about possible consequences which I do now understand at the
moment.

My case I mentioned above is as follows:

Log into, say, tty3 as a normal user, say `echo`. The tty3 ownership
changes, and the `echo` user becomes the owner of tty3 which sounds
reasonable.
Now sudo as another user who has all access permissions to tty and vcs, In
my case this is root.
Press Control+O, MC screws up the background shell, the root user sees the
blank screen rather than previously executed commands and MC starts
thinking the terminal is dumb asking to press any key after executing
commands. And this happens for the root user! MC overwrote the root
privileges! Does it sound reasonable to you?

Any explanations are welcome.
Thank you.

--
Konstantin I.,
___
mc-devel mailing list
https://mail.gnome.org/mailman/listinfo/mc-devel