On Thu, Apr 8, 2021 at 7:55 AM Simo Sorce <simo@xxxxxxxxxx> wrote: > > I'm not sure this makes so much sense to do _in wireguard_. If you > > feel like the FIPS-allergic part is actually blake, 25519, chacha, and > > poly1305, then wouldn't it make most sense to disable _those_ modules > > instead? And then the various things that rely on those (such as > > wireguard, but maybe there are other things too, like > > security/keys/big_key.c) would be naturally disabled transitively? > > The reason why it is better to disable the whole module is that it > provide much better feedback to users. Letting init go through and then > just fail operations once someone tries to use it is much harder to > deal with in terms of figuring out what went wrong. > Also wireguard seem to be poking directly into the algorithms > implementations and not use the crypto API, so disabling individual > algorithm via the regular fips_enabled mechanism at runtime doesn't > work. What I'm suggesting _would_ work in basically the exact same way as this patch. Namely, something like: diff --git a/lib/crypto/curve25519.c b/lib/crypto/curve25519.c index 288a62cd29b2..b794f49c291a 100644 --- a/lib/crypto/curve25519.c +++ b/lib/crypto/curve25519.c @@ -12,11 +12,15 @@ #include <crypto/curve25519.h> #include <linux/module.h> #include <linux/init.h> +#include <linux/fips.h> bool curve25519_selftest(void); static int __init mod_init(void) { + if (!fips_enabled) + return -EOPNOTSUPP; + if (!IS_ENABLED(CONFIG_CRYPTO_MANAGER_DISABLE_TESTS) && WARN_ON(!curve25519_selftest())) return -ENODEV; Making the various lib/crypto/* modules return EOPNOTSUPP will in turn mean that wireguard will refuse to load, due to !fips_enabled. It has the positive effect that all other things that use it will also be EOPNOTSUPP. For example, what are you doing about big_key.c? Right now, I assume nothing. But this way, you'd get all of the various effects for free. Are you going to continuously audit all uses of non-FIPS crypto and add `if (!fips_enabled)` to every new use case, always, everywhere, from now into the boundless future? By adding `if (!fips_enabled)` to wireguard, that's what you're signing yourself up for. Instead, by restricting the lib/crypto/* modules to !fips_enabled, you can get all of those transitive effects without having to do anything additional. Alternatively, I agree with Eric - why not just consider this outside your boundary? Jason