[PATCH] symlink.7: expound upon fs.protected_symlinks

[PATCH] symlink.7: expound upon fs.protected_symlinks

[наб]

[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]

This is on by default in Debian, maybe the next reader won't spend an
hour tracing the kernel.

Closes: https://bugs.debian.org/1033477
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 man7/symlink.7 | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/man7/symlink.7 b/man7/symlink.7
index 77fefb743..550b60486 100644
--- a/man7/symlink.7
+++ b/man7/symlink.7
@@ -84,10 +84,23 @@ magic links have been used as attack vectors in various exploits.
 The owner and group of an existing symbolic link can be changed
 using
 .BR lchown (2).
-The only time that the ownership of a symbolic link matters is
-when the link is being removed or renamed in a directory that
-has the sticky bit set (see
-.BR stat (2)).
+Only when a symbolic link is in a sticky directory (see
+.BR stat (2))
+does its ownership matter\[em]deletions and renames are subject
+to standard semantics.
+.\" Documentation/admin-guide/sysctl/fs.rst:
+.\" fs/namei.c#may_follow_link()
+Additionally, if the
+.I fs.protected_symlinks
+sysctl is set, a symbolic link may only be followed if:
+.br
+\[bu] not in a sticky, world-writable
+.RB ( o+wt )
+directory,
+.br
+\[bu] owned by the user which follows it, or
+.br
+\[bu] owned by the same user which owns the directory it resides in.
 .PP
 The last access and last modification timestamps
 of a symbolic link can be changed using
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] symlink.7: expound upon fs.protected_symlinks

[Jakub Wilk]

* наб <nabijaczleweli@nabijaczleweli.xyz>, 2023-03-25 22:00:
>-The only time that the ownership of a symbolic link matters is
>-when the link is being removed or renamed in a directory that
>-has the sticky bit set (see
>-.BR stat (2)).

Wait, but stat.2 doesn't say anything about the sticky bit?

Oh, apparently this info was removed in e8ff4f53ab9a7cbd ("Remove 
information migrated to inode(7) page").

>+Only when a symbolic link is in a sticky directory (see
>+.BR stat (2))
>+does its ownership matter\[em]deletions and renames are subject
>+to standard semantics.

That's weird phrasing. What was wrong with the original sentence?

>+Additionally, if the
>+.I fs.protected_symlinks
>+sysctl is set, a symbolic link may only be followed if:

FWIW, this is also documented in proc.5; not sure if we need it in two 
places.

>+\[bu] owned by the user which follows it, or

s/which/who/?

>+\[bu] owned by the same user which owns the directory it resides in.

Ditto.

-- 
Jakub Wilk

[PATCH v2] symlink.7: cross-link to proc.5 for fs.protected_symlinks

[наб]

[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]

This is on by default in Debian, maybe the next reader won't spend an
hour tracing the kernel

Fixes: e8ff4f53ab9a7cbd ("Remove information migrated to inode(7) page")
Closes: https://bugs.debian.org/1033477
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
This definitely needs to be referenced here, because "The only time that
the ownership of a symbolic link matters is when the link is being
removed or renamed in a directory that has the sticky bit set" is an
abject lie, especially since Debian ships with fs.protected_symlinks=1;
the minimum here is to cross-ref to an extended description.

I straight-up didn't know proc(5) listed sysctls, and I looked!
(Well, defined as "apropos sysctl" didn't list anything in particular,
 and sysctl(2) has an unannotated proc(5) in SEE ALSO.)

It'd be nice if, idk, the proc.5 SH were extended with "process
information, system information, and sysctl pseudo-filesystem" or
whatever, or there were a sysctl.[47] alias for proc.5,
because as it stands, I would never have guesssed there's a listing of
sysctls in proc(5).

 man7/symlink.7 | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/man7/symlink.7 b/man7/symlink.7
index 77fefb743..4403f6214 100644
--- a/man7/symlink.7
+++ b/man7/symlink.7
@@ -84,10 +84,14 @@ magic links have been used as attack vectors in various exploits.
 The owner and group of an existing symbolic link can be changed
 using
 .BR lchown (2).
-The only time that the ownership of a symbolic link matters is
+The ownership of a symbolic link matters
 when the link is being removed or renamed in a directory that
 has the sticky bit set (see
-.BR stat (2)).
+.BR inode (7)),
+and when the
+.I fs.protected_symlinks
+sysctl is set (see
+.BR proc (5)).
 .PP
 The last access and last modification timestamps
 of a symbolic link can be changed using
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] symlink.7: cross-link to proc.5 for fs.protected_symlinks

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 2240 bytes --]

