Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.kernel > #60115 > unrolled thread

Bug#889098: enforce fs.protected_hardlinks in sysctl.d by default

Back to article view | Back to linux.debian.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Bug#889098: enforce fs.protected_hardlinks in sysctl.d by default Antoine Beaupré <anarcat@debian.org> - 2018-02-01 23:20 +0100
    Bug#889098: enforce fs.protected_hardlinks in sysctl.d by default Moritz Mühlenhoff <jmm@inutil.org> - 2018-02-02 21:30 +0100
      Bug#889098: enforce fs.protected_hardlinks in sysctl.d by default Antoine Beaupré <anarcat@debian.org> - 2018-02-02 23:30 +0100
        Bug#889098: enforce fs.protected_hardlinks in sysctl.d by default Craig Small <csmall@debian.org> - 2018-02-03 02:00 +0100
          Bug#889098: enforce fs.protected_hardlinks in sysctl.d by default Ben Hutchings <ben@decadent.org.uk> - 2018-02-03 14:20 +0100
      Bug#889098: enforce fs.protected_hardlinks in sysctl.d by default Salvatore Bonaccorso <carnil@debian.org> - 2018-02-03 11:00 +0100
        Bug#889098: enforce fs.protected_hardlinks in sysctl.d by default Antoine Beaupré <anarcat@debian.org> - 2018-02-03 15:50 +0100

#60115 — Bug#889098: enforce fs.protected_hardlinks in sysctl.d by default

Control: retitle 889098 enforce fs.protected_hardlinks in sysctl.d by default

Package: procps
Version: 2:3.3.12-3
Severity: normal
Tags: security

Following the disclosure of CVE-2017-18078, there was an elaborate
discussion on the #debian-devel and #debian-security IRC channels
regarding the scope of the vulnerability. It was then realized that
the impact of this was broader than just systemd: any time a command
like `chown -R` is ran over an untrusted directory, by root, the same
problem occurs.

This is of course mitigated by the fs.protected_hardlinks kernel
configuration, which is enforced through a patch on all the official
Debian kernels distributed by Debian, including in wheezy. See for
example:

https://sources.debian.org/src/linux/3.16.7-ckt20-1+deb8u3/debian/patches/debian/fs-enable-link-security-restrictions-by-default.patch/
https://sources.debian.org/src/linux/4.14.13-1/debian/patches/debian/fs-enable-link-security-restrictions-by-default.patch/

There are, however, people *not* running Debian-built kernels, and
sometimes for good reasons. This is a configuration that we should
still support.

Therefore, it seems to me we should enable this more broadly, for
example in /etc/sysctl.d/protected-hardlinks.conf. Configuring this in
user space is actually what is recommended by Linus Torvalds and the
upstream Linux kernel:

https://github.com/torvalds/linux/commit/561ec64ae67ef25cac8d72bb9c4bfc955edfd415

systemd ships this configuration as well, but this was deliberately
removed from Debian's systemd configuration:

https://salsa.debian.org/systemd-team/systemd/commit/3e1bfe0d84545557d268c1293fff0d5f3db3b5c7

I agree with the above perspective: systemd is not sufficient to
resolve that issue. We still have other init systems and we shouldn't
fix this in systemd, but in a broader package. This is why I am
proposing to fix this in procps, which ultimately owns /etc/sysctl.d/
(and /etc/sysctl.conf).

This is not a strong position: if people think this belongs in systemd
more than procps, or there is some more relevant place this can be
done *by default*, let's do it there and not quibble over that
peculiar bikeshed. :)

I would suggest adding the following configuration:

# Enable hard link protection
fs.protected_hardlinks = 1

Note that the original systemd config also enables softlink
protection:

https://salsa.debian.org/systemd-team/systemd/blob/master/sysctl.d/50-default.conf

I'm not sure if that's also relevant here so I'd keep this to
hardlinks for now to avoid unnecessary debate.

Incidentally, I wonder if we should remove the patch we have on the
Debian kernels to change the defaults, and instead rely on the
sysctl. I have added the kernel team in CC to have their input.

Thanks!

