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

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

Open
p6rt opened this issue May 3, 2016 · 11 comments
Open

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

p6rt opened this issue May 3, 2016 · 11 comments

Comments

@p6rt
Copy link

p6rt commented May 3, 2016

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

Searchable as RT128062$

@p6rt
Copy link
Author

p6rt commented May 3, 2016

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 <unit> at <unknown file> 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
Copy link
Author

p6rt commented May 3, 2016

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
Copy link
Author

p6rt commented May 3, 2016

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

@p6rt
Copy link
Author

p6rt commented May 3, 2016

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
Copy link
Author

p6rt commented May 3, 2016

@p6rt
Copy link
Author

p6rt commented Oct 31, 2016

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
Copy link
Author

p6rt commented Oct 31, 2016

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 <unit> at <unknown file> 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
Copy link
Author

p6rt commented Jul 17, 2017

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 <unit> at <unknown file> 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
Copy link
Author

p6rt commented Jul 17, 2017

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
Copy link
Author

p6rt commented Jul 17, 2017

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
Copy link
Author

p6rt commented Jul 17, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant