Re: Enabling branch protection on amd64 and arm64
Hi Guillem, On Thu, Aug 31, 2023 at 02:12:51AM +0200, Guillem Jover wrote: > So this happened, and Johannes reported that this seems to be breaking > cross-building. :( > > The problem, which is in fact not new, but is made way more evident > now, is that the flags used are accepted only per arch, so when > passing for example CFLAGS (the host ones) into CC_FOR_BUILD, then > that one will not know about them and fail. (We have had this problem > up to now as we set flags per arch as some are broken in some arches, > but it being a problem depends on the host and build arches involved.) I agree that the problem is not new. In general, stuff we compile with the build architecture compiler is not installed into any .deb. We only build such things for running them during build. Most flags do not influence the behaviour of the resulting executables. tim64 may be a notable exception here. So for a lot of cases, you can just pretend that for build tools you don't need any Debian-specified compiler flags. All you need to do here is deleting the flags when invoking build tools. I've hit such cases in the past and done just that. > I'm thinking about uploading later today a workaround to disable these > flags for now when cross-building. And then for the next release after > that support for _FOR_BUILD which can then take into account > these arch differences. I think some upstream code can already make > use of these, but this might need going over packaging or upstream > build systems to adapt/fix stuff. :/ I'd rather not. These have always been bugs and some of them have patches or have been uploaded. I don't expect them to be that many. It's rather few packages that use the build architecture compiler at all. Of those, a portion manages to keep host flags out. Let's just fix the others. In case you really want to pass the correct flags, you may use CFLAGS_FOR_BUILD=$(dpkg-architecture -a$(DEB_BUILD_ARCH) -f -c dpkg-buildflags --get CFLAGS) Guillem pointed out that these are still affected by DEB__MAINT_ and DEB__, so we should eventually rely on dpkg providing more support here, but I don't see the lack of such support as a blocker here. > And until that's done I don't think the workaround can be lifted, > and cross-compiling will generate different binaries than > non-cross-compiling. Another option would be to revert this change > until we can add it safely, but that would also be unfortunate. > OTOH, upstream code that uses stuff like CFLAGS with things like > CC_FOR_BUILD are already broken in regards cross-building, so perhaps > this can be an opportunity to flush them out? Such cross vs native differences are very bad from my point of view, because we have very little tools to detect them. It's an area where we lack QA. Let's not make that worse. In the grand scheme of things breaking cross builds, I think this is a drop in the bucket. Helmut
Re: Bug#1021292: Enabling branch protection on amd64 and arm64
Hi Guillem, On 2023-08-31 02:12, Guillem Jover wrote: > So this happened, and Johannes reported that this seems to be breaking > cross-building. :( > > The problem, which is in fact not new, but is made way more evident > now, is that the flags used are accepted only per arch, so when > passing for example CFLAGS (the host ones) into CC_FOR_BUILD, then > that one will not know about them and fail. [...] > I'm thinking about uploading later today a workaround to disable these > flags for now when cross-building. Yeah this sounds like a good idea, thanks very much for taking care of the issue.
Re: Enabling branch protection on amd64 and arm64
Hi! On Sun, 2023-08-27 at 12:51:53 +0200, Guillem Jover wrote: > On Tue, 2023-06-27 at 16:09:40 +0100, Wookey wrote: > > OK. We're all agreed on that then. Guillem can stick it in the next > > dpkg upload. So this happened, and Johannes reported that this seems to be breaking cross-building. :( The problem, which is in fact not new, but is made way more evident now, is that the flags used are accepted only per arch, so when passing for example CFLAGS (the host ones) into CC_FOR_BUILD, then that one will not know about them and fail. (We have had this problem up to now as we set flags per arch as some are broken in some arches, but it being a problem depends on the host and build arches involved.) I'm thinking about uploading later today a workaround to disable these flags for now when cross-building. And then for the next release after that support for _FOR_BUILD which can then take into account these arch differences. I think some upstream code can already make use of these, but this might need going over packaging or upstream build systems to adapt/fix stuff. :/ And until that's done I don't think the workaround can be lifted, and cross-compiling will generate different binaries than non-cross-compiling. Another option would be to revert this change until we can add it safely, but that would also be unfortunate. OTOH, upstream code that uses stuff like CFLAGS with things like CC_FOR_BUILD are already broken in regards cross-building, so perhaps this can be an opportunity to flush them out? Thanks, Guillem
Re: Enabling branch protection on amd64 and arm64
Hi! On Tue, 2023-06-27 at 16:09:40 +0100, Wookey wrote: > On 2023-06-27 16:58 +0200, Moritz Mühlenhoff wrote: > > Am Wed, Jun 21, 2023 at 05:41:36PM +0200 schrieb Emanuele Rocca: > > > On 2022-10-26 08:20, Moritz Mühlenhoff wrote: > > > > I think this should rather be applied early after the Bookworm > > > > release (and ideally we can also finish off the necessary testing > > > > and add -fstack-clash-protection at least for amd64 and other archs > > > > which are ready for it (#918914)). > > > > > > Can we go ahead with the dpkg patch now, any specific tests you had in > > > mind before applying it? > > > > Note that I'm not the one driving this change (I'll start a separate > > thread for -fstack-clash-protection in the next days), but the original > > request was from Wookey. > > > Personally I think now at the beginning of the new development cycle > > is the ideal time to start this. > > OK. We're all agreed on that then. Guillem can stick it in the next > dpkg upload. Right, I've queued the patch for 1.22.0, which I'm planning to upload around today/tomorrow. > I've not yet grokked James' comments above either which maybe imply > adjustments to the patch? That's x86 stuff which is not my area of > expertise. From a quick skim this seems most relevant for code that controls the CPU state such as the kernel. I think we can go as is, and can amend the flags if needed. Thanks, Guillem
Re: Enabling branch protection on amd64 and arm64
On 2023-06-27 16:58 +0200, Moritz Mühlenhoff wrote: > Am Wed, Jun 21, 2023 at 05:41:36PM +0200 schrieb Emanuele Rocca: > > Hey Moritz, > > > > On 2022-10-26 08:20, Moritz Mühlenhoff wrote: > > > I think this should rather be applied early after the Bookworm > > > release (and ideally we can also finish off the necessary testing > > > and add -fstack-clash-protection at least for amd64 and other archs > > > which are ready for it (#918914)). > > > > Can we go ahead with the dpkg patch now, any specific tests you had in > > mind before applying it? > > Note that I'm not the one driving this change (I'll start a separate > thread for -fstack-clash-protection in the next days), but the original > request was from Wookey. > Personally I think now at the beginning of the new development cycle > is the ideal time to start this. OK. We're all agreed on that then. Guillem can stick it in the next dpkg upload. I've not yet grokked James' comments above either which maybe imply adjustments to the patch? That's x86 stuff which is not my area of expertise. Wookey -- Principal hats: Debian, Wookware, ARM http://wookware.org/ signature.asc Description: PGP signature
Re: Enabling branch protection on amd64 and arm64
Am Wed, Jun 21, 2023 at 05:41:36PM +0200 schrieb Emanuele Rocca: > Hey Moritz, > > On 2022-10-26 08:20, Moritz Mühlenhoff wrote: > > I think this should rather be applied early after the Bookworm > > release (and ideally we can also finish off the necessary testing > > and add -fstack-clash-protection at least for amd64 and other archs > > which are ready for it (#918914)). > > Can we go ahead with the dpkg patch now, any specific tests you had in > mind before applying it? Note that I'm not the one driving this change (I'll start a separate thread for -fstack-clash-protection in the next days), but the original request was from Wookey. Personally I think now at the beginning of the new development cycle is the ideal time to start this. Cheers, Moritz
Re: Enabling branch protection on amd64 and arm64
Hey Moritz, On 2022-10-26 08:20, Moritz Mühlenhoff wrote: > I think this should rather be applied early after the Bookworm > release (and ideally we can also finish off the necessary testing > and add -fstack-clash-protection at least for amd64 and other archs > which are ready for it (#918914)). Can we go ahead with the dpkg patch now, any specific tests you had in mind before applying it? ema
Re: Enabling branch protection on amd64 and arm64
On Nov 01, Sebastian Ramacher wrote: > > this change is only targeted at two archs, which I'd hope could cope with > > it. > If we ignore/break MA: same co-installability, sure. Sure, but this means that a much smaller subset of packages will need to be rebuilt on all architectures. -- ciao, Marco signature.asc Description: PGP signature
Re: Enabling branch protection on amd64 and arm64
On Tue, Nov 01, 2022 at 01:09:39AM +0100, Sebastian Ramacher wrote: > > this change is only targeted at two archs, which I'd hope could cope with > > it. > If we ignore/break MA: same co-installability, sure. point taken, thanks! -- cheers, Holger ⢀⣴⠾⠻⢶⣦⠀ ⣾⠁⢠⠒⠀⣿⡁ holger@(debian|reproducible-builds|layer-acht).org ⢿⡄⠘⠷⠚⠋⠀ OpenPGP: B8BF54137B09D35CF026FE9D 091AB856069AAA1C ⠈⠳⣄ Punk ist nicht tot. Punk trägt Maske, ist solidarisch und schützt sich und andere. (@Kreuzpirat) signature.asc Description: PGP signature
Re: Enabling branch protection on amd64 and arm64
On 2022-10-31 23:28:21 +, Holger Levsen wrote: > On Thu, Oct 27, 2022 at 12:27:12AM +0200, Sebastian Ramacher wrote: > > Some of the architectures already have a hard time keeping up with the > > normal load. > > this change is only targeted at two archs, which I'd hope could cope with it. If we ignore/break MA: same co-installability, sure. Cheers -- Sebastian Ramacher
Re: Enabling branch protection on amd64 and arm64
On Thu, Oct 27, 2022 at 12:27:12AM +0200, Sebastian Ramacher wrote: > Some of the architectures already have a hard time keeping up with the > normal load. this change is only targeted at two archs, which I'd hope could cope with it. > Enabling these flags as soon as the trixie release cycle starts, sounds > like a better idea. Adoption of these flags will then naturally progress > and before the trixie release we can rebuild whatever remains. even^walso if this is done only for the trixie cycle I think it would be good to binNMU all affected packages, which I would guess to be around 25-33% of the archive. because else we cannot really say whether we have enabled this feature archive wide and whether it works/builds ;) summary: I think the next step should be calculating how many packages are affected. because the above 25-33% is my guestimate only. -- cheers, Holger ⢀⣴⠾⠻⢶⣦⠀ ⣾⠁⢠⠒⠀⣿⡁ holger@(debian|reproducible-builds|layer-acht).org ⢿⡄⠘⠷⠚⠋⠀ OpenPGP: B8BF54137B09D35CF026FE9D 091AB856069AAA1C ⠈⠳⣄ https://showyourstripes.info signature.asc Description: PGP signature
Re: Enabling branch protection on amd64 and arm64
On 2022-10-26 20:20:48 +0200, Moritz Mühlenhoff wrote: > Wookey wrote: > > So the immediate issue now is whether or not to enable this by default > > in bookworm? > > The majority of packages will not be rebuilt until the release, so > if we add this now it means that packages pick up the change when > they are rebuilt in stable via a security update or point release. > That's not very appealing, independent of the supposed low risk > factor. > > I think this should rather be applied early after the Bookworm > release (and ideally we can also finish off the necessary testing > and add -fstack-clash-protection at least for amd64 and other archs > which are ready for it (#918914)). I agree that it's too late for bookworm. If we'd enable it now, we'd want to rebuild the archive before releasing bookworm to avoid surprises with any security or stable updates in the future. Rebuilding the world, however, seems unrealistic at this stage. Some of the architectures already have a hard time keeping up with the normal load. Enabling these flags as soon as the trixie release cycle starts, sounds like a better idea. Adoption of these flags will then naturally progress and before the trixie release we can rebuild whatever remains. Cheers -- Sebastian Ramacher
Re: Enabling branch protection on amd64 and arm64
On 2022-10-26 14:23 -0500, Richard Laager wrote: > > How hard would it be to rebuild everything? > > I don't actually know what facilities Debian has for that. Would it be a > binNMU of everything? It would. We don't do that. In the past it would have wildly overloaded our buildds. Such a thing may be more tractable these days in terms of available build resources on most arches, but it's still a very big deal, and this change certainly doesn't warrant it. Wookey -- Principal hats: Debian, Wookware, ARM http://wookware.org/ signature.asc Description: PGP signature
Re: Enabling branch protection on amd64 and arm64
On 10/26/22 13:20, Moritz Mühlenhoff wrote: Wookey wrote: So the immediate issue now is whether or not to enable this by default in bookworm? The majority of packages will not be rebuilt until the release How hard would it be to rebuild everything? I don't actually know what facilities Debian has for that. Would it be a binNMU of everything? -- Richard OpenPGP_signature Description: OpenPGP digital signature
Re: Enabling branch protection on amd64 and arm64
Wookey wrote: > So the immediate issue now is whether or not to enable this by default > in bookworm? The majority of packages will not be rebuilt until the release, so if we add this now it means that packages pick up the change when they are rebuilt in stable via a security update or point release. That's not very appealing, independent of the supposed low risk factor. I think this should rather be applied early after the Bookworm release (and ideally we can also finish off the necessary testing and add -fstack-clash-protection at least for amd64 and other archs which are ready for it (#918914)). Cheers, Moritz
Re: Enabling branch protection on amd64 and arm64
On 2022-10-25 16:10 +0100, Simon McVittie wrote:
> On Tue, 25 Oct 2022 at 15:34:26 +0100, Wookey wrote:
> > These are hardware features (new instructions) that 'tag' pointers and
> > branch targets to make it much harder for malicious code to implement
> > ROP (return oriented programming) and JOP (Jump oriented programming)
> > attacks.
> >
> > They have been implemented on both architectures in such a way that
> > they can be generally enabled and are simply ignored on hardware that
> > doesn't support them (the new instructions are in the NOP space).
>
> Does this have the same restrictions as CET, which gcc briefly enabled
> on x86 by default, but had to roll back[1] and later enable on a smaller
> subset of architectures[2], because the new instructions are only NOPs
> on x86_64 and modern i386, and are non-baseline (illegal instruction)
> on older or more-embedded i386 like the ones in our current i386 baseline?
I'm not sure (I know a lot more about the arm64 side of this than the
amd64 side), but we are only enabling this on amd64, not i386. amd64
binaries don't run on i386 so I don't think any practical issue
actually arises. You have reminded me that I guess it should be turned
on for x32 too.
> If yes, we'll have to be careful to only enable this on architectures
> where our baseline allows it. IIRC, Geode and VIA CPUs are the ones that
> usually cause trouble for i386.
Right, and that's the plan.
The patch looks approx like this:
+# Branch protection
+if ($use_feature{hardening}{branch}) {
+my $flag;
+if ($cpu eq 'arm64') {
+$flag = '-mbranch-protection=standard';
+} elsif ($cpu eq 'amd64') {
+$flag = '-fcf-protection';
+}
+$flags->append($_, $flag) foreach @compile_flags;
+}
Wookey
--
Principal hats: Debian, Wookware, ARM
http://wookware.org/
signature.asc
Description: PGP signature
Re: Enabling branch protection on amd64 and arm64
On Tue, 25 Oct 2022 at 15:34:26 +0100, Wookey wrote: > These are hardware features (new instructions) that 'tag' pointers and > branch targets to make it much harder for malicious code to implement > ROP (return oriented programming) and JOP (Jump oriented programming) > attacks. > > They have been implemented on both architectures in such a way that > they can be generally enabled and are simply ignored on hardware that > doesn't support them (the new instructions are in the NOP space). Does this have the same restrictions as CET, which gcc briefly enabled on x86 by default, but had to roll back[1] and later enable on a smaller subset of architectures[2], because the new instructions are only NOPs on x86_64 and modern i386, and are non-baseline (illegal instruction) on older or more-embedded i386 like the ones in our current i386 baseline? If yes, we'll have to be careful to only enable this on architectures where our baseline allows it. IIRC, Geode and VIA CPUs are the ones that usually cause trouble for i386. Of course, raising the i386 baseline would mitigate or solve that, at the cost of dropping support for some CPUs. [1] https://tracker.debian.org/news/1254900/accepted-gcc-11-1120-4-source-into-unstable/ [2] https://tracker.debian.org/news/1256872/accepted-gcc-11-1120-5-source-into-unstable/ smcv
Enabling branch protection on amd64 and arm64
I have been in discussion with Guillem about enabling the various branch protection mechanisms available on newer x86 and arm CPUs. These are hardware features (new instructions) that 'tag' pointers and branch targets to make it much harder for malicious code to implement ROP (return oriented programming) and JOP (Jump oriented programming) attacks. They have been implemented on both architectures in such a way that they can be generally enabled and are simply ignored on hardware that doesn't support them (the new instructions are in the NOP space). These features have been enabled in other distros for some time and we've done an archive rebuild of arm64 to check that there was not significant breakage. There is a lot of discussion of the details of this and the pros/cons of enabling this by default in the thread so I will try to keep this mail as a summary and suggest you go read https://lists.debian.org/debian-dpkg/2022/05/msg00022.html and https://lists.debian.org/debian-dpkg/2022/06/msg0.html if you want to know how it works, and all the details. We decided that the best thing to do was create a new hardening flags feature called 'branch' to add to the existing set. This enables -mbranch-protection=standard on arm64, and -fcf-protection on amd64 (yes it would have been nice if the gcc people had used common flags across the arches, but there you go) If/when other arches get similar functionality those would be enabled under this heading too (Are there any already that I don;t know about?) There is a dpkg branch containing this feature here: https://git.hadrons.org/git/debian/dpkg/dpkg.git/log/?h=next/dpkg-buildflags-feature-branch And a bug to track progress here: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1021292 So the immediate issue now is whether or not to enable this by default in bookworm? It's a significant protection on newish hardware, which those who've worked on it (and I now, having investigated) believe should be on by default. We have a general policy of enabling reaosnably low-cost security features by default, and this is one of those. It's a fairly low-risk thing to do, especially as others have gone before us (Rhel made -fcf-protection the gcc default in 2018, and Suse in Oct 2021. Suse turned on branch-protection=standard (ie BTI+PAC) on arm64 in nov 2020), but it is changing the defaults. Like all dpkg-buildflags it can easily be disabled for a particular package and there is a kernel option to turn it off on a particular machine if issues are encountered (and no doubt we will find a couple). I hope that all makes sense. Wookey -- Principal hats: Debian, Wookware, ARM http://wookware.org/ signature.asc Description: PGP signature
