Re: [PATCH v3 0/5] DH: SP800-56A rev 3 compliant validation checks

[Marcelo Henrique Cerri <marcelo.cerri@canonical.com>]

[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]

Reviewed-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
Tested-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>

On Mon, Jul 20, 2020 at 07:05:45PM +0200, Stephan Müller wrote:
> Hi,
> 
> This patch set adds the required checks to make all aspects of
> (EC)DH compliant with SP800-56A rev 3 assuming that all keys
> are ephemeral. The use of static keys adds yet additional
> validations which are hard to achieve in the kernel.
> 
> SP800-56A rev 3 mandates various checks:
> 
> - validation of remote public key defined in section 5.6.2.2.2
>   is already implemented in:
> 
>   * ECC: crypto_ecdh_shared_secret with the call of
>     ecc_is_pubkey_valid_partial
> 
>   * FFC: dh_compute_val when the req->src is read and validated with
>     dh_is_pubkey_valid
> 
> - validation of generated shared secret: The patch set adds the
>   shared secret validation as defined by SP800-56A rev 3. For
>   ECDH this only implies that the validation of the shared secret
>   is moved before the shared secret is returned to the caller.
> 
>   For DH, the validation is required to be performed against the prime
>   of the domain parameter set.
> 
>   This patch adds the MPI library file mpi_sub_ui that is required
>   to calculate P - 1 for the DH check. It would be possible, though
>   to simply set the LSB of the prime to 0 to obtain P - 1 (since
>   P is odd per definition) which implies that mpi_sub_ui would not
>   be needed. However, this would require a copy operation from
>   the existing prime MPI value into a temporary MPI where the
>   modification can be performed. Such copy operation is not available.
>   Therefore, the solution with the addition of mpi_sub_ui was chosen.
> 
>   NOTE: The function mpi_sub_ui is also added with the patch set
>   "[PATCH v5 2/8] lib/mpi: Extend the MPI library" currently sent
>   to the linux-crypto mailing list.
> 
> - validation of the generated local public key: Patches 4 and 5 of
>   this patch set adds the required checks.
> 
> Changes to v2:
> 
> - add reference to GnuMP providing the basis for patch 2 and updating
>   the copyright note in patch 2
> 
> Changes to v1:
> 
> - fix reference to Gnu MP as outlined by Ard Biesheuvel
> - addition of patches 4 and 5
> 
> Marcelo Henrique Cerri (1):
>   lib/mpi: Add mpi_sub_ui()
> 
> Stephan Mueller (4):
>   crypto: ECDH - check validity of Z before export
>   crypto: DH - check validity of Z before export
>   crypto: DH SP800-56A rev 3 local public key validation
>   crypto: ECDH SP800-56A rev 3 local public key validation
> 
>  crypto/dh.c          | 38 +++++++++++++++++++++
>  crypto/ecc.c         | 42 +++++++++++++++++++++---
>  crypto/ecc.h         | 14 ++++++++
>  include/linux/mpi.h  |  3 ++
>  lib/mpi/Makefile     |  1 +
>  lib/mpi/mpi-sub-ui.c | 78 ++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 172 insertions(+), 4 deletions(-)
>  create mode 100644 lib/mpi/mpi-sub-ui.c
> 
> -- 
> 2.26.2
> 
> 
> 
> 

-- 
Regards,
Marcelo


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]