libscrypt-kdf

[gperciva]

No description provided.

[gperciva]

From offline communication, libtool is ok. I've therefore reworked this to use libtool, in order to provide a shared library as well.

Two remaining questions:

• should we provide a way to query CPU-specific code? (something like cpusupport_x86_shani(), although reworked to not require CPUSUPPORT_ defines)
  That could plausibly influence the choice of N, r, p.

  Offline answer: no.

• should we provide a convenience wrapper function, such as insecure_scrypt() (which has hard-coded values for N, r, p, and possibly takes care of the salt as well)?
  I expect that the answer to this one is no, but it's something I saw in downstream projects, so I figured it would be good to have a pre-emptive official ruling.

  Offline answer: no.

[gperciva]

Note: before the first release that includes libscrypt, we should update -version-info 0 in Makefile.am.

[cperciva]

Dumb question: Is it possible to make libtool necessary only for people who want libscrypt and not for people who only want the utility?

[gperciva]

I'll check again, but I tried hiding LT_INIT inside an AS_IF but it still generated the configure checks. It's just possible that I forgot to :w the file before running autoreconf -i (hence the double-check), but my general impression is that autotools really, really, really wants you to use libtool, so I wasn't surprised when it didn't work.

[gperciva]

Looks like I wasn't wrong. If configure.ac contains:

AM_COND_IF([LIBSCRYPT],
        [LT_INIT], [AC_PROG_RANLIB])

then only one of those should be executed. But instead, I get:

$ autoreconf -i
libtoolize: Remember to add 'LT_INIT' to configure.ac.
libtoolize: 'AC_PROG_RANLIB' is rendered obsolete by 'LT_INIT'

which amusingly seems to imply that LT_INIT is a cat in a box.

Looking at the resulting configure, it looks like it contains both LT_INIT and RANLIB stuff.

[gperciva]

Rebased onto master

[cperciva]

Sigh... the wonders of autoconf...

Ok, we definitely need to make this version 1.3.0.

[gperciva]

Not that I disagree with the number 1.3.0, but it just occurred to me that I only tested "does it check for libtool?", not "does it bail if it doesn't find libtool?". Also, I was looking at the "build from git", not "build from a release tarball".

I'll test the latter now.

[cperciva]

Hmm, I wonder if it makes sense for this to be crypto_scrypt.h and crypto_scrypt(). Within our tree that namespacing makes sense, but for someone else who is just linking with -lscrypt maybe it would make more sense for this to be scrypt.h and scrypt()?

[gperciva]

First thought: if somebody doesn't check the header file, they're not going to know which parameters are for the salt, N, r, etc.

That said, it's quite plausible that somebody would look for a scrypt.h file. But I'd rather not rename the file upon install, since that might cause confusion if somebody looks for it in our git repository.

What would you feel about a symlink for scrypt.h ?

[cperciva]

I was thinking maybe that we could simply have a separate scrypt.h which gets used for libscrypt. Keeping it in sync with crypto_scrypt.h doesn't seem like it would be too hard given how often that file changes.

[gperciva]

... ok. I'm guessing that file would be

#ifndef SCRYPT_H
#define SCRYPT_H
#include "crypto_scrypt.h"
#endif

[cperciva]

No, because I'd prefer to install just one header file, not two. ;-)

And it needs some magic so that people can call scrypt() and end up running crypto_scrypt code.

[gperciva]

ok, I'll add that.

Thinking about release plans... how about we have 1.2.2 without libscrypt, but add a note saying that we're adding a libscrypt library and expect to have 1.3.0 in a week or two. (or maybe that should be a separate message)

Basically, get the current code out there (in case libtool turns out to be a huge pain for some people), and attract more eyeballs on the libscrypt branch before we have a "libscrypt 1.0.0" out there.

[gperciva]

Good news: libtool is not at all a requirement for compiling an official tarball. I tried it on Ubuntu and freebsd; the latter only had:

$ pkg info
pkg-1.10.5_5                   Package manager
vim-console-8.1.0985           Improved version of the vi editor (console only)
$

so there's absolutely no dependency on autotools.

[gperciva]

In a comment, you said:

And it needs some magic so that people can call scrypt() and end up running crypto_scrypt code.

Do you actually want magic (something fancy), or just a plain function call?

[cperciva]

I was thinking that #define scrypt crypto_scrypt along with the declaration of crypto_scrypt would suffice.

[gperciva]

Oops, I took off the initial underscore, but forgot about the trailing one.

BTW: can we not have an initial underscore here?

[cperciva]

Sure.

[gperciva]

I forgot that I gave a sample compile command in the README. We probably don't need this makefile, then.

[gperciva]

I'm leaning towards making a libtool PR, which switches to that but doesn't introduce any libscrypt stuff. But I tend to prefer multiple small PRs.

[gperciva]

Updated PR with rebase

[gperciva]

Updated with rename to scrypt-kdf.

[cperciva]

LGTM, should I merge this now?

[gperciva]

I've just rebased it onto master so that we'll have nicer-looking history. Then it's good for a merge. :)

[gperciva]

actually, I'll do a final test of this on OSX.

[gperciva]

Good to go now. We'll assume that if somebody is installing the library on OSX to a non-standard location, they know how to set up C_INCLUDE_PATH etc.