PS: sorry for the duplicate email, I had a copy-paste problem with
reportbug and forgot to re-set a subject.

-- 
We don't need any more heroes.
We just need someone to take out recycling.
                        - Banksy

[toc] | [next] | [standalone]

#60123

Antoine Beaupré wrote:
> There are, however, people *not* running Debian-built kernels, and
> sometimes for good reasons. This is a configuration that we should
> still support.

Is it supported, but it's also clearly documented that people need to
enable this sysctl for custom kernels:
https://www.debian.org/releases/jessie/amd64/release-notes/ch-whats-new.en.html#security

> Incidentally, I wonder if we should remove the patch we have on the
> Debian kernels to change the defaults, and instead rely on the
> sysctl. I have added the kernel team in CC to have their input.

Why revert the kernel? That doesn't buy us anything. It would be
better to ask upstream to revisit this decision (e.g. by contacting
KSPP mailing list). I suppose that SuSE, Ubuntu and Red Hat have
are shipping similar patches/defaults, so it's probably safe to say
that those protections are now the status quo (as opposed to five
years ago when that feature was freshly introduced).

Cheers,
        Moritz

[toc] | [prev] | [next] | [standalone]

#60124

On 2018-02-02 21:25:31, Moritz Mühlenhoff wrote:
> Antoine Beaupré wrote:
>> There are, however, people *not* running Debian-built kernels, and
>> sometimes for good reasons. This is a configuration that we should
>> still support.
>
> Is it supported, but it's also clearly documented that people need to
> enable this sysctl for custom kernels:
> https://www.debian.org/releases/jessie/amd64/release-notes/ch-whats-new.en.html#security

True. I guess what I'm arguing for is to do this explicitly from here
on.

>> Incidentally, I wonder if we should remove the patch we have on the
>> Debian kernels to change the defaults, and instead rely on the
>> sysctl. I have added the kernel team in CC to have their input.
>
> Why revert the kernel? That doesn't buy us anything. It would be
> better to ask upstream to revisit this decision (e.g. by contacting
> KSPP mailing list). I suppose that SuSE, Ubuntu and Red Hat have
> are shipping similar patches/defaults, so it's probably safe to say
> that those protections are now the status quo (as opposed to five
> years ago when that feature was freshly introduced).

It was just an idea: I'm fine with keeping the patch and I think it's a
good idea to enforce this in two places, to keep defense in depth.

I'm not sure I want to go through the emotional trauma of trying to
bring this upstream, unfortunately. ;)

Thanks for the response.

A.

-- 
All governments are run by liars and nothing they say should be
believed.
                       - I. F. Stone

[toc] | [prev] | [next] | [standalone]

#60125

[Multipart message — attachments visible in raw view] — view raw

Hi Antoine (and kernel and security teams),
  Thanks for giving me the background as it's a kernel vulnerability not a
Procps one I wasn't aware of it.

The change to Procps is pretty simple but given that you need to be running
a non Debian kernel without this parameter what's groups' opinion of the
urgency?

I can throw in the sysctl configuration file and upload a release this
weekend if the consensus is it's needed or wait for the next upstream
Procps release which would be a month or so away.

 - Craig

>
>
>

-- 
Craig Small             https://dropbear.xyz/     csmall at : dropbear.xyz
Debian GNU/Linux        https://www.debian.org/   csmall at : debian.org
Mastodon: @smallsees@social.dropbear.xyz             Twitter: @smallsees
GPG fingerprint:      5D2F B320 B825 D939 04D2  0519 3938 F96B DF50 FEA5

[toc] | [prev] | [next] | [standalone]

#60127

[Multipart message — attachments visible in raw view] — view raw

On Sat, 2018-02-03 at 00:45 +0000, Craig Small wrote:
> Hi Antoine (and kernel and security teams),
>   Thanks for giving me the background as it's a kernel vulnerability not a
> Procps one I wasn't aware of it.

It's not a kernel vulnerability, but a class of application
vulnerabilities that the kernel can protect against.

Ben.

> The change to Procps is pretty simple but given that you need to be running
> a non Debian kernel without this parameter what's groups' opinion of the
> urgency?
> 
> I can throw in the sysctl configuration file and upload a release this
> weekend if the consensus is it's needed or wait for the next upstream
> Procps release which would be a month or so away.

