Raku / old-issue-tracker

Tickets from RT
https://github.com/Raku/old-issue-tracker/issues
2 stars 1 forks source link

(MoarVM) chdir does not respect group reading privilege #5294

Open p6rt opened 8 years ago

p6rt commented 8 years ago

Migrated from rt.perl.org#128062 (status was 'open')

Searchable as RT128062$

p6rt commented 8 years ago

From @molecules

# This is on 64-bit CentOS 7, but I have also observed this behavior on # CentOS 6.7. User is "test" and has its own group called "test". It should # therefore be possible possible to use "chdir" in Perl6 to change to a # directory that has full privileges for its home group "test".

# First we see that "chdir" works when permissions are liberal # The user "test" is not the user owner, but its group does own it. [test@​localhost tests]$ ls -ld path drwxrwxr-x. 2 foo test 22 May 3 10​:13 path [test@​localhost tests]$ perl6 To exit type 'exit' or '^D'

chdir 'path' "/home/test/tests/path".IO chdir '..' "/home/test/tests".IO

# "chdir" doesn't work after removing "other" read/execute privileges

shell('sudo chmod o-rx path'); ... chdir 'path' Failed to change the working directory to '/home/test/tests/path'​: did not pass 'd r' test   in block \ at \ line 1

exit

# But the "test" user does belong to its own "test" group [test@​localhost tests]$ groups test

# This is using Rakudo-Star 2016.04 on MoarVM 2016.04 [test@​localhost tests]$ perl6 -v This is Rakudo version 2016.04 built on MoarVM version 2016.04 implementing Perl 6.c.

p6rt commented 8 years ago

From @geekosaur

On Tue, May 3, 2016 at 12​:01 PM, Christopher Bottoms \< perl6-bugs-followup@​perl.org> wrote​:

Failed to change the working directory to '/home/test/tests/path'​: did not pass 'd r' test

Urgh. Please tell me it is not testing permissions before changing directory; that's pointless and (although I think "safe" here) wide open to race conditions. If you want to check it afterward to try to guess why it failed, fine; but let the OS do the check instead of trying to guess at it.

(And I can't wait to see this fail on a network filesystem which doesn't use Unix permissions​: NFS4, AFS, CIFS, ....)

-- brandon s allbery kf8nh sine nomine associates allbery.b@​gmail.com ballbery@​sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

p6rt commented 8 years ago

The RT System itself - Status changed from 'new' to 'open'

p6rt commented 8 years ago

From @molecules

Urgh. Please tell me it is not testing permissions before changing directory; that's pointless and (although I think "safe" here) wide open to race conditions. If you want to check it afterward to try to guess why it failed, fine; but let the OS do the check instead of trying to guess at it.

Yes, it is doing such checks. I was tempted to work on them myself, until I found out that the functions actually doing the checking are implemented in the virtual machines themselves (e.g. MoarVM, JVM).

Related pages​:

https://github.com/rakudo/rakudo/blob/nom/src/core/IO/Path.pm#L269 https://github.com/rakudo/rakudo/blob/nom/src/core/IO/Path.pm#L570 https://github.com/rakudo/rakudo/blob/nom/src/core/Rakudo/Internals.pm#L1127 https://github.com/rakudo/rakudo/search?q=nqp%3A%3Afilereadable https://github.com/perl6/nqp/search?l=perl6&q=filereadable

p6rt commented 8 years ago

From @molecules

One more related page​: https://github.com/perl6/nqp/blob/master/src/vm/moar/QAST/QASTOperationsMAST.nqp#L2026

p6rt commented 7 years ago

From @molecules

File numbers change, so some of the previous links aren't pointing to the desired lines anymore.

Try these "search" links should be helpful​:

https://github.com/rakudo/rakudo/search?q="did+not+pass+'d+"&type=Code https://github.com/rakudo/rakudo/search?q=FILETEST-R https://github.com/rakudo/rakudo/search?q=nqp%3A%3Afilereadable https://github.com/perl6/nqp/search?l=perl6&q=filereadable

p6rt commented 7 years ago

From @molecules

I found that this is not a problem on Perl 6 with a JVM backend. So this is MoarVM-specific.

On Tue May 03 09​:01​:15 2016, molecules wrote​:

# This is on 64-bit CentOS 7, but I have also observed this behavior on # CentOS 6.7. User is "test" and has its own group called "test". It should # therefore be possible possible to use "chdir" in Perl6 to change to a # directory that has full privileges for its home group "test".

# First we see that "chdir" works when permissions are liberal # The user "test" is not the user owner, but its group does own it. [test@​localhost tests]$ ls -ld path drwxrwxr-x. 2 foo test 22 May 3 10​:13 path [test@​localhost tests]$ perl6 To exit type 'exit' or '^D'

chdir 'path' "/home/test/tests/path".IO chdir '..' "/home/test/tests".IO

# "chdir" doesn't work after removing "other" read/execute privileges

