4
\$\begingroup\$

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 is the hashing code and provides a simple wrapper around SHA-1 and SHA-256 but the pattern is simple enough that we can expand it for other hashing mechanisms.

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

Usage Example:

DigestStore<Sha1>    hash;    // <- destination of hash
Sha1                 hasher;

hasher.hash("This string can be hashsed", hash);

hash.h

#ifndef THORS_ANVIL_CRYPTO_HASH_H
#define THORS_ANVIL_CRYPTO_HASH_H

#ifdef  __APPLE__
#define COMMON_DIGEST_FOR_OPENSSL
#include <CommonCrypto/CommonDigest.h>
#define THOR_SHA1(data, len, dst)       CC_SHA1(data, len, dst)
#define THOR_SHA256(data, len, dst)     CC_SHA256(data, len, dst)
#else
#include <openssl/sha.h>
#define THOR_SHA1(data, len, dst)       SHA1(data, len, dst)
#define THOR_SHA256(data, len, dst)     SHA256(data, len, dst)
#endif

#include <string>
#include <array>

//
// Wrapper for sha1 and sha256 hashing algorithms
//
// Provides a simple wrapper class with the appropriates types and size
// for the resulting "digest" object. Also provides several type safe
// versions of the hashing algorithm to allow multiple know types to
// be safely hashed.

namespace ThorsAnvil::Crypto
{

using Byte      = char unsigned;
using DigestPtr = Byte*;

template<typename Hash>
using Digest = typename Hash::DigestStore;

template<std::size_t size>
class DigestStore
{
    std::array<Byte, size>      data;
    public:
        using iterator = typename std::array<Byte, size>::iterator;

        operator Digest()                           {return &data[0];}
        std::string_view  view()                    {return std::string_view(reinterpret_cast<char const*>(&data[0]), std::size(data));}
        Byte&             operator[](std::size_t i) {return data[i];}
        iterator          begin()                   {return std::begin(data);}
        iterator          end()                     {return std::end(data);}
};

// These versions of the hashing function are good for hashing short
// amounts of text. Use these for passwords and validation hashes
// do not use them for hashing large documents.
struct Sha1
{
    static constexpr std::size_t digestSize = SHA_DIGEST_LENGTH;
    using DigestStore = DigestStore<SHA_DIGEST_LENGTH>;

    void hash(DigestStore& src,             DigestStore& dst)   {THOR_SHA1(src, SHA_DIGEST_LENGTH, dst);}
    void hash(std::string_view src,         DigestStore& dst)   {THOR_SHA1(reinterpret_cast<Byte const*>(&src[0]), std::size(src), dst);}
    void hash(std::string const& src,       DigestStore& dst)   {THOR_SHA1(reinterpret_cast<Byte const*>(&src[0]), std::size(src), dst);}
    // Use only if you know the destination is large enough!!
    void hashUnsafe(std::string_view src,   DigestPtr dst)      {THOR_SHA1(reinterpret_cast<Byte const*>(&src[0]), std::size(src), dst);}
};
struct Sha256
{
    static constexpr std::size_t digestSize = SHA256_DIGEST_LENGTH;
    using DigestStore = DigestStore<SHA256_DIGEST_LENGTH>;

    void hash(DigestStore& src,             DigestStore& dst)   {THOR_SHA256(src, SHA256_DIGEST_LENGTH, dst);}
    void hash(std::string_view src,         DigestStore& dst)   {THOR_SHA256(reinterpret_cast<Byte const*>(&src[0]), std::size(src), dst);}
    void hash(std::string const& src,       DigestStore& dst)   {THOR_SHA256(reinterpret_cast<Byte const*>(&src[0]), std::size(src), dst);}
    // Use only if you know the destination is large enough!
    void hashUnsafe(std::string_view src,   Digestptr dst)      {THOR_SHA256(reinterpret_cast<Byte const*>(&src[0]), std::size(src), dst);}
};

}