Hi!

On 3/27/23 14:29, наб wrote:
> This is on by default in Debian, maybe the next reader won't spend an
> hour tracing the kernel
> 
> Fixes: e8ff4f53ab9a7cbd ("Remove information migrated to inode(7) page")
> Closes: https://bugs.debian.org/1033477
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Patch applied.

> ---
> This definitely needs to be referenced here, because "The only time that
> the ownership of a symbolic link matters is when the link is being
> removed or renamed in a directory that has the sticky bit set" is an
> abject lie, especially since Debian ships with fs.protected_symlinks=1;
> the minimum here is to cross-ref to an extended description.
> 
> I straight-up didn't know proc(5) listed sysctls, and I looked!
> (Well, defined as "apropos sysctl" didn't list anything in particular,
>  and sysctl(2) has an unannotated proc(5) in SEE ALSO.)
> 
> It'd be nice if, idk, the proc.5 SH were extended with "process
> information, system information, and sysctl pseudo-filesystem" or
> whatever, or there were a sysctl.[47] alias for proc.5,
> because as it stands, I would never have guesssed there's a listing of
> sysctls in proc(5).

Feel free to send a patch for it :)

Cheers,
Alex

> 
>  man7/symlink.7 | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/man7/symlink.7 b/man7/symlink.7
> index 77fefb743..4403f6214 100644
> --- a/man7/symlink.7
> +++ b/man7/symlink.7
> @@ -84,10 +84,14 @@ magic links have been used as attack vectors in various exploits.
>  The owner and group of an existing symbolic link can be changed
>  using
>  .BR lchown (2).
> -The only time that the ownership of a symbolic link matters is
> +The ownership of a symbolic link matters
>  when the link is being removed or renamed in a directory that
>  has the sticky bit set (see
> -.BR stat (2)).
> +.BR inode (7)),
> +and when the
> +.I fs.protected_symlinks
> +sysctl is set (see
> +.BR proc (5)).
>  .PP
>  The last access and last modification timestamps
>  of a symbolic link can be changed using

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 1/2] proc.5: add "system information, and sysctl" to .SH

[наб]

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]

procfs hosts a whole host of information about the system,
as well as sysctls; proc(5) hosts a description of a lot of sysctls,
and at present there's no way to find that out.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 man5/proc.5 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man5/proc.5 b/man5/proc.5
index 39c7eb8ab..e2e3c3409 100644
--- a/man5/proc.5
+++ b/man5/proc.5
@@ -36,7 +36,7 @@
 .\"
 .TH proc 5 (date) "Linux man-pages (unreleased)"
 .SH NAME
-proc \- process information pseudo-filesystem
+proc \- process information, system information, and sysctl pseudo-filesystem
 .SH DESCRIPTION
 The
 .B proc
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] proc.5: add "system information, and sysctl" to .SH

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 1029 bytes --]

By SH you mean NAME section.  SH is just Section Heading :)

On 4/1/23 00:04, наб wrote:
> procfs hosts a whole host of information about the system,
> as well as sysctls; proc(5) hosts a description of a lot of sysctls,
> and at present there's no way to find that out.
> 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

I'll give a day or so to this patch, in case anyone wants to comment.

Thanks,
Alex

> ---
>  man5/proc.5 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man5/proc.5 b/man5/proc.5
> index 39c7eb8ab..e2e3c3409 100644
> --- a/man5/proc.5
> +++ b/man5/proc.5
> @@ -36,7 +36,7 @@
>  .\"
>  .TH proc 5 (date) "Linux man-pages (unreleased)"
>  .SH NAME
> -proc \- process information pseudo-filesystem
> +proc \- process information, system information, and sysctl pseudo-filesystem
>  .SH DESCRIPTION
>  The
>  .B proc

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] proc.5: add "system information, and sysctl" to .SH

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 916 bytes --]

Hi!

On 4/1/23 00:04, наб wrote:
> procfs hosts a whole host of information about the system,
> as well as sysctls; proc(5) hosts a description of a lot of sysctls,
> and at present there's no way to find that out.
> 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Patch applied.  Thanks,
Alex

> ---
>  man5/proc.5 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man5/proc.5 b/man5/proc.5
> index 39c7eb8ab..e2e3c3409 100644
> --- a/man5/proc.5
> +++ b/man5/proc.5
> @@ -36,7 +36,7 @@
>  .\"
>  .TH proc 5 (date) "Linux man-pages (unreleased)"
>  .SH NAME
> -proc \- process information pseudo-filesystem
> +proc \- process information, system information, and sysctl pseudo-filesystem
>  .SH DESCRIPTION
>  The
>  .B proc

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/2] sysctl.7: create as .so man5/proc.5