shell('sudo chmod o-rx path'); ... chdir 'path' Failed to change the working directory to '/home/test/tests/path'​: did not pass 'd r' test in block \ at \ line 1

exit

# But the "test" user does belong to its own "test" group [test@​localhost tests]$ groups test

# This is using Rakudo-Star 2016.04 on MoarVM 2016.04 [test@​localhost tests]$ perl6 -v This is Rakudo version 2016.04 built on MoarVM version 2016.04 implementing Perl 6.c.

p6rt commented 7 years ago

From @molecules

This doesn't seem to be a problem with Rakudo 2017.04, so was probably fixed by Zoffix's I/O work (see http://blogs.perl.org/users/zoffix_znet/2017/04/perl-6-io-tpf-grant-monthly-report-april-2017.html). Thanks!

I think that the only thing lacking is a good regression test in roast. I could easily write a Unix-specific test, but I think we want something more general.

On Mon, 31 Oct 2016 08​:08​:15 -0700, molecules wrote​:

I found that this is not a problem on Perl 6 with a JVM backend. So this is MoarVM-specific.

On Tue May 03 09​:01​:15 2016, molecules wrote​:

# This is on 64-bit CentOS 7, but I have also observed this behavior on # CentOS 6.7. User is "test" and has its own group called "test". It should # therefore be possible possible to use "chdir" in Perl6 to change to a # directory that has full privileges for its home group "test".

# First we see that "chdir" works when permissions are liberal # The user "test" is not the user owner, but its group does own it. [test@​localhost tests]$ ls -ld path drwxrwxr-x. 2 foo test 22 May 3 10​:13 path [test@​localhost tests]$ perl6 To exit type 'exit' or '^D'

chdir 'path' "/home/test/tests/path".IO chdir '..' "/home/test/tests".IO

# "chdir" doesn't work after removing "other" read/execute privileges

shell('sudo chmod o-rx path'); ... chdir 'path' Failed to change the working directory to '/home/test/tests/path'​: did not pass 'd r' test in block \ at \ line 1

exit

# But the "test" user does belong to its own "test" group [test@​localhost tests]$ groups test

# This is using Rakudo-Star 2016.04 on MoarVM 2016.04 [test@​localhost tests]$ perl6 -v This is Rakudo version 2016.04 built on MoarVM version 2016.04 implementing Perl 6.c.

p6rt commented 7 years ago

From @zoffixznet

On Mon, 17 Jul 2017 10​:26​:43 -0700, molecules wrote​:

This doesn't seem to be a problem with Rakudo 2017.04, so was probably fixed by Zoffix's I/O work (see http://blogs.perl.org/users/zoffix_znet/2017/04/perl-6-io-tpf-grant- monthly-report-april-2017.html). Thanks!

I think that the only thing lacking is a good regression test in roast. I could easily write a Unix-specific test, but I think we want something more general.

What was "fixed" is chdir by default only does the `.d` test now (it used to do `.r` test too). If that's what was needed, then there's a hole battery[^1] of tests covering this behaviour.

But I suspect the old behaviour is still there when doing `chdir :d, :r, 'some dir'`, so it does the `.r` too. THAT should respect group privileges, right?

[1] https://github.com/perl6/roast/blob/9b51341b6c77bfe6f0ce045431942eadc4301d78/S32-io/chdir.t#L61-L140

p6rt commented 7 years ago

From @geekosaur

It should not be testing, it should just try to do the operation and complain after if it fails. Race conditions should not be a language 'feature'.

On Mon, Jul 17, 2017 at 6​:02 PM, Zoffix Znet via RT \< perl6-bugs-followup@​perl.org> wrote​:

On Mon, 17 Jul 2017 10​:26​:43 -0700, molecules wrote​:

This doesn't seem to be a problem with Rakudo 2017.04, so was probably fixed by Zoffix's I/O work (see http://blogs.perl.org/users/zoffix_znet/2017/04/perl-6-io-tpf-grant- monthly-report-april-2017.html). Thanks!

I think that the only thing lacking is a good regression test in roast. I could easily write a Unix-specific test, but I think we want something more general.

What was "fixed" is chdir by default only does the `.d` test now (it used to do `.r` test too). If that's what was needed, then there's a hole battery[^1] of tests covering this behaviour.

But I suspect the old behaviour is still there when doing `chdir :d, :r, 'some dir'`, so it does the `.r` too. THAT should respect group privileges, right?

[1] https://github.com/perl6/roast/blob/9b51341b6c77bfe6f0ce045431942e adc4301d78/S32-io/chdir.t#L61-L140

-- brandon s allbery kf8nh sine nomine associates allbery.b@​gmail.com ballbery@​sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

p6rt commented 7 years ago

From @zoffixznet

On Mon, 17 Jul 2017 16​:12​:05 -0700, allbery.b@​gmail.com wrote​:

It should not be testing, it should just try to do the operation and complain after if it fails. Race conditions should not be a language 'feature'.

We're talking about &chdir() not &*chdir(). Aside from setting $*CWD, testing is all it does and without testing, it'll never fail.