Adding HTTP Basic Auth support?

[mmd-osm]

Initially discussed here: #140 (comment)

As this topic could also be relevant in the context of GDPR, I'm creating a separate issue for it. After all, people might want to use e.g. the /map call along with Basic Auth to get full meta data.

Although cgimap supports OAuth for viewing redacted versions of historical elements, it doesn't support HTTP Basic Auth. Only moderators use the redacted-history API, and they can probably live without HTTP Basic Auth. But there might be other people using Basic for the regular user API. Do we want to support that too? Or work with something like cgimap-ruby to allow the auth to be handled at a higher level?

[mmd-osm]

Just adding a few pointers to the corresponding Rails port implementation:

User auth call:
https://github.com/openstreetmap/openstreetmap-website/blob/master/app/models/user.rb#L130

Library functions:

https://github.com/openstreetmap/openstreetmap-website/blob/master/lib/password_hash.rb#L18-L30
https://github.com/openstreetmap/openstreetmap-website/blob/master/lib/password_hash.rb#L48-L52

Create and upgrade doesn't seem necessary for cgimap.

Test cases:

https://github.com/openstreetmap/openstreetmap-website/blob/master/test/lib/password_hash_test.rb

[mmd-osm]

I ported the library function check and the test cases to c++ / crypto++ now. Creating and upgrading passwords is deliberately left to the Rails port. Now, finding a good place for parsing the Basic Auth header and reading all relevant fields from users table are still left to do.

#define CRYPTOPP_ENABLE_NAMESPACE_WEAK 1

#include <crypto++/base64.h>
#include <crypto++/config.h>
#include <crypto++/filters.h>
#include <crypto++/hex.h>
#include <crypto++/md5.h>
#include <crypto++/pwdbased.h>
#include <crypto++/secblock.h>
#include <crypto++/sha.h>
#include <sys/types.h>
#include <cassert>
#include <iostream>
#include <string>

#include <boost/algorithm/string.hpp>

using namespace CryptoPP;

class PasswordHash {
 public:
  static bool check(const std::string& pass_crypt, const std::string& pass_salt,
                    const std::string& candidate) {
    std::string hashed_candidate;

    if (pass_salt.empty()) {
      const auto digest = md5_hash(candidate);
      return (boost::iequals(digest, pass_crypt));
    }

    std::vector<std::string> params;  // fields: algorithm, iterations, salt
    boost::split(params, pass_salt, boost::is_any_of("!"));

    if (params.size() != 3) {
      const auto digest = md5_hash(pass_salt + candidate);
      return (boost::iequals(digest, pass_crypt));
    }

    const auto algorithm = params[0];
    const auto iterations = stoi(params[1]);
    const auto salt = params[2];

    const auto pass_crypt_decoded = base64decode(pass_crypt);

    if (algorithm == "sha512")
      hashed_candidate = PBKDF2_HMAC_SHA_string<SHA512>(
          candidate, salt, iterations, pass_crypt_decoded.size());
    else
      throw std::runtime_error("Unknown hash algorithm");

    return (hashed_candidate == pass_crypt);  // both strings are base64 encoded
  }

 private:
  template <class T>
  static std::string PBKDF2_HMAC_SHA_string(const std::string& pass,
                                            const std::string& salt,
                                            const uint iterations,
                                            const uint outputBytes) {
    SecByteBlock result(outputBytes);
    std::string base64Result;

    PKCS5_PBKDF2_HMAC<T> pbkdf;

    pbkdf.DeriveKey(result, result.size(), 0x00, (byte*)pass.data(),
                    pass.size(), (byte*)salt.data(), salt.size(), iterations);

    ArraySource resultEncoder(
        result, result.size(), true,
        new Base64Encoder(new StringSink(base64Result), false));

    return base64Result;
  }

  static std::string md5_hash(const std::string& s) {
    Weak::MD5 hash;
    std::string digest;
    StringSource ss(
        s, true,
        new HashFilter(hash, new HexEncoder(new StringSink(digest), false)));
    return digest;
  }

