On 2022-06-04 06:25 +0200, Guillem Jover wrote: > On Sun, 2022-05-29 at 14:19:05 +0100, Wookey wrote: > > Discussion: > > > The -mbranch-protection=standard option could be set either by dpkg or > > by gcc. I'm not quite sure how we decide which is most appropriate? > > I think if the option has potential for breakage or for a performance > hit, then it tends to be better to enable them via dpkg hardening > flags, otherwise people building in Debian systems that are not > interested in the packaging defaults might get surprised.
Makes sense. On the other hand ARM people would like this to be an achitecture default everywhere so that the protection afforded actually gets used. Because this hardware is not widespread yet beyond phone hardware (and Apple M1) Linux hasn't been run much on it yet so it is early days and we don't know if there will be significant issues that might mke turning it on everywhere a bad plan. It's easy enough to make it a gcc default later, after initial implementaiton as a hardening flag. > > They could be an unconditional architecture default, or could be part > > of the dpkg hardening flags. It could be one hardening feature 'PACBTI', > > or separate 'PAC' and 'BTI' features (with corresponding logic to set > > the right flags). > > > A notable aspect of this flag is that it is arch-specific. It should > > not be issued when targetting arches other than arm64 as it is not a > > valid flag there. dpkg-buildflags gets this right with below patch. > > The -fcf-protection option on amd64 has the same > > characteristic and indeed is pretty-much the same functionality (as > > BTI). > If there's potential for needing to disable them (for whatever reason), > then adding them into their own feature would be better. But having a > feature that is arch-specific (not just that it does not currently > work on some arches), seems not ideal. > > I think I'd probably want a new feature that can potentially be used > on other arches such as with -fcf-protection on amd64, yes. > > Say hardening=+branch or perhaps branchprotection or similar. OK, that sounds sensible to me. I agree that a generic name makes sense. We are quite likely to get something like PAC on x86 too at some point and then that can be added easily in this framework. Does 'jumpprotection' (or shorter 'jumpprot' or even 'jump') work any better given that 'returns' are not normally described as 'branches' (and PAC protects against 'return-oriented-programming' attacks and BTI aginst 'jump-oriented-programming' attacks)? But the name is bikeshedding. 'branch' is succinct and fine with me. > > The hardening features seem to assume that they are all > > implemented/available on all architectures? I'm not sure if extra work > > would be needed for a hardening feature that only existed on one arch > > (so far) (PAC). (BTI exists on two arches). Perhaps all this is an > > argument for just turning it on by default in gcc instead? > > The features can be easily shadowed for specific arches, if for > example they currently have no proper support for the flags. But > I'd rather abstract this than have a new feature per option denoting > similar protections for each arch. Agreed. > > As I said, I'm not sure what our policy is on what goes in gcc default > > flags, dpkg default flags or dpkg feature flags. So I'm open to > > suggestions on the best/right way to implement this, then will prepare > > patches/file bugs. > > I think the attached patch might do (it's still missing man page > update), assuming that the flags work on all compiler variables, and > the amd64 case could be disabled for now if there are concerns of > enabling both at the same time (it could be brought up later). cf-protection has been enabled by default on redhat since 2018 and Suse gcc since oct 2021. Only Suse has also enabled branch-protection=standard on arm64 (in Tumbleweed since nov 2020) - Rhel has not done it yet, but is trying to. So the x86 version of this has has a good level of testing without causing any trouble I am aware of, and the arm64 has had reasonable expsure in Suse for a while too. > The main issue is that because most packages just do hardening=+all, > even adding such feature disabled by default, would imply pretty much > enabling it immediately, so we might as well just enable it by > default. But to do that, the usual step after having done an archive > rebuild (if it seemed appropriate) and which you already did, would be > to bring this up on the debian-devel mailing list. If there are no > objections after a bit, I'm fine adding the support on the next dpkg > release after that time. > > > The only known compatibility risk is running modern binaries (built > > with PAC enabled) on modern hardware (ARMv8.3 or newer) on binaries > > (e.g. in an old chroot) built with gcc older than 7 (so that the > > tagged pointers are not recognised and properly masked), which might > > cause hangs/faults during exception unwinding, should some > > exception-causing error occur. > > I'm not sure I understand this sentence above about “running modern > binaries … on binaries”? Did you mean run-time linked, or is this > related to the kernel? Or something else? Sorry. I'm a bit vague about the details, but the issue as I understand it is to do with exception handling/stack unwinding, which is handled by a low-level gcc library, so if a binary built with PAC enabled (so it has PAC instructions) is run on new hardware (so it has tagged pointers) but in an old enough rootfs/container (so the gcc is <7), then the bit of libgcc/libunwind(?) responsible for unwinding the function stack for error reporting will be confused by the extra tag bits/instructions in NOP space. That's not a very likely combination, but could happen if someone puts an old enough chroot on new (8.3) hardware and then puts some newer-than-now binaries into it. GCC 7 was mid 2017. In a debian context this means running a Stretch (debian 9) chroot on a graviton3/M1 or newer and putting unstable (from mid 2022) binaries onto it (and wanting functioning exception handling, but not disabling PAC in the kernel). This sort of package mixing (stretch+bookworm) is discouraged, so the issue would be most likely to arise if running new 3rd-party binaries from elsewhere (and they crash). The right thing to do in this case would be setting arm64.nopauth on the kernel command line, then everything would work fine. I'm not sure if it will always fail or just might - I'll have to find out. Overall my feeling is that this is not a large enough problem/risk to delay enabling 'branch' hardening for another release, but I should try it to see exactly how it manifests. <snip patch> Cheers for that. I'll test it, but it looks fine. And I should try and add some man-page words. Wookey -- Principal hats: Debian, Wookware, ARM http://wookware.org/
signature.asc
Description: PGP signature
