On Mon, May 8, 2023 at 3:29 AM Peter Xu <peterx@xxxxxxxxxx> wrote: > > On Fri, May 05, 2023 at 06:42:08PM +0200, Florent Revest wrote: > > On Thu, May 4, 2023 at 10:06 PM Peter Xu <peterx@xxxxxxxxxx> wrote: > > > > > > On Thu, May 04, 2023 at 07:09:38PM +0200, Florent Revest wrote: > > > > Joey recently introduced a Memory-Deny-Write-Executable (MDWE) prctl which tags > > > > current with a flag that prevents pages that were previously not executable from > > > > becoming executable. > > > > This tag always gets inherited by children tasks. (it's in MMF_INIT_MASK) > > > > > > > > At Google, we've been using a somewhat similar downstream patch for a few years > > > > now. To make the adoption of this feature easier, we've had it support a mode in > > > > which the W^X flag does not propagate to children. For example, this is handy if > > > > a C process which wants W^X protection suspects it could start children > > > > processes that would use a JIT. > > > > > > > > I'd like to align our features with the upstream prctl. This series proposes a > > > > new NO_INHERIT flag to the MDWE prctl to make this kind of adoption easier. It > > > > sets a different flag in current that is not in MMF_INIT_MASK and which does not > > > > propagate. > > > > > > I don't think I have enough context, so sorry if I'm going to ask a naive > > > question.. > > > > Not at all! :) You're absolutely right, it's important to address these points. > > > > > I can understand how current MDWE helps on not allowing any modifi-able > > > content from becoming executable. How could NO_INHERIT help if it won't > > > inherit and not in MMF_INIT_MASK? > > > > The way I see it, enabling MDWE is just a small step towards hardening > > a binary anyway. It can possibly make exploitation a bit harder in the > > case where the attacker has _just_: a write primitive they can use to > > write a shellcode somewhere and a primitive to make that page > > executable later. It's a fairly narrow protection already and I think > > it only really helps as part of a broader "defense in depth" strategy. > > > > > IIUC it means the restriction will only apply to the current process. Then > > > I assume the process can escape from this rule simply by a fork(). If so, > > > what's the point to protect at all? > > > > If we assume enough control from the attacker, then MDWE is already > > useless since it can be bypassed by writing to a file and then > > mmapping that file with PROT_EXEC. I think that's a good example of > > how "perfect can be the enemy of good" in security hardening. MDWE > > isn't a silver-bullet but it's a cheap trick and it makes a small dent > > in reducing the attack surface so it seems worth having anyway ? > > > > But indeed, to address your question, if you choose to use this > > NO_INHERIT flag: you're no longer protected if the attacker can fork() > > as part of their exploitation. I think it's been a useful trade-off > > for our internal users since, on the other hand, it also makes > > adoption a lot easier: our C++ services developers can trivially opt > > into a potpourri of hardening features without having to think too > > much about how they work under-the-hood. The default behavior has been > > to use a NO_INHERIT strategy so users don't get bad surprises the day > > when they try to spawn a JITted subcommand. In the meantime, their C++ > > service still gets a little bit of extra protection. > > > > > And, what's the difference of this comparing to disabling MDWE after being > > > enabled (which seems to be forbidden for now, but it seems fork() can play > > > a similar role of disabling it)? > > > > That would be functionally somewhat similar, yes. I think it mostly > > comes down to ease of adoption. I imagine that users who would opt > > into NO_INHERIT are those who are interested in MDWE for the binary > > they are writing but aren't 100% confident in what subprocesses they > > will run and so they don't have to think about disabling it after > > every fork. > > Okay, that makes sense to me. Thanks. > > Since the original MDWE was for systemd, I'm wondering what will happen if > some program like what you said is invoked by systemd and with MDWE enabled > already. Good question > Currently in your patch IIUC MDWE_NO_INHERIT will fail directly on MDWE > enabled process, Yes, I tried to stay close to the spirit of the existing logic (which doesn't allow any sort of privilege gains) but this is not particularly a requirement on our side so I'm quite flexible here. Maybe Joey has an input here ? > but then it makes me think whether it makes more sense to > allow converting MDWE->MDWE_NO_INHERIT in this case. It seems to provide a > most broad coverage on system daemons using MDWE starting from systemd > initial process, meanwhile allows specific daemons to fork into anything > like a JIT process so it can make itself NO_INHERIT. Attackers won't > leverage this because MDWE_NO_INHERIT also means MDWE enabled. I should have cc-ed systemd folks who could have opinions on this. I will for v2. + cc Topi & Lennart