[наб]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

This way "whatis sysctl" shows the list of sysctls and documentation,
in addition to the syscall and program. Indeed, the latter two aren't
all that useful without the former.

Not a single concrete file, and it's more of an interface => goes in 7.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 man7/sysctl.7 | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 man7/sysctl.7

diff --git a/man7/sysctl.7 b/man7/sysctl.7
new file mode 100644
index 000000000..d8be74ac8
--- /dev/null
+++ b/man7/sysctl.7
@@ -0,0 +1 @@
+.so man5/proc.5
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] sysctl.7: create as .so man5/proc.5

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 1096 bytes --]

Hi!

On 4/1/23 00:04, наб wrote:
> This way "whatis sysctl" shows the list of sysctls and documentation,
> in addition to the syscall and program. Indeed, the latter two aren't
> all that useful without the former.

I wonder if proc.5 should be split.  To me, it's a huge page where
it's hard to find what you want, unless you already know what to look
for.  I haven't had to use it often, which is why I didn't care enough
to do it myself, but if anyone thinks it's a good idea, this is an
invitation to do it :)

Cheers,
Alex

> 
> Not a single concrete file, and it's more of an interface => goes in 7.
> 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> ---
>  man7/sysctl.7 | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 man7/sysctl.7
> 
> diff --git a/man7/sysctl.7 b/man7/sysctl.7
> new file mode 100644
> index 000000000..d8be74ac8
> --- /dev/null
> +++ b/man7/sysctl.7
> @@ -0,0 +1 @@
> +.so man5/proc.5

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] sysctl.7: create as .so man5/proc.5

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 1018 bytes --]

Hi наб,

On 4/1/23 00:04, наб wrote:
> This way "whatis sysctl" shows the list of sysctls and documentation,
> in addition to the syscall and program. Indeed, the latter two aren't
> all that useful without the former.
> 
> Not a single concrete file, and it's more of an interface => goes in 7.
> 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> ---
>  man7/sysctl.7 | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 man7/sysctl.7
> 
> diff --git a/man7/sysctl.7 b/man7/sysctl.7
> new file mode 100644
> index 000000000..d8be74ac8
> --- /dev/null
> +++ b/man7/sysctl.7
> @@ -0,0 +1 @@
> +.so man5/proc.5

How about a SEE ALSO reference in the sysctl(8) page instead?  I had
a look at that page, and it hints that you should look at proc(5)
--it mentions /proc/sys/, although there's no explicit reference to
proc(5)--.

Cheers,
Alex


-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] sysctl.7: create as .so man5/proc.5

[наб]

[-- Attachment #1: Type: text/plain, Size: 699 bytes --]

On Sun, Apr 02, 2023 at 01:04:37AM +0200, Alejandro Colomar wrote:
> On 4/1/23 00:04, наб wrote:
> > This way "whatis sysctl" shows the list of sysctls and documentation,
> > in addition to the syscall and program. Indeed, the latter two aren't
> > all that useful without the former.
> > 
> > Not a single concrete file, and it's more of an interface => goes in 7.
> How about a SEE ALSO reference in the sysctl(8) page instead?  I had
> a look at that page, and it hints that you should look at proc(5)
> --it mentions /proc/sys/, although there's no explicit reference to
> proc(5)--.
Sure, forwarded as
  https://gitlab.com/procps-ng/procps/-/merge_requests/177

Best,
наб

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] sysctl.7: create as .so man5/proc.5

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 881 bytes --]

On 4/2/23 01:26, наб wrote:
> On Sun, Apr 02, 2023 at 01:04:37AM +0200, Alejandro Colomar wrote:
>> On 4/1/23 00:04, наб wrote:
>>> This way "whatis sysctl" shows the list of sysctls and documentation,
>>> in addition to the syscall and program. Indeed, the latter two aren't
>>> all that useful without the former.
>>>
>>> Not a single concrete file, and it's more of an interface => goes in 7.
>> How about a SEE ALSO reference in the sysctl(8) page instead?  I had
>> a look at that page, and it hints that you should look at proc(5)
>> --it mentions /proc/sys/, although there's no explicit reference to
>> proc(5)--.
> Sure, forwarded as
>   https://gitlab.com/procps-ng/procps/-/merge_requests/177

Nice :)

Best,
Alex

> 
> Best,
> наб

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]