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
x/sys/unix: fchmodat under linux silently fails when passing flags #20130
Comments
The cause of this the prototype for the syscall generator is wrong as while the libc version supports flags the syscall version doesn't: |
FYI, there is a patch for adding a fchmodat2() syscall to the Linux kernel which will support a flags parameter with |
CL https://golang.org/cl/46474 mentions this issue. |
Currently Linux' fchmodat(2) syscall implementation doesn't support the flags parameter (though it might in future versions [1]). Fchmodat in x/sys/unix takes the parameter and (wrongly) passes it on to the syscall which will ignore it. According to the POSIX.1-2008 manual page [2], AT_SYMLINK_NOFOLLOW is the only valid value for the flags parameter and EOPNOTSUPP should be returned in case changing the mode of a symbolic link is not supported by the underlying system. EINVAL should be returned for any other value of the flags parameter. [1] https://patchwork.kernel.org/patch/9596301/ [2] http://pubs.opengroup.org/onlinepubs/9699919799/functions/chmod.html Change the Fchmodat implementation accordingly and also add the corresponding test. Fixes golang/go#20130 Change-Id: I62e677e6674d3702eaf388af0ac3d7e623a35c24 Reviewed-on: https://go-review.googlesource.com/46474 Reviewed-by: Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
When a volume had a symlink, the symlink would be resolved outside the volume during UID/GID translation. The os.Walk used during translation would call Lstat on each object, and this is what is used during translation. This means that the Mode() would be for the symlink itself (which is 777). However, the chmod call would act through the symlink, changing the permissions on the underlying object. This meant that any symlink in the volume would result in a chmod 0777 call directed at a file on the host system itself, allowing a hostile volume to change permissions on important system files (/usr/bin, /etc/passwd, etc) by creating those files. The most correct fix here would be a lchmod call, but that doesn't exist because POSIX assumes symlinks don't have independent permissions (and so are always 0777). This means this fix should be correct on platforms for which this assumption holds; however, this assumption does not hold on OSX. I am unaware of any high-level go API that solves that problem, which means a more nuanced fix here would require a direct syscall on OSX (see golang/go#20130 for some discussion).
When a volume had a symlink, the symlink would be resolved outside the volume during UID/GID translation. The os.Walk used during translation would call Lstat on each object, and this is what is used during translation. This means that the Mode() would be for the symlink itself (which is 777). However, the chmod call would act through the symlink, changing the permissions on the underlying object. This meant that any symlink in the volume would result in a chmod 0777 call directed at a file on the host system itself, allowing a hostile volume to change permissions on important system files (/usr/bin, /etc/passwd, etc) by creating those files. The most correct fix here would be a lchmod call, but that doesn't exist because POSIX assumes symlinks don't have independent permissions (and so are always 0777). This means this fix should be correct on platforms for which this assumption holds; however, this assumption does not hold on OSX. I am unaware of any high-level go API that solves that problem, which means a more nuanced fix here would require a direct syscall on OSX (see golang/go#20130 for some discussion). concourse/concourse#1736
When a volume had a symlink, the symlink would be resolved outside the volume during UID/GID translation. The os.Walk used during translation would call Lstat on each object, and this is what is used during translation. This means that the Mode() would be for the symlink itself (which is 777). However, the chmod call would act through the symlink, changing the permissions on the underlying object. This meant that any symlink in the volume would result in a chmod 0777 call directed at a file on the host system itself, allowing a hostile volume to change permissions on important system files (/usr/bin, /etc/passwd, etc) by creating those files. The most correct fix here would be a lchmod call, but that doesn't exist because POSIX assumes symlinks don't have independent permissions (and so are always 0777). This means this fix should be correct on platforms for which this assumption holds; however, this assumption does not hold on OSX. I am unaware of any high-level go API that solves that problem, which means a more nuanced fix here would require a direct syscall on OSX (see golang/go#20130 for some discussion). concourse/concourse#1736
Change https://golang.org/cl/118658 mentions this issue: |
As mentioned in #25845, port CL 46474 from golang.org/x/sys/unix to the syscall package. Currently Linux' fchmodat(2) syscall implementation doesn't support the flags parameter (though it might in future versions [1]). Fchmodat in the syscall package takes the parameter and (wrongly) passes it on to the syscall which will ignore it. According to the POSIX.1-2008 manual page [2], AT_SYMLINK_NOFOLLOW is the only valid value for the flags parameter and EOPNOTSUPP should be returned in case changing the mode of a symbolic link is not supported by the underlying system. EINVAL should be returned for any other value of the flags parameter. [1] https://patchwork.kernel.org/patch/9596301/ [2] http://pubs.opengroup.org/onlinepubs/9699919799/functions/chmod.html Updates #20130 Updates #25845 Change-Id: I1021dd0e6a4f4cb3557cb1c1b34dd618c378cda6 Reviewed-on: https://go-review.googlesource.com/118658 Reviewed-by: Ian Lance Taylor <iant@golang.org>
As mentioned in golang#25845, port CL 46474 from golang.org/x/sys/unix to the syscall package. Currently Linux' fchmodat(2) syscall implementation doesn't support the flags parameter (though it might in future versions [1]). Fchmodat in the syscall package takes the parameter and (wrongly) passes it on to the syscall which will ignore it. According to the POSIX.1-2008 manual page [2], AT_SYMLINK_NOFOLLOW is the only valid value for the flags parameter and EOPNOTSUPP should be returned in case changing the mode of a symbolic link is not supported by the underlying system. EINVAL should be returned for any other value of the flags parameter. [1] https://patchwork.kernel.org/patch/9596301/ [2] http://pubs.opengroup.org/onlinepubs/9699919799/functions/chmod.html Updates golang#20130 Updates golang#25845 Change-Id: I1021dd0e6a4f4cb3557cb1c1b34dd618c378cda6 Reviewed-on: https://go-review.googlesource.com/118658 Reviewed-by: Ian Lance Taylor <iant@golang.org>
These were silently ignored until now (!) but are rejected by Go stdlib master since yesterday or something. Drop the flags so the tests work again, until we figure out a better solution. golang/go#20130
These were silently ignored until now (!) but are rejected by Go 1.11 stdlib. Drop the flags so the tests work again, until we figure out a better solution. golang/go#20130
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version
)?go1.8.1
What operating system and processor architecture are you using (
go env
)?linux amd64
What did you do?
Using
syscall.Fchmodat
and setting theAT_SYMLINK_NOFOLLOW
flag, e.g.What did you expect to see?
Expected to see an error, as according to the kernel source the syscall doesn't accept the flags parameter and the glibc source returns an ENOTSUP.
What did you see instead?
Instead, the call silently drops the flags parameter and follows the symlink, applying the fchmodat to the destination (or failing if the destination file can not be altered).
The text was updated successfully, but these errors were encountered: