C++ Crypto:Part 3-pbkdf2

Question

Looking around for modern Crypto libraries.
Could not find anything good.

I know I probably did this all wrong so work with me here. There will be four different reviews for four structures that build on each other:

1. Hashing
2. Hashed Key
3. Password Key
4. Salted Challenge Response

This review is for an implementation of pbkdf2 implementation. This is a way of using HMAC that allows you to make incrementally more expensive to create the hash on failures. The idea is that you can slow down attacks by making it harder to make lots of guesses.

The data structures and implementation presented in these questions are based on RFC2104 and this post on codeproject.

Usage Example

Digest<Pbkdf2<HMac<Sha1>>>    digest;
Pbkdf2<HMac<Sha1>>            pbkdf2;

pbkdf2.hash("The password", "A Salt", 2048, digest);

pbkdf2.h

#ifndef THORS_ANVIL_CRYPTO_PBKDF2_H
#define THORS_ANVIL_CRYPTO_PBKDF2_H

#include "hmac.h"
#include <string>

// RFC-2898 PKCS #5: Password-Based Cryptography Specification Version 2.0

namespace ThorsAnvil::Crypto
{

// Look in hmac.h for good examples of PRF
// ThorsAnvil::Crypto::HMac
template<typename PRF>
struct Pbkdf2
{
    static constexpr std::size_t digestSize = PRF::digestSize;
    using DigestStore    = typename PRF::DigestStore;

    void hash(std::string const& password, std::string const& salt, long iter, DigestStore& digest)
    {
#pragma vera-pushoff
        using namespace std::string_literals;
#pragma vera-pop
        PRF             prf;
        DigestStore     tmp;

        prf.hash(password, salt + "\x00\x00\x00\x01"s, tmp);
        std::copy(std::begin(tmp), std::end(tmp), std::begin(digest));

        for (int loop = 1; loop < iter; ++loop)
        {
            prf.hash(password, tmp.view(), tmp);
            for (std::size_t loop = 0; loop < digestSize; ++loop)
            {
                digest[loop] = digest[loop] ^ tmp[loop];
            }
        }
    }
};

}

#endif

Answer 1

void hash(std::string const& password, std::string const& salt, long iter, DigestStore& digest)
...
for (int loop = 1; loop < iter; ++loop)
        ...
        for (std::size_t loop = 0; loop < digestSize; ++loop)

"A long an int and a std::size_t all walked into a bar..." couldn't they all be std::size_t?

Also, overriding the loop variable is unnecessarily confusing.

---

    std::copy(std::begin(tmp), std::end(tmp), std::begin(digest));

I feel like this could just be digest = tmp; with an appropriate assignment operator.

---

salt + "\x00\x00\x00\x01"s

Could this be a named constant (or even a named function)? Or maybe add a comment to explain why this is here.

---

            digest[loop] = digest[loop] ^ tmp[loop];

Nitpick: could use ^=

---

Perhaps the output should be a return value, not written to a reference?

---

I don't know enough about cryptography / security to assess the algorithm / safety aspects of this.