7
\$\begingroup\$

To do password exchange on the SQL server I need to use SHA-1 to generate a hash (no plain password exchange).

This is done differently on Linux/Mac so I abstracted it slightly.

ThorCryptWrapper.h

#ifndef THORS_ANVIL_MYSQL_DETAILS_THOR_CRYPTO_WRAPPER_H
#define THORS_ANVIL_MYSQL_DETAILS_THOR_CRYPTO_WRAPPER_H

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

namespace ThorsAnvil
{
    namespace MySQL
    {

typedef unsigned char           ThorSHADigestStore[SHA_DIGEST_LENGTH];

inline void thorSHA1(ThorSHADigestStore& dest, ThorSHADigestStore& src)
{
    THOR_SHA1(src, SHA_DIGEST_LENGTH, dest);
}

inline void thorSHA1(ThorSHADigestStore& dest, std::string const& src)
{
    THOR_SHA1(reinterpret_cast<const unsigned char*>(&src[0]), src.length(), dest);
}

    }
}

#endif
\$\endgroup\$
4
  • 2
    \$\begingroup\$ Is there a really good reason you're writing code to use SHA-1? You do realize that collision attacks have been known for quite a while, and actually implemented to produce real collisions, right? This doesn't directly affect use for password hashing (AFAIK, no preimage attack has been published) but still points to a hash that's seriously compromised. If at all possible, I'd advise switching to something newer (e.g., SHA-256 or SHA-3). \$\endgroup\$ Commented Mar 20, 2017 at 17:14
  • 2
    \$\begingroup\$ @JerryCoffin: This is just implementing the MySQL "mysql_native_password" handshake requirements. They use SHA-1 as there specification. SHA1( password ) XOR SHA1( "20-bytes random data from server" <concat> SHA1( SHA1( password ) ) ). Documented here: dev.mysql.com/doc/internals/en/… You can see my use of it here: ThorsSQL Lib: Part 3: Layer 5 HandShake \$\endgroup\$ Commented Mar 20, 2017 at 19:22
  • \$\begingroup\$ For what it's worth: dev.mysql.com/doc/refman/5.6/en/… \$\endgroup\$ Commented Mar 20, 2017 at 20:03
  • \$\begingroup\$ @JerryCoffin: I know that's why in my code review I include: Currently this client does not support that and will throw an exception (A fun task for somebody that want to try :-) ) \$\endgroup\$ Commented Mar 20, 2017 at 20:14

1 Answer 1

3
\$\begingroup\$

There's not a huge amount to review here.


It looks like the Apple code is intended to be a drop-in replacement for OpenSSL, so you could probably just rename to match, rather than creating a new name:

#ifdef  __APPLE__
#define COMMON_DIGEST_FOR_OPENSSL
#include <CommonCrypto/CommonDigest.h>
#define SHA1 CC_SHA1
#else
#include <openssl/sha.h>
#endif

Whichever approach you take, don't forget to #undef the macro when you've finished using it (definitely before the end of the header), to avoid polluting the macro namespace for others.


I would prefer src.data() to &src[0] as the idiomatic way to access a string's characters as an array of char.

And I think I prefer sizeof src to SHA_DIGEST_LENGTH in the first overload, so that the connection is clear.

Is there any reason that src can't be a reference to const in both versions?

inline void thorSHA1(ThorSHADigestStore& dest, ThorSHADigestStore const& src)
{
    THOR_SHA1(src, sizeof src, dest);
}

inline void thorSHA1(ThorSHADigestStore& dest, std::string const& src)
{
    auto const src_bytes = reinterpret_cast<const unsigned char*>(src.data());
    THOR_SHA1(src_bytes, src.length(), dest);
}

I withhold comment on the suitability of SHA-1 for this purpose, as that appears to be something you're stuck with.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.