libgcrypt's gcry_kdf_scrypt has incorrectly set N and P values
Closed, InvalidPublic

Description

Looks like in libgcrypt's scrypt algorithm implementation, N and P values are wrongly set.

From 1.6.3 to 1.8.1 the values are same as wrong (If i am correct):

u64 N = subalgo;    /* CPU/memory cost paramter.  */
u32 r;              /* Block size.  */
u32 p = iterations; /* Parallelization parameter.  */

Ideally it should have been

u64 N = iterations;    /* CPU/memory cost paramter.  */ <<<<<<<<<<<
u32 r;              /* Block size.  */
u32 p = subalgo; /* Parallelization parameter.  */ <<<<<<<<<<<

Please let me know

lbandlav created this task.Jan 12 2018, 1:02 PM
lbandlav triaged this task as Normal priority.

With the current implementation when the r is set to GCRY_KDF_SCRYPT, on a 3 core system, it almost took 35 minutes to generate the hash, where as with r=41 it was around 4 minutes and 20 seconds.
when i corrected the the values, i.e. N=16384, p=1 and r=GCRY_KDF_SCRYPT, it took less than a second to generate the hash.

werner added a subscriber: werner.

tests/t-kdf uses test vectors from an I-D and obviously works fine. Maybe that I-D has a different parameter naming than what is used in your examples. I simply can't say without researching the whole thing. Please let t me know a concrete bug where that KDF is not compatible with other implementations. As an example here is one of our test vectors:

{
  "password", 8,
  "NaCl", 4,
  1024,  /* N */
  8,     /* r */
  16,    /* p */
  64,    /* dklen */
  "\xfd\xba\xbe\x1c\x9d\x34\x72\x00\x78\x56\xe7\x19\x0d\x01\xe9\xfe"
  "\x7c\x6a\xd7\xcb\xc8\x23\x78\x30\xe7\x73\x76\x63\x4b\x37\x31\x62"
  "\x2e\xaf\x30\xd9\x2e\x22\xa3\x88\x6f\xf1\x09\x27\x9d\x98\x30\xda"
  "\xc7\x27\xaf\xb9\x4a\x83\xee\x6d\x83\x60\xcb\xdf\xa2\xcc\x06\x40"
},

and I checked that the parameters are passed to gcry_kdf_derive in the correct way (subalgo=N, iterations=p)

Here's what i got from 1.8.1 code (downloaded from gnupg).

/* Derive a key from a passphrase. KEYSIZE gives the requested size

of the keys in octets.  KEYBUFFER is a caller provided buffer
filled on success with the derived key.  The input passphrase is
taken from (PASSPHRASE,PASSPHRASELEN) which is an arbitrary memory
buffer.  ALGO specifies the KDF algorithm to use; these are the
constants GCRY_KDF_*.  SUBALGO specifies an algorithm used
internally by the KDF algorithms; this is usually a hash algorithm
but certain KDF algorithm may use it differently.  {SALT,SALTLEN}
is a salt as needed by most KDF algorithms.  ITERATIONS is a
positive integer parameter to most KDFs.  0 is returned on success,
or an error code on failure.  */

gpg_err_code_t
_gcry_kdf_derive (const void *passphrase, size_t passphraselen,

int algo, int subalgo,
const void *salt, size_t saltlen,
unsigned long iterations,
size_t keysize, void *keybuffer)

Here Iterations are 20000 for PBKDF2 (type8) and 16384 for scrypt (type9). So those corresponding API's

  ec = _gcry_kdf_pkdf2 (passphrase, passphraselen, subalgo,
                        salt, saltlen, iterations, keysize, keybuffer);

ec = _gcry_kdf_scrypt (passphrase, passphraselen, algo, subalgo,
                       salt, saltlen, iterations, keysize, keybuffer);

gcry_err_code_t
_gcry_kdf_scrypt (const unsigned char *passwd, size_t passwdlen,

int algo, int subalgo,
const unsigned char *salt, size_t saltlen,
unsigned long iterations,
size_t dkLen, unsigned char *DK)

{

u64 N = subalgo;    /* CPU/memory cost paramter.  */
u32 r;              /* Block size.  */
u32 p = iterations; /* Parallelization parameter.  */

Here iterations are assigned to p rather than N.

In the t-kdf examples, N is always greater than p, but going by the decsripton and argument naming conventions, once would set iterations (20000 or 16384) to above said numbers for the respective algorithm. Hope this is clear.

Your are looking at the libgcrypt code. Unfortunately that does not help us. What I would like to see are two protocol implementations, using sccryptone with libgcrypt and one with anoter scruypt implementation. Do they both work? If so, there is no bug in libgcrypt's code - at best the parameter have been given different names and we can point other name use in the docs.

I would also suggest to discuss this at the gcrypt-devel list so that you can get get comments from others as well.

In any case we can't change anything in the libgcrypt code because that would break applications which are using sccrypt for 5 years now.

lbandlav added a comment.EditedJan 12 2018, 5:51 PM

Hope you've got the problem with the current naming conventions for arguments and the result by going them. We should either document the arguments properly or change the code as i have pointed out. Since the iterations argument used properly in the case PBKDF2 (type8) within the same wrapper api gcry_kdf_derive.

Will be posting it in gcrypt-devel shortly.

Have posted in gcrypt-devel mailer.. thanks

werner closed this task as Invalid.Dec 17 2018, 9:55 AM

I have seen no responses on your two mails to the ML and given th athere is no concrete protocol bug, I close this issue. If you can show a concrete bug please re-open this issue again.