weidai11 / cryptopp

free C++ class library of cryptographic schemes
https://cryptopp.com
Other
4.89k stars 1.51k forks source link

Add KeyDerivationFunction interface #610

Closed noloader closed 6 years ago

noloader commented 6 years ago

This has been a long time coming... We have not really moved forward into the newer password hashing algorithms because we lacked the infrastructure to do so. In the old days we only needed to manage salts and iteration counts. Nowadays we have several additional parameters, like hardness factors.

The additional parameters beg for a KeyDerivationFunction interface that accepts a NameValuepairs to pass arbitrary parameters in an extensible way. While not readily apparent, a KeyDerivationFunction interface also provides unified testing of the algorithms using test vectors. The base interface allows us to provide factory methods to create a KDF without the need of knowing a particular constructor. The KDF is then configured using NameValuepairs.

This ticket will track the addition of the KeyDerivationFunction interface and the associated changes for existing KDF's and PBKDF's.

noloader commented 6 years ago

Here's the first cut on the interface based on applying it to HKDF class.

/// \brief Interface for key derivation functions
/// \since Crypto++ 6.2
class CRYPTOPP_DLL CRYPTOPP_NO_VTABLE KeyDerivationFunction : public Algorithm
{
public:
    virtual ~KeyDerivationFunction() {}

    virtual const Algorithm & GetAlgorithm() const =0;

    /// \brief Provides the name of this algorithm
    /// \return the standard algorithm name
    virtual std::string AlgorithmName() const =0;

    /// \brief Determine minimum number of bytes
    /// \returns Minimum number of bytes which can be derived
    virtual unsigned int MinDerivedLength() const;

    /// \brief Determine maximum number of bytes
    /// \returns Maximum number of bytes which can be derived
    virtual unsigned int MaxDerivedLength() const;

    /// \brief Returns a valid key length for the derivation function
    /// \param keylength the size of the derived key, in bytes
    /// \returns the valid key length, in bytes
    virtual size_t GetValidDerivedLength(size_t keylength) const =0;

    /// \brief Returns whether keylength is a valid key length
    /// \param keylength the requested keylength
    /// \return true if the derived keylength is valid, false otherwise
    /// \details Internally the function calls GetValidKeyLength()
    virtual bool IsValidDerivedLength(size_t keylength) const {
        return keylength == GetValidDerivedLength(keylength);
    }

    /// \brief Derive a key from secret
    /// \param derived the output byte buffer
    /// \param derivedLen the size of the output byte buffers, in bytes
    /// \param seed the input byte buffer
    /// \param seedLen the size of the input byte buffers, in bytes
    /// \param params additional initialization parameters to configure this object
    /// \throws InvalidKeyLength if the key length is invalid
    /// \details DeriveKey() provides a standard interface to derive a key from
    ///   a seed and other parameters. Each class that derives from KeyDerivationFunction
    ///   provides an overload that accepts most parameters used by the derivation function.
    virtual void DeriveKey(byte *derived, size_t derivedLen, const byte *secret, size_t secretLen,
        const NameValuePairs& params) const =0;

    /// \brief Set or change parameters
    /// \param params additional initialization parameters to configure this object
    /// \details SetParameters() is useful for setting common parameters when an object is
    ///   reused. Some derivation function classes may choose to implement it.
    virtual void SetParameters(const NameValuePairs& params);

protected:

    /// \brief Validates the derived key length
    /// \param length the size of the derived key material, in bytes
    /// \throws InvalidKeyLength if the key length is invalid
    void ThrowIfInvalidDerivedLength(size_t length) const;
};

And HKDF looks like:

template <class T>
class HKDF : public KeyDerivationFunction
{
public:
    virtual ~HKDF() {}

    static std::string StaticAlgorithmName () {
        static const std::string name(std::string("HKDF(") +
            std::string(T::StaticAlgorithmName()) + std::string(")"));
        return name;
    }

    const Algorithm & GetAlgorithm() const {
        return *this;
    }

    std::string AlgorithmName() const {
        return StaticAlgorithmName();
    }

    unsigned int MaxDerivedLength() const {
        return static_cast<unsigned int>(T::DIGESTSIZE) * 255;
    }