#endif
\$\endgroup\$
3
  • 1
    \$\begingroup\$ @Emma I had not. Looks like I will need to start using one of those. But I just hate reading these badly written C++ (which is why I started this hack) projects that are crappy wrappers around C rather than using good type safety and nice clean C++ interfaces and techniques. Though I did enjoy writting these if I do any hard core stuff I will definetly use a proper library but these simple classes will let me do some quick checks (which is currently writting a C++ Mongo interface (and yes I know one exists but I can write a better one). Using ThorsSerializer to automate the serialization. \$\endgroup\$ Commented Aug 25, 2020 at 21:02
  • 1
    \$\begingroup\$ too clean. I'll take it. \$\endgroup\$ Commented Aug 25, 2020 at 22:23
  • 1
    \$\begingroup\$ @Emma: Priority 1: Impossible (or really hard for the user to break) to break. 2: Ease of use. 3: Speed. \$\endgroup\$ Commented Aug 25, 2020 at 22:24

2 Answers 2

Reset to default
3
\$\begingroup\$

Proper capitalization of SHA1

The algorithm's name is SHA1, not Sha1, so I think it is better to use all caps here. That makes grepping the code for a particular algorithm easier.

You only need one class per hash algorithm

Indeed, as you mentioned in your own answer, the Sha1 class seems superfluous, since it doesn't store any state. However, instead of creating static functions inside a Sha1 namespace, you could make those functions member functions of the class that holds the actual state. This avoids repeating the type; for example:

Sha1::DigestStore digest;
Sha1::hash("Bob", digest)

Becomes:

Sha1::DigestStore digest;
digest.hash("Bob");

Sha1::Digest vs. Digest<Sha1>

I think having a namespace Sha1 with a DigestStore and functions inside it is a bad choice. There is more you can do with SHA1 than just create plain hashes, for example you might want to create a HMAC instead of a plain hash. So you would have to add functions to create a HMAC to each namespace that implements a hash algorithm. It's much better to have classes Digest and HMAC that are templated on the hash algorithm.

Allow hashes to be updated

The code you wrote only performs one-shot conversions of some input to a hash. However, it is not uncommon for programs to not have all the data they want to create a hash for in a single, contiguous memory region. In those cases, you want to write:

std::ostream output;
Digest<SHA1> digest;

digest.add("Header");
digest.add("Data");
digest.add("Footer");

output << digest.view();

Some digest algorithms might require you to call some function to calculate the final hash value after adding all the data. You could add an explicit finish() function or call this implicitly when accessing the digest result.

Getting the result out

You internally store the hash as a std::array<std::byte, size>. That's the proper thing to do. I don't think it is necessary to provide any member functions other than one that gets you a const reference to that array. It's up to the caller to convert it to whatever form they way. A std::array is already implicitly convertible to a std::span. And once you have a reference to the array, it's easy to get the begin and end iterators from it.

Add comparison operators

It's quite common to want to check whether two hashes are identical, so it would be helpful to at least define operator==() and operator!=() to the class that holds the digest result.

\$\endgroup\$
3
  • \$\begingroup\$ I had to think about it overnight. But I think I like the idea of putting the hash() method on the Digest object. Not 100% sure how to design it yet. But look for a review soon. :-) \$\endgroup\$ Commented Aug 27, 2020 at 17:22
  • \$\begingroup\$ I wanted to name the types SHA1 but an all uppercase name makes my skin crawl (thinking of Macros). Not sure how to proceed here, I have to overcome my dread. \$\endgroup\$ Commented Aug 27, 2020 at 17:23
  • \$\begingroup\$ I was planning on adding an ability to update (add more text) as its a common feature for hashing. Currently I did not do it because I was only doing it for short pieces of text not document sized text. But long term that is definitely something I want to add; I just don't have a use case that I can experiment on with the interface. \$\endgroup\$ Commented Aug 27, 2020 at 17:26
1
\$\begingroup\$

I think I am going to change the interface.

Currently the usage patter is:

typename Sha1::DigestStore      digest;
Sha1                            hasher;

hasher.hash("Bob", digest);

There does not seem a need to create a Sha1 object. I think a better interface may be to make all the methods static so the usage becomes:

typename Sha1::DigestStore      digest;

Sha1::hash("Bob", digest);

The DigestStore may need some other accesses functions. It currently allows iteration but there can be a use case where we have a const_iterator.

Still trying to understand when best to use string_view. Unfortunately it still does not play well with normal strings. So we may have to provide a way to also extract a string from the buffer. In that case it would be nice if we could have the data from the DigestStore into a string (which means not using std::array) but need to have a good use case to make that work better.

Not sure how that will work yet. Please provide a hint if you have am idea.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.