On Thu, Apr 08, 2021 at 03:55:59PM -0600, Jason A. Donenfeld wrote: > 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: Hi Jason, I agree that the best way is to disable the crypto modules in FIPS mode. But the code in lib/crypto looks not the same with crypto/. For modules in crypto, there is an alg_test() to check if the crytpo is FIPS allowed when do register. - crypto_register_alg() - crypto_wait_for_test() - crypto_probing_notify(CRYPTO_MSG_ALG_REGISTER, larval->adult) - cryptomgr_schedule_test() - cryptomgr_test() - alg_test() But in lib/crypto the code are more like a library. We can call it anytime and there is no register. Maybe we should add a similar check in lib/crypto. But I'm not familiar with crypto code... Not sure if anyone in linux-crypto@ would like help do that. > > 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; Question here, why it is !fips_enabled? Shouldn't we return error when fips_enabled? Thanks Hangbin