    size_t GetValidDerivedLength(size_t keylength) const;

    void DeriveKey(byte *derived, size_t derivedLen, const byte *secret, size_t secretLen,
        const NameValuePairs& params) const;

    void DeriveKey(byte *derived, size_t derivedLen, const byte *secret, size_t secretLen,
        const byte *salt, size_t saltLen, const byte* info, size_t infoLen) const;

protected:
    typedef byte NullVectorType[T::DIGESTSIZE];
    static const NullVectorType& GetNullVector() {
        static const NullVectorType s_NullVector = {0};
        return s_NullVector;
    }
};

The interesting function is:

template <class T>
void HKDF<T>::DeriveKey(byte *derived, size_t derivedLen, const byte *secret, size_t secretLen,
    const byte *salt, size_t saltLen, const byte* info, size_t infoLen) const
{
    AlgorithmParameters params =
        MakeParameters("Secret", ConstByteArrayParameter(secret, secretLen));

    if(salt && saltLen)
        params.operator()(Name::Salt(), ConstByteArrayParameter(salt, saltLen));

    if(info && infoLen)
        params.operator()("Info", ConstByteArrayParameter(info, infoLen));

    DeriveKey(derived, derivedLen, params);
}
anonimal commented 6 years ago
 static const std::string name(std::string("HKDF(") + std::string(T::StaticAlgorithmName()) + std::string(")"));

I don't mean to put a wrench in the works, but what are your thoughts on non-trivial destruction of objects with static duration? Concerns noted here.

noloader commented 6 years ago

Thanks @anonimal,

That's weird... I was thinking about you earlier in the week. I hope things are well.

Yeah, we can return a new copy on each call. I'll be sure to include it with my check-in.


I checked-in the changes for testing on my GitHub at NoLoader | GitHub. Also see Commit 9dc010acb6ec.

noloader commented 6 years ago

@denisbider, @mouse07410, @MarcelRaad,

For the KeyDerivationFunction interface this is the main one that all subclasses must implement. It is intended to be the cornerstone like UncheckedSetKey is for SymmetricCipher.

DeriveKey(byte *derived, size_t derivedLen, const byte *secret, size_t secretLen,
            const NameValuePairs& params)

Derived classes can implement additional DeriveKey methods. For example, HKDF has one that accepts all of its parameters:

DeriveKey(byte *derived, size_t derivedLen, const byte *secret, size_t secretLen,
            const byte *salt, size_t saltLen, const byte* info, size_t infoLen)

The small question I have is, should secret be called seed or something else? I'm trying to capture what many folks will recognize.

The broader question is, should we (1) make more functions abstract (like MinDerivedLength and MaxDerivedLength) and provide an adapter KeyDerivationFunctionImpl?

The reason for the broader question is, most of the classes Wei wrote were more abstract at this level in the library. A little higher up he provided default implementations through classes with *Impl.h names (like MessageAuthenticationCodeImpl).

