Re: [PATCH 02/10] crypto: rsa_helper - add raw integer parser actions

[Stephan Mueller <smueller@chronox.de>]

Am Montag, 21. März 2016, 15:17:20 schrieb Tudor-Dan Ambarus:

Hi Tudor,

> Hi Stephan,
> 
> > -----Original Message-----
> > From: Stephan Mueller [mailto:smueller@chronox.de]
> > Sent: Friday, March 18, 2016 9:47 PM
> > To: Tudor-Dan Ambarus
> > Cc: herbert@gondor.apana.org.au; tadeusz.struk@intel.com; linux-
> > crypto@vger.kernel.org; Horia Ioan Geanta Neag
> > Subject: Re: [PATCH 02/10] crypto: rsa_helper - add raw integer parser
> > actions
> > 
> > > +int rsa_check_key_length(unsigned int len)
> > > +{
> > > +	switch (len) {
> > > +	case 512:
> > > +	case 1024:
> > > +	case 1536:
> > > +	case 2048:
> > > +	case 3072:
> > > +	case 4096:
> > > +		return 0;
> > > +	}
> > 
> > I know that you copied the code to a new location that was there already.
> > But
> > based on the discussion we had for DH, does it make sense that the kernel
> > adds
> > such (artificial) limits?
> 
> [ta] This is not within the scope of this patch set, but we can remove the
> restrictions in a subsequent patch. Marcel has suggested to not impose
> limits on the minimum length of the key. What about the maximum?

Why any restrictions at all? There is no mathematical reason. There is only a 
technical reason which depends on the implementation.

I am not sure how large the keys can be for the software fallback. But maybe 
it would make sense to have a general cap like 32k where the software fallback 
can handle all key sizes up to that point.

> > > +
> > > +	return -EINVAL;
> > > +}
> > > +EXPORT_SYMBOL_GPL(rsa_check_key_length);
> > > +
> > > +int raw_rsa_get_n(void *context, size_t hdrlen, unsigned char tag,
> > > +		  const void *value, size_t vlen)
> > > +{
> > > +	struct rsa_raw_ctx *ctx = context;
> > > +	struct rsa_raw_key *key = &ctx->key;
> > > +	const char *ptr = value;
> > > +	int ret = -EINVAL;
> > > +
> > > +	while (!*ptr && vlen) {
> > > +		ptr++;
> > > +		vlen--;
> > > +	}
> > > +
> > > +	key->n_sz = vlen;
> > > +	/* In FIPS mode only allow key size 2K & 3K */
> > > +	if (fips_enabled && (key->n_sz != 256 && key->n_sz != 384)) {
> > 
> > Again, you copied that code that used to be there . But very very
> > recently,
> > NIST allowed 4k keys too. May I ask to allow it here?
> 
> I suggest to do this in a separate patch. Can you send us a pointer to the
> NIST specification?

Sure, just let us not forget about it.
> 
> Thank you,
> ta


Ciao
Stephan