On Wed, Jan 25, 2023 at 4:30 PM Dave Hansen <dave.hansen@xxxxxxxxx> wrote: > On 1/24/23 17:28, Eric Biggers wrote: > > To mitigate this CPU vulnerability, it's possible to enable "Data > > Operand Independent Timing Mode" (DOITM) by setting a bit in a MSR. > > While Intel's documentation suggests that this bit should only be set > > where "necessary", that is highly impractical, given the fact that > > cryptography can happen nearly anywhere in the kernel and userspace, and > > the fact that the entire kernel likely needs to be protected anyway. > > I think this misses a key point from the documentation: > > This functionality is intended for use by software which has > already applied other techniques to mitigate software timing > side channels, such as those documented in Intel's Guidelines > for Mitigating Timing Side Channels Against Cryptographic > Implementations. > > Translating from Intel-speak: Intel thinks that DOITM purely a way to > make the CPU run slower if you haven't already written code specifically > to mitigate timing side channels. All pain, no gain. > > The kernel as a whole is not written that way. The kernel as a whole also doesn't really use the FPU registers for anything other than checksumming and cryptography and stuff like that (it's disabled in the compiler flags because the FPU registers normally contain userspace state that must not be clobbered). The instructions listed on that Intel help page are all weird PM* and VP* arithmetic instructions that can't be generated from C code in the kernel (except for weird subsystems in which every function is only callable in kernel-FPU-enabled mode and the compiler is set to enable FPU instruction generation, by which I mean amdgpu). So with the current generations of CPUs, based on that Intel help page, "the kernel as a whole" should not be affected by this setting, it's mostly just cryptographic helpers written in assembly. From a quick-and-dirty grep: $ find arch/x86/ -name '*.S' | xargs egrep -li '(PMADDUBSW|PMULUDQ|VPMULHRSW|PMADDWD|VPLZCNTD|VPMULHUW|PMULDQ|VPLZCNTQ|VPMULHW|PMULHRSW|VPMADD52HUQ|VPMULLD|PMULHUW|VPMADD52LUQ|VPMULLQ|PMULHW|VPMADDUBSW|VPMULLW|PMULLD|VPMADDWD|VPMULUDQ|PMULLW|VPMULDQ)' arch/x86/crypto/nh-avx2-x86_64.S arch/x86/crypto/nh-sse2-x86_64.S arch/x86/crypto/poly1305-x86_64-cryptogams.S That's NHPoly1305 and Poly1305, two crypto functions. Poly1305 is used for the authentication part of AEAD schemes (in particular WireGuard uses Poly1305 for all its authentication), NHPoly1305 is for Adiantum (weird Android disk encryption stuff AFAIK). > I'm sure the crypto > folks that are cc'd can tell us specifically if the kernel crypto code > is written following those recommendations. Note that all the memory-management and block layer code that copies page contents around and swaps them to disk and so on should (ideally) be cryptographically timing-safe (yes I know that's very much not true with zram swap), because it operates on basically all userspace memory, including userspace memory used by cryptographic code. On top of that, some cryptographic secrets (including SSH authentication keys) need to be persisted somewhere - some cryptographic key material is stored in filesystems. And of course there are also secrets that are not quite cryptographic, and can be similarly important and small as cryptographic keys, such as cookies and OAuth tokens stored in the various databases of something like a browser. I think part of the reason why KSM is nowadays disabled in most cloud environments is that it introduced timing differences in the MM subsystem (between taking a write fault on a private page and taking a write fault on a KSM-shared page), and those timing differences could be used to leak information between VMs. (See, for example, <https://gruss.cc/files/remote_dedup.pdf>.) So I think there is quite a bit more kernel code that *handles cryptographic intermediate states and cryptographic secrets* (and non-cryptographic secrets that have similar properties) than kernel code that *performs cryptography*. Do we have a guarantee that things like page copying (for example as part of migration or forking), usercopy (for example for filesystem access), and whatever checksumming and other things might reasonably be happening in the block layer (including for swap) and in filesystems are going to be data-timing-independent on future processors? > So, let's call this patch what it is: a potential global slowdown which > protects a very small amount of crypto code, probably just in userspace. And a bunch of data paths that this crypto code's secrets might go through, in particular in the kernel. Also note that the kernel can do a bunch of important cryptography on its own, like for disk encryption, filesystem encryption, SSL/TLS sockets (yes you can have the kernel do the easy parts of TLS for you), IPSec, WireGuard, and probably a bunch of other things. > That is probably the code that's generating your RSA keys, so it's > quite important, but it's also a _very_ small total amount of code. Yeah but all the code that handles storage of your RSA keys really also needs a similar level of protection when touching them. See https://arxiv.org/pdf/2108.04600.pdf for an example: "We analyze the exploitability of base64 decoding functions across several widely used cryptographic libraries. Base64 decoding is used when loading keys stored in PEM format. We show that these functions by themselves leak sufficient information even if libraries are executed in trusted execution environments. In fact, we show that recent countermeasures to transient execution attacks such as LVI ease the exploitability of the observed faint leakages, allowing us to robustly infer sufficient information about RSA private keys with a single trace. We present a complete attack, including a broad library analysis, a high-resolution last level cache attack on SGX enclaves, and a fully parallelized implementation of the extend-and-prune approach that allows a complete key recovery at medium costs." > There's another part here which I think was recently added to the > documentation: > > Intel expects the performance impact of this mode may be > significantly higher on future processors. > > That's _meant_ to be really scary and keep folks from turning this on by > default, aka. what this patch does. Your new CPU will be really slow if > you turn this on! Boo! It does sound really scary, because it sounds like that list of instructions Intel published might be useless for trying to reason about the security impact of turning this optimization on for future hardware? If the performance impact is going to be higher on newer CPUs then I would *definitely* want DOITM to be on by default, and then when Intel can say what instructions are actually affected on a new CPU, we can figure out whether it's safe to turn it off as a per-cpu-generation thing? > All that said, and given the information that Intel has released, I > think this patch is generally the right thing to do. I don't think > people are wrong for looking at "DODT" as being a new vulnerability. > Intel obviously doesn't see it that way, which is why "DODT" has (as far > as I can tell) not been treated with the same security pomp and > circumstance as other stuff. > > Last, if you're going to propose that this be turned on, I expect to see > at least _some_ performance data. DOITM=1 isn't free, even on Ice Lake. So all in all, it sounds to me like this should only affect the performance of crypto code in current CPUs (where we really want data-independent timing), so it should definitely be enabled on current CPUs. And on future CPUs it's a surprise package of potential security problems, so for those we should be turning it on even more?
