Re: [PATCH net-next] [RESEND] wireguard: disable in FIPS mode
From: Hangbin Liu <hidden>
Date: 2021-04-09 02:41:56
Also in:
linux-crypto
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 [off-list ref] wrote:quoted
quoted
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.
quoted hunk ↗ jump to hunk
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