| Message ID | 20211209090358.28231-1-nstange@suse.de |
|---|---|
| Headers | show |
| Series | crypto: dh - infrastructure for NVM in-band auth and FIPS conformance | expand |
Am Donnerstag, 9. Dezember 2021, 10:03:40 CET schrieb Nicolai Stange:
Hi Nicolai,
I successfully tested the entire patch set with the NIST ACVP reference
implementation which covers key generation for all safe prime groups defined
in your patch set.
Tested-by: Stephan Mueller <smueller@chronox.de>
Ciao
Stephan
On 12/9/21 10:03 AM, Nicolai Stange wrote: > With the previous patches, the testmgr now has up to four test vectors for > DH which all test more or less the same thing: > - the two vectors from before this series, > - the vector for the ffdhe3072 group, enabled if > CONFIG_CRYPTO_DH_GROUPS_RFC7919 is set and > - the vector for the modp2048 group, similarly enabled if > CONFIG_CRYPTO_DH_GROUPS_RFC3526 is set. > > In order to avoid too much redundancy during DH testing, enable only a > subset of these depending on the kernel config: > - if CONFIG_CRYPTO_DH_GROUPS_RFC7919 is set, enable only the ffdhe3072 > vector, > - otherwise, if CONFIG_CRYPTO_DH_GROUPS_RFC3526 is set, enable only > the modp2048 vector and > - only enable the original two vectors if neither of these options > has been selected. > > Note that an upcoming patch will make the DH implementation to reject any > domain parameters not corresponding to some safe-prime group approved by > SP800-56Arev3 in FIPS mode. Thus, having CONFIG_FIPS enabled, but > both of CONFIG_CRYPTO_DH_GROUPS_RFC7919 and > CONFIG_CRYPTO_DH_GROUPS_RFC3526 unset wouldn't make much sense as it would > render the DH implementation unusable in FIPS mode. Conversely, any > reasonable configuration would ensure that the original, non-conforming > test vectors would not get to run in FIPS mode. > > Signed-off-by: Nicolai Stange <nstange@suse.de> > --- > crypto/testmgr.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
Hannes Reinecke <hare@suse.de> writes: > I have run this implementation against my NVMe In-band authentication > test suite and have found no issues. > > Tested-by: Hannes Reinecke <hare@suse.de> Thank you! Nicolai
On Thu, Dec 09, 2021 at 09:03:52AM +0000, Nicolai Stange wrote: > A previous patch made the dh-generic implementation's ->set_secret() to > generate an ephemeral key in case the input ->key_size is zero, just in > analogy with ecdh. Make the qat crypto driver's DH implementation to > behave consistently by doing the same. I ran a few tests on QAT GEN2 HW and this patch/set does not causes regressions. On the headline of the commit, should this be crypto: qat - ... ? > Signed-off-by: Nicolai Stange <nstange@suse.de> > Reviewed-by: Hannes Reinecke <hare@suse.de> Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Am Freitag, 14. Januar 2022, 11:55:26 CET schrieb Herbert Xu: Hi Herbert, > > On an unrelated note, this will break trusted_key_tpm_ops->init() in > > FIPS mode, because trusted_shash_alloc() would fail to get a hold of > > sha1. AFAICT, this could potentially make the init_trusted() module_init > > to fail, and, as encrypted-keys.ko imports key_type_trusted, prevent the > > loading of that one as well. Not sure that's desired... > > Well if sha1 is supposed to be forbidden in FIPS mode why should SHA-1 is approved in all use cases except signatures. Ciao Stephan
On Fri, 2022-01-14 at 13:35 +0100, Stephan Mueller wrote: > Am Freitag, 14. Januar 2022, 11:55:26 CET schrieb Herbert Xu: > > Hi Herbert, > > > > On an unrelated note, this will break trusted_key_tpm_ops->init() > > > in FIPS mode, because trusted_shash_alloc() would fail to get a > > > hold of sha1. AFAICT, this could potentially make the > > > init_trusted() module_init to fail, and, as encrypted-keys.ko > > > imports key_type_trusted, prevent the loading of that one as > > > well. Not sure that's desired... > > > > Well if sha1 is supposed to be forbidden in FIPS mode why should > > SHA-1 is approved in all use cases except signatures. Actually, even that's not quite true: you can't use it in a FIPS compliant system to *generate* signatures, but you can still use it in a FIPS compliant system to verify legacy signatures (signatures created before sha-1 was deprecated). It's still also completely acceptable as a hash for HMAC. The supporting document is this one: https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf The bottom line is removing SHA-1 to achieve "FIPS compliance" is the wrong approach. You just have to make sure you can never use it to generate signatures. James
Hi all, first of all, to the people primarily interested in security/keys/, there's a rather trivial change to security/keys/dh.c in patch 2/18. It would be great to get ACKs for that... This is v2, v1 can be found at https://lore.kernel.org/r/20211201004858.19831-1-nstange@suse.de For a list of changes, see below. Quote from v1's cover letter: === Hannes' recent work on NVME in-band authentication ([1]) needs access to the RFC 7919 DH group parameters and also some means to generate ephemeral keys. He currently implements both as part of his patchset (patches 3/12 and 8/12). After some internal discussion, we decided to split off the bits needed from crypto/dh into a separate series, i.e. this one here: - for the RFC 7919 DH group parameters, it's undesirable from a performance POV to serialize the well-known domain parameters via crypto_dh_encode_key() just to deserialize them shortly after again, - from an architectural POV, it would be preferrable to have the key generation code in crypto/dh.c rather than in drivers/nvme/, just in analogy to how key generation is supported by crypto/ecdh.c already. Patches 1-13/18 implement all that is needed for the NVME in-band authentication support. Unfortunately, due to the lack of HW, I have not been able to test the changes to the QAT or HPRE drivers (other than mere compile tests). Yet I figured it would be a good idea to have them behave consistently with dh_generic, and so I chose to introduce support for privkey generation to these as well. By coincidence, NIST SP800-56Arev3 compliance effectively requires that the domain parameters are checked against an approved set, which happens to consists of those safe-prime group parameters specified in RFC 7919, among others. Thus, introducing the RFC 7919 parameters to the kernel allows for making the DH implementation to conform to SP800-56Arev3 with only little effort. I used the opportunity to work crypto/dh towards SP800-56Arev3 conformance with the rest of this patch series, i.e. patches 14-18/18. I can split these into another series on its own, if you like. But as they depend on the earlier patches 1-13/18, I sent them alongside for now. === This patchset has been tested with and without fips_enabled on x86_64, ppc64le and s390x, the latter being big endian. Changes v1 -> v2: - Throughout the patchset: - Upcase enum group_id members and strip superfluous _RFCXYZ_ parts from the names. - Carry Hannes' Reviewed-bys from v1 over for those patches which have not changed (except for that group_id member renaming) - [03/18] ("crypto: dh - optimize domain parameter serialization for well-known groups"): - For better portability, don't serialize/deserialize directly from/to an enum group_id, but use an intermediate int for that. - [05/18] ("crypto: testmgr - add DH RFC 7919 ffdhe2048 test vector") - Use ffdhe3072 TVs rather than ones for ffdhe2048. Requested by Hannes, because "the NVMe spec mandates for its TLS profile the ffdhe3072 group". - [13/18] ("crypto: testmgr - add DH test vectors for key generation") - Use ffdhe3072 in place of ffdhe2048 here as well. - Rather than introducing completely new keypairs, reuse the ones from the known answer test introduced previously in this patchset. Thanks, Nicolai [1] https://lkml.kernel.org/r/20211123123801.73197-1-hare@suse.de Nicolai Stange (18): crypto: dh - remove struct dh's ->q member crypto: dh - constify struct dh's pointer members crypto: dh - optimize domain parameter serialization for well-known groups crypto: dh - introduce RFC 7919 safe-prime groups crypto: testmgr - add DH RFC 7919 ffdhe3072 test vector crypto: dh - introduce RFC 3526 safe-prime groups crypto: testmgr - add DH RFC 3526 modp2048 test vector crypto: testmgr - run only subset of DH vectors based on config crypto: dh - implement private key generation primitive crypto: dh - introduce support for ephemeral key generation to dh-generic crypto: dh - introduce support for ephemeral key generation to hpre driver crypto: dh - introduce support for ephemeral key generation to qat driver crypto: testmgr - add DH test vectors for key generation lib/mpi: export mpi_rshift crypto: dh - store group id in dh-generic's dh_ctx crypto: dh - calculate Q from P for the full public key verification crypto: dh - try to match domain parameters to a known safe-prime group crypto: dh - accept only approved safe-prime groups in FIPS mode crypto/Kconfig | 20 +- crypto/dh.c | 73 +- crypto/dh_helper.c | 691 +++++++++++++++++- crypto/testmgr.h | 388 +++++++++- drivers/crypto/hisilicon/hpre/hpre_crypto.c | 11 + drivers/crypto/qat/qat_common/qat_asym_algs.c | 9 + include/crypto/dh.h | 52 +- lib/mpi/mpi-bit.c | 1 + security/keys/dh.c | 2 +- 9 files changed, 1189 insertions(+), 58 deletions(-)