The only reason I avoided an adapter is, KeyDerivationFunctionImpl is about as simple as can be. It is not like HashTransformation, where different behaviors follow based on the subclass (hashing versus mac'ing).

The final question, does this design look OK, or should we change something?

MarcelRaad commented 6 years ago

The design looks good to me. seed sounds reasonable. I'd have to think a bit more about the adapter question, but I don't have a strong opinion about it.

noloader commented 6 years ago

Thanks @MarcelRaad,

It looks like DeriveKey needs to return size_t instead of void. Some of the password based schemes take a time limit and iteration count, and and the function returns the number of iterations.

It also like like MinDerivedLength and MaxDerivedLength should return size_t instead of unsigned int.

mouse07410 commented 6 years ago

I prefer the term "secret"

noloader commented 6 years ago

Thanks @mouse07410,

Sounds good. I changed the documentation to include seed in the description:

/// \brief Derive a key from secret
/// \param derived the output byte buffer
/// \param derivedLen the size of the output byte buffers, in bytes
/// \param secret the seed input buffer
/// \param secretLen the size of the seed input byte buffers, in bytes
/// \param params additional initialization parameters to configure this object
/// \throws InvalidDerivedLength if <tt>derivedLen</tt> is invalid for the scheme
/// \details DeriveKey() provides a standard interface to derive a key from
///   a seed seed and other parameters. Each class that derives from KeyDerivationFunction
///   provides an overload that accepts most parameters used by the derivation function.
virtual size_t DeriveKey(byte *derived, size_t derivedLen, const byte *secret, size_t secretLen, const NameValuePairs& params) const =0;
noloader commented 6 years ago

@denisbider, @mouse07410, @MarcelRaad,

Based on the existing Crypto++ implementations and two additional implementations I have stashed away (Scrypt and Argon2), things settled/converged on:

class CRYPTOPP_DLL CRYPTOPP_NO_VTABLE KeyDerivationFunction : public Algorithm
{
public:
    virtual ~KeyDerivationFunction() {}

    /// \brief Provides the name of this algorithm
    /// \return the standard algorithm name
    virtual std::string AlgorithmName() const =0;

    /// \brief Determine minimum number of bytes
    /// \returns Minimum number of bytes which can be derived
    virtual size_t MinDerivedLength() const;

    /// \brief Determine maximum number of bytes
    /// \returns Maximum number of bytes which can be derived
    virtual size_t MaxDerivedLength() const;

    /// \brief Returns a valid key length for the derivation function
    /// \param keylength the size of the derived key, in bytes
    /// \returns the valid key length, in bytes
    virtual size_t GetValidDerivedLength(size_t keylength) const =0;

    /// \brief Returns whether keylength is a valid key length
    /// \param keylength the requested keylength
    /// \return true if the derived keylength is valid, false otherwise
    /// \details Internally the function calls GetValidKeyLength()
    virtual bool IsValidDerivedLength(size_t keylength) const {
        return keylength == GetValidDerivedLength(keylength);
    }

    /// \brief Derive a key from a seed
    /// \param derived the derived output buffer
    /// \param derivedLen the size of the derived buffer, in bytes
    /// \param secret the seed input buffer
    /// \param secretLen the size of the secret buffer, in bytes
    /// \param params additional initialization parameters to configure this object
    /// \returns the number of iterations performed
    /// \throws InvalidDerivedLength if <tt>derivedLen</tt> is invalid for the scheme
    /// \details DeriveKey() provides a standard interface to derive a key from
    ///   a secret seed and other parameters. Each class that derives from KeyDerivationFunction
    ///   provides an overload that accepts most parameters used by the derivation function.
    /// \details the number of iterations performed by DeriveKey() may be 1. For example, a
    ///   scheme like HKDF does not use the iteration count so it returns 1.
    virtual size_t DeriveKey(byte *derived, size_t derivedLen, const byte *secret, size_t secretLen, const NameValuePairs& params = g_nullNameValuePairs) const =0;

    /// \brief Set or change parameters
    /// \param params additional initialization parameters to configure this object
    /// \details SetParameters() is useful for setting common parameters when an object is
    ///   reused. Some derivation function classes may choose to implement it.
    virtual void SetParameters(const NameValuePairs& params);

protected:
    /// \brief Returns the base class Algorithm
    /// \return the base class Algorithm
    virtual const Algorithm & GetAlgorithm() const =0;

    /// \brief Validates the derived key length
    /// \param length the size of the derived key material, in bytes
    /// \throws InvalidKeyLength if the key length is invalid
    void ThrowIfInvalidDerivedLength(size_t length) const;
};

What I am finding is we have to pass some scheme-specific options using simple C-strings, like for Scrypt. It is OK because the test framework reads the name/value pairs for the test vector file and passes them to the object created by the factory:

size_t Scrypt::DeriveKey(byte *derived, size_t derivedLen,
    const byte *secret, size_t secretLen, const NameValuePairs& params) const
{
    unsigned int cost = (unsigned int)params.GetIntValueWithDefault("Cost", 2);
    unsigned int blockSize = (unsigned int)params.GetIntValueWithDefault("BlockSize", 8);
    unsigned int parallelization = (unsigned int)params.GetIntValueWithDefault("Parallelization", 1);

    ConstByteArrayParameter salt;
    (void)params.GetValue("Salt", salt);

    return DeriveKey(derived, derivedLen, secret, secretLen, salt.begin(), salt.size(), cost, blockSize, parallelization);
}