-- 
Ben Hutchings
Every program is either trivial or else contains at least one bug

[toc] | [prev] | [next] | [standalone]

#60126

Hi

On Fri, Feb 02, 2018 at 09:25:31PM +0100, Moritz Mühlenhoff wrote:
> Antoine Beaupré wrote:
> > There are, however, people *not* running Debian-built kernels, and
> > sometimes for good reasons. This is a configuration that we should
> > still support.
> 
> Is it supported, but it's also clearly documented that people need to
> enable this sysctl for custom kernels:
> https://www.debian.org/releases/jessie/amd64/release-notes/ch-whats-new.en.html#security

Just to add a note: if procps is as well going to ship this hardening
for fs.protected_hardlinks then I think it would be best to follow the
kernel and do the same for fs.protected_symlinks as well, not only
the fs.protected_hardlinks.

> > Incidentally, I wonder if we should remove the patch we have on the
> > Debian kernels to change the defaults, and instead rely on the
> > sysctl. I have added the kernel team in CC to have their input.
> 
> Why revert the kernel? That doesn't buy us anything. It would be
> better to ask upstream to revisit this decision (e.g. by contacting
> KSPP mailing list). I suppose that SuSE, Ubuntu and Red Hat have
> are shipping similar patches/defaults, so it's probably safe to say
> that those protections are now the status quo (as opposed to five
> years ago when that feature was freshly introduced).

Agreed with you and Ben to actually not revert the sane defaults in
the Debian kernel.

Btw, upstream did initially as well set those, then reverted due to
some userspace programms breaking, they are/were rare, but the rule is
to not break userspace (this was done in the referenced commit, "VFS:
don't do protected {sym,hard}links by default", where it's noted that
it e.g. broke AFD.) 

Regards,
Salvatore

[toc] | [prev] | [next] | [standalone]

#60128

On 2018-02-03 10:54:18, Salvatore Bonaccorso wrote:
> Hi
>
> On Fri, Feb 02, 2018 at 09:25:31PM +0100, Moritz Mühlenhoff wrote:
>> Antoine Beaupré wrote:
>> > There are, however, people *not* running Debian-built kernels, and
>> > sometimes for good reasons. This is a configuration that we should
>> > still support.
>> 
>> Is it supported, but it's also clearly documented that people need to
>> enable this sysctl for custom kernels:
>> https://www.debian.org/releases/jessie/amd64/release-notes/ch-whats-new.en.html#security
>
> Just to add a note: if procps is as well going to ship this hardening
> for fs.protected_hardlinks then I think it would be best to follow the
> kernel and do the same for fs.protected_symlinks as well, not only
> the fs.protected_hardlinks.

Agreed.

>> > Incidentally, I wonder if we should remove the patch we have on the
>> > Debian kernels to change the defaults, and instead rely on the
>> > sysctl. I have added the kernel team in CC to have their input.
>> 
>> Why revert the kernel? That doesn't buy us anything. It would be
>> better to ask upstream to revisit this decision (e.g. by contacting
>> KSPP mailing list). I suppose that SuSE, Ubuntu and Red Hat have
>> are shipping similar patches/defaults, so it's probably safe to say
>> that those protections are now the status quo (as opposed to five
>> years ago when that feature was freshly introduced).
>
> Agreed with you and Ben to actually not revert the sane defaults in
> the Debian kernel.
>
> Btw, upstream did initially as well set those, then reverted due to
> some userspace programms breaking, they are/were rare, but the rule is
> to not break userspace (this was done in the referenced commit, "VFS:
> don't do protected {sym,hard}links by default", where it's noted that
> it e.g. broke AFD.) 

Right. But we've been running with this as default in Debian for a
while. We also have good mechanisms (config file tracking) to allow
custom changes for users that build their own kernels, although that
might need a release notes update or something because that won't be
flagged by those mechanisms.

A.

-- 
Drowning people
Sometimes die
Fighting their rescuers.
                        - Octavia Butler

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.kernel

csiph-web