  static std::string base64decode(const std::string& s) {
    std::string result;
    StringSource ss(s, true, new Base64Decoder(new StringSink(result)));
    return result;
  }
};

int main(int argc, char* argv[]) {
  std::cout << "Testing PasswordHash...";

  // test_md5_without_salt
  assert(PasswordHash::check("5f4dcc3b5aa765d61d8327deb882cf99", "",
                             "password") == true);
  assert(PasswordHash::check("5f4dcc3b5aa765d61d8327deb882cf99", "", "wrong") ==
         false);

  // test_md5_with_salt
  assert(PasswordHash::check("67a1e09bb1f83f5007dc119c14d663aa", "salt",
                             "password") == true);
  assert(PasswordHash::check("67a1e09bb1f83f5007dc119c14d663aa", "salt",
                             "wrong") == false);
  assert(PasswordHash::check("67a1e09bb1f83f5007dc119c14d663aa", "wrong",
                             "password") == false);

  // test_pbkdf2_1000_32_sha512
  assert(PasswordHash::check(
             "ApT/28+FsTBLa/J8paWfgU84SoRiTfeY8HjKWhgHy08=",
             "sha512!1000!HR4z+hAvKV2ra1gpbRybtoNzm/CNKe4cf7bPKwdUNrk=",
             "password") == true);

  assert(PasswordHash::check(
             "ApT/28+FsTBLa/J8paWfgU84SoRiTfeY8HjKWhgHy08=",
             "sha512!1000!HR4z+hAvKV2ra1gpbRybtoNzm/CNKe4cf7bPKwdUNrk=",
             "wrong") == false);

  assert(PasswordHash::check(
             "ApT/28+FsTBLa/J8paWfgU84SoRiTfeY8HjKWhgHy08=",
             "sha512!1000!HR4z+hAvKV2ra1gwrongtoNzm/CNKe4cf7bPKwdUNrk=",
             "password") == false);

  // test_pbkdf2_10000_32_sha512
  assert(PasswordHash::check(
             "3wYbPiOxk/tU0eeIDjUhdvi8aDP3AbFtwYKKxF1IhGg=",
             "sha512!10000!OUQLgtM7eD8huvanFT5/WtWaCwdOdrir8QOtFwxhO0A=",
             "password") == true);

  assert(PasswordHash::check(
             "3wYbPiOxk/tU0eeIDjUhdvi8aDP3AbFtwYKKxF1IhGg=",
             "sha512!10000!OUQLgtM7eD8huvanFT5/WtWaCwdOdrir8QOtFwxhO0A=",
             "wrong") == false);

  assert(PasswordHash::check(
             "3wYbPiOxk/tU0eeIDjUhdvi8aDP3AbFtwYKKxF1IhGg=",
             "sha512!10000!OUQLgtMwronguvanFT5/WtWaCwdOdrir8QOtFwxhO0A=",
             "password") == false);

  std::cout << "finished." << std::endl;

  return 0;
}

[tomhughes]

Do we really want to be adding basic auth here? Ideally we'd be phasing it out, not extending it...

[mmd-osm]

Right, this is a bit guided by the following comment by @pnorman in #140 (comment):

I expect there's a fair number of editors using that. Some even over HTTP.

Right now, for feature parity, we'd need to either keep HTTP Basic requests going to the rails port or support them in cgimap.

I wouldn't be opposed to dropping basic auth support, and would support dropping basic auth without HTTPS, but this issue tracker isn't the right place to make that policy change.

As we don't want to funnel all those calls through the Rails port or cgimap-ruby, I see this as an intermediate step to be still somewhat downward compatible when GDPR restrictions or the changeset upload need an authenticated user.

Fully agree though on a more mid term goal to phase out Basic auth.

[pnorman]

Do we really want to be adding basic auth here? Ideally we'd be phasing it out, not extending it...

If we want to phase it out, we should do that in the rails port. But we don't have any plans for that yet, do we?

---

I wouldn't be opposed to dropping basic auth support, and would support dropping basic auth without HTTPS, but this issue tracker isn't the right place to make that policy change.

The basic auth without HTTPS point is now moot

[mmd-osm]

Basic auth support has been added in #152 now.