diff mbox series

crypto: rsa - restrict plaintext/ciphertext values more in FIPS mode

Message ID 20240121194901.344206-1-git@jvdsn.com (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show
Series crypto: rsa - restrict plaintext/ciphertext values more in FIPS mode | expand

Commit Message

Joachim Vandersmissen Jan. 21, 2024, 7:49 p.m. UTC
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 <git@jvdsn.com>
---
 crypto/rsa.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

Comments

Herbert Xu Jan. 26, 2024, 5:58 a.m. UTC | #1
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,
Joachim Vandersmissen Jan. 26, 2024, 6:13 a.m. UTC | #2
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,
Herbert Xu Jan. 26, 2024, 8:29 a.m. UTC | #3
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,
diff mbox series

Patch

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;