SP 800-56Br2, Section 7.1.1 [1] specifies that:
1. If m does not satisfy 1 < m < (n – 1), output an indication that m is
out of range, and exit without further processing.
Similarly, Section 7.1.2 of the same standard specifies that:
1. If the ciphertext c does not satisfy 1 < c < (n – 1), output an
indication that the ciphertext is out of range, and exit without further
processing.
[1] https://doi.org/10.6028/NIST.SP.800-56Br2
Signed-off-by: Joachim Vandersmissen <[email protected]>
---
crypto/rsa.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/crypto/rsa.c b/crypto/rsa.c
index b9cd11fb7d36..b5c67e6f8774 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -24,12 +24,36 @@ struct rsa_mpi_key {
MPI qinv;
};
+static int rsa_check_payload_fips(MPI x, MPI n)
+{
+ MPI n1;
+
+ if (mpi_cmp_ui(x, 1) <= 0)
+ return -EINVAL;
+
+ n1 = mpi_alloc(0);
+ if (!n1)
+ return -ENOMEM;
+
+ if (mpi_sub_ui(n1, n, 1) || mpi_cmp(x, n1) >= 0) {
+ mpi_free(n1);
+ return -EINVAL;
+ }
+
+ mpi_free(n1);
+ return 0;
+}
+
/*
* RSAEP function [RFC3447 sec 5.1.1]
* c = m^e mod n;
*/
static int _rsa_enc(const struct rsa_mpi_key *key, MPI c, MPI m)
{
+ /* For FIPS, SP 800-56Br2, Section 7.1.1 requires 1 < m < n - 1 */
+ if (fips_enabled && rsa_check_payload_fips(m, key->n))
+ return -EINVAL;
+
/* (1) Validate 0 <= m < n */
if (mpi_cmp_ui(m, 0) < 0 || mpi_cmp(m, key->n) >= 0)
return -EINVAL;
@@ -50,6 +74,11 @@ static int _rsa_dec_crt(const struct rsa_mpi_key *key, MPI m_or_m1_or_h, MPI c)
MPI m2, m12_or_qh;
int ret = -ENOMEM;
+ /* For FIPS, SP 800-56Br2, Section 7.1.2 requires 1 < c < n - 1 */
+ if (fips_enabled && rsa_check_payload_fips(c, key->n))
+ return -EINVAL;
+
+
/* (1) Validate 0 <= c < n */
if (mpi_cmp_ui(c, 0) < 0 || mpi_cmp(c, key->n) >= 0)
return -EINVAL;
--
2.43.0
On Sun, Jan 21, 2024 at 01:49:00PM -0600, Joachim Vandersmissen wrote:
>
> static int _rsa_enc(const struct rsa_mpi_key *key, MPI c, MPI m)
> {
> + /* For FIPS, SP 800-56Br2, Section 7.1.1 requires 1 < m < n - 1 */
> + if (fips_enabled && rsa_check_payload_fips(m, key->n))
> + return -EINVAL;
> +
> /* (1) Validate 0 <= m < n */
> if (mpi_cmp_ui(m, 0) < 0 || mpi_cmp(m, key->n) >= 0)
> return -EINVAL;
I think this check makes sense in general, so why not simply
replace the second check above with the new check?
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Hi Herbert,
On 1/25/24 23:58, Herbert Xu wrote:
> On Sun, Jan 21, 2024 at 01:49:00PM -0600, Joachim Vandersmissen wrote:
>> static int _rsa_enc(const struct rsa_mpi_key *key, MPI c, MPI m)
>> {
>> + /* For FIPS, SP 800-56Br2, Section 7.1.1 requires 1 < m < n - 1 */
>> + if (fips_enabled && rsa_check_payload_fips(m, key->n))
>> + return -EINVAL;
>> +
>> /* (1) Validate 0 <= m < n */
>> if (mpi_cmp_ui(m, 0) < 0 || mpi_cmp(m, key->n) >= 0)
>> return -EINVAL;
> I think this check makes sense in general, so why not simply
> replace the second check above with the new check?
Yes, mathematically speaking the values 1 and n - 1 aren't suitable for
RSA (they will always be fixed points). I simply didn't want to
introduce a breaking change. If you think a breaking change is
acceptable, I can update the patch to replace the RFC3447 check with the
stricter check.
>
> Thanks,
On Fri, Jan 26, 2024 at 12:13:00AM -0600, Joachim Vandersmissen wrote:
>
> Yes, mathematically speaking the values 1 and n - 1 aren't suitable for RSA
> (they will always be fixed points). I simply didn't want to introduce a
> breaking change. If you think a breaking change is acceptable, I can update
> the patch to replace the RFC3447 check with the stricter check.
Please do. We can always change it later if someone complains.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt