address review comments

google/cloud/CMakeLists.txt

@@ -665,9 +665,7 @@ if (GOOGLE_CLOUD_CPP_ENABLE_REST)
         internal/rest_request.cc
         internal/rest_request.h
         internal/rest_response.cc
-        internal/rest_response.h
-        internal/write_base64.cc
-        internal/write_base64.h)
+        internal/rest_response.h)
     target_link_libraries(
         google_cloud_cpp_rest_internal
         PUBLIC absl::span google-cloud-cpp::common CURL::libcurl

google/cloud/google_cloud_cpp_rest_internal.bzl

@@ -38,7 +38,6 @@ google_cloud_cpp_rest_internal_hdrs = [
     "internal/rest_options.h",
     "internal/rest_request.h",
     "internal/rest_response.h",
-    "internal/write_base64.h",
 ]
 
 google_cloud_cpp_rest_internal_srcs = [
@@ -60,5 +59,4 @@ google_cloud_cpp_rest_internal_srcs = [
     "internal/rest_client.cc",
     "internal/rest_request.cc",
     "internal/rest_response.cc",
-    "internal/write_base64.cc",
 ]

google/cloud/internal/make_jwt_assertion.cc

@@ -25,8 +25,7 @@ StatusOr<std::string> MakeJWTAssertionNoThrow(std::string const& header,
                                               std::string const& pem_contents) {
   auto const body =
       UrlsafeBase64Encode(header) + '.' + UrlsafeBase64Encode(payload);
-  auto pem_signature = internal::SignStringWithPem(
-      body, pem_contents, oauth2_internal::JwtSigningAlgorithms::RS256);
+  auto pem_signature = internal::SignUsingSha256(body, pem_contents);
   if (!pem_signature) return std::move(pem_signature).status();
   return body + '.' + UrlsafeBase64Encode(*pem_signature);
 }

google/cloud/internal/oauth2_credential_constants.h

@@ -23,15 +23,6 @@ namespace cloud {
 namespace oauth2_internal {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 
-/**
- * Supported signing algorithms used in JWT auth flows.
- *
- * We currently only support RSA with SHA-256, but use this enum for
- * readability and easy addition of support for other algorithms.
- */
-// NOLINTNEXTLINE(readability-identifier-naming)
-enum class JwtSigningAlgorithms { RS256 };
-
 /// The max lifetime in seconds of an access token.
 constexpr std::chrono::seconds GoogleOAuthAccessTokenLifetime() {
   return std::chrono::seconds(3600);

google/cloud/internal/openssl_util.cc

@@ -43,26 +43,8 @@ inline std::unique_ptr<EVP_MD_CTX, decltype(&EVP_MD_CTX_free)> GetDigestCtx() {
 #endif
 }  // namespace
 
-StatusOr<std::vector<std::uint8_t>> Base64Decode(std::string const& str) {
-  return google::cloud::internal::Base64DecodeToBytes(str);
-}
-
-std::string Base64Encode(std::string const& str) {
-  google::cloud::internal::Base64Encoder enc;
-  for (auto c : str) enc.PushBack(c);
-  return std::move(enc).FlushAndPad();
-}
-
-std::string Base64Encode(std::vector<std::uint8_t> const& bytes) {
-  google::cloud::internal::Base64Encoder enc;
-  for (auto c : bytes) enc.PushBack(c);
-  return std::move(enc).FlushAndPad();
-}
-
-StatusOr<std::vector<std::uint8_t>> SignStringWithPem(
-    std::string const& str, std::string const& pem_contents,
-    oauth2_internal::JwtSigningAlgorithms alg) {
-  using ::google::cloud::oauth2_internal::JwtSigningAlgorithms;
+StatusOr<std::vector<std::uint8_t>> SignUsingSha256(
+    std::string const& str, std::string const& pem_contents) {
 
   auto digest_ctx = GetDigestCtx();
   if (!digest_ctx) {
@@ -71,12 +53,7 @@ StatusOr<std::vector<std::uint8_t>> SignStringWithPem(
                   "could not create context for OpenSSL digest. ");
   }
 
-  EVP_MD const* digest_type = nullptr;
-  switch (alg) {
-    case JwtSigningAlgorithms::RS256:
-      digest_type = EVP_sha256();
-      break;
-  }
+  EVP_MD const* digest_type = EVP_sha256();
   if (digest_type == nullptr) {
     return Status(StatusCode::kInvalidArgument,
                   "Invalid ServiceAccountCredentials: "
@@ -167,21 +144,7 @@ StatusOr<std::vector<std::uint8_t>> UrlsafeBase64Decode(
   } else if (b64str.length() % 4 == 3) {
     b64str.append("=");
   }
-  return Base64Decode(b64str);
-}
-
-std::vector<std::uint8_t> MD5Hash(std::string const& payload) {
-  MD5_CTX md5;
-  MD5_Init(&md5);
-  MD5_Update(&md5, payload.c_str(), payload.size());
-
-  std::vector<std::uint8_t> hash(MD5_DIGEST_LENGTH, 0);
-  // Note: MD5_Final consumes a `unsigned char*` in its first parameter, on some
-  // platforms (PowerPC and ARM I read), the default `char` is unsigned. In
-  // those platforms it is possible that `std::uint8_t != unsigned char` and
-  // the `reinterpret_cast<>` is not trivial (but still safe I think).
-  MD5_Final(reinterpret_cast<unsigned char*>(hash.data()), &md5);
-  return hash;
+  return Base64DecodeToBytes(b64str);
 }
 
 }  // namespace internal

google/cloud/internal/openssl_util.h

@@ -15,10 +15,9 @@
 #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_INTERNAL_OPENSSL_UTIL_H
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_INTERNAL_OPENSSL_UTIL_H
 
-#include "google/cloud/internal/oauth2_credential_constants.h"
+#include "google/cloud/internal/base64_transforms.h"
 #include "google/cloud/status_or.h"
 #include "google/cloud/version.h"
-#include <algorithm>
 #include <string>
 #include <vector>
 
@@ -27,31 +26,15 @@ namespace cloud {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace internal {
 
-/**
- * Decodes a Base64-encoded string.
- */
-StatusOr<std::vector<std::uint8_t>> Base64Decode(std::string const& str);
-
-/**
- * Encodes a string using Base64.
- */
-std::string Base64Encode(std::string const& str);
-
-/**
- * Encodes a byte array using Base64.
- */
-std::string Base64Encode(std::vector<std::uint8_t> const& bytes);
-
 /**
  * Signs a string with the private key from a PEM container.
  *
  * @return Returns the signature as an *unencoded* byte array. The caller
  *   might want to use `Base64Encode()` or `HexEncode()` to convert this byte
  *   array to a format more suitable for transmission over HTTP.
  */
-StatusOr<std::vector<std::uint8_t>> SignStringWithPem(
-    std::string const& str, std::string const& pem_contents,
-    oauth2_internal::JwtSigningAlgorithms alg);
+StatusOr<std::vector<std::uint8_t>> SignUsingSha256(
+    std::string const& str, std::string const& pem_contents);
 
 /**
  * Returns a Base64-encoded version of @p bytes. Using the URL- and
@@ -62,7 +45,9 @@ StatusOr<std::vector<std::uint8_t>> SignStringWithPem(
  */
 template <typename Collection>
 inline std::string UrlsafeBase64Encode(Collection const& bytes) {
-  std::string b64str = Base64Encode(bytes);
+  Base64Encoder encoder;
+  for (auto c : bytes) encoder.PushBack(c);
+  std::string b64str = std::move(encoder).FlushAndPad();
   std::replace(b64str.begin(), b64str.end(), '+', '-');
   std::replace(b64str.begin(), b64str.end(), '/', '_');
   auto end_pos = b64str.find_last_not_of('=');
@@ -77,9 +62,6 @@ inline std::string UrlsafeBase64Encode(Collection const& bytes) {
  */
 StatusOr<std::vector<std::uint8_t>> UrlsafeBase64Decode(std::string const& str);
 
-/// Compute the MD5 hash of @p payload
-std::vector<std::uint8_t> MD5Hash(std::string const& payload);
-
 }  // namespace internal
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace cloud

google/cloud/internal/openssl_util_test.cc

@@ -13,7 +13,6 @@
 // limitations under the License.
 
 #include "google/cloud/internal/openssl_util.h"
-#include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 
 namespace google {
@@ -22,18 +21,16 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace internal {
 namespace {
 
-using ::google::cloud::testing_util::StatusIs;
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 
-TEST(OpensslUtilTest, Base64Encode) {
+TEST(OpensslUtilTest, UrlsafeBase64Encode) {
   // Produced input using:
   //     echo 'TG9yZ+W0gaXBz/dW1cMACg==' | openssl base64 -d | od -t x1
   std::vector<std::uint8_t> input{
       0x4c, 0x6f, 0x72, 0x67, 0xe5, 0xb4, 0x81, 0xa5,
       0xc1, 0xcf, 0xf7, 0x56, 0xd5, 0xc3, 0x00, 0x0a,
   };
-  EXPECT_EQ("TG9yZ+W0gaXBz/dW1cMACg==", Base64Encode(input));
   EXPECT_EQ("TG9yZ-W0gaXBz_dW1cMACg", UrlsafeBase64Encode(input));
 }
 
@@ -44,16 +41,10 @@ TEST(OpensslUtilTest, Base64Decode) {
       0x4c, 0x6f, 0x72, 0x67, 0xe5, 0xb4, 0x81, 0xa5,
       0xc1, 0xcf, 0xf7, 0x56, 0xd5, 0xc3, 0x00, 0x0a,
   };
-  EXPECT_THAT(Base64Decode("TG9yZ+W0gaXBz/dW1cMACg==").value(),
-              ElementsAreArray(expected));
   EXPECT_THAT(UrlsafeBase64Decode("TG9yZ-W0gaXBz_dW1cMACg").value(),
               ElementsAreArray(expected));
 }
 
-TEST(OpensslUtilTest, Base64Failure) {
-  EXPECT_THAT(Base64Decode("xx"), StatusIs(StatusCode::kInvalidArgument));
-}
-
 TEST(OpensslUtilTest, Base64DecodePadding) {
   // Produced input using:
   // cSpell:disable
@@ -67,39 +58,13 @@ TEST(OpensslUtilTest, Base64DecodePadding) {
   // QUJDRAo=
   // cSpell:enable
 
-  EXPECT_THAT(Base64Decode("QQ==").value(), ElementsAre('A'));
   EXPECT_THAT(UrlsafeBase64Decode("QQ").value(), ElementsAre('A'));
-  EXPECT_THAT(Base64Decode("QUI=").value(), ElementsAre('A', 'B'));
   EXPECT_THAT(UrlsafeBase64Decode("QUI").value(), ElementsAre('A', 'B'));
-  EXPECT_THAT(Base64Decode("QUJD").value(), ElementsAre('A', 'B', 'C'));
   EXPECT_THAT(UrlsafeBase64Decode("QUJD").value(), ElementsAre('A', 'B', 'C'));
-  EXPECT_THAT(Base64Decode("QUJDRA==").value(),
-              ElementsAre('A', 'B', 'C', 'D'));
   EXPECT_THAT(UrlsafeBase64Decode("QUJDRA").value(),
               ElementsAre('A', 'B', 'C', 'D'));
 }
 
-TEST(OpensslUtilTest, MD5HashEmpty) {
-  auto const actual = MD5Hash("");
-  // I used this command to get the expected value:
-  // /bin/echo -n "" | openssl md5
-  auto const expected =
-      std::vector<std::uint8_t>{0xd4, 0x1d, 0x8c, 0xd9, 0x8f, 0x00, 0xb2, 0x04,
-                                0xe9, 0x80, 0x09, 0x98, 0xec, 0xf8, 0x42, 0x7e};
-  EXPECT_THAT(actual, ElementsAreArray(expected));
-}
-
-TEST(OpensslUtilTest, MD5HashSimple) {
-  auto const actual = MD5Hash("The quick brown fox jumps over the lazy dog");
-  // I used this command to get the expected value:
-  // /bin/echo -n "The quick brown fox jumps over the lazy dog" |
-  //     openssl md5 -binary | openssl base64
-  auto const expected =
-      std::vector<std::uint8_t>{0x9e, 0x10, 0x7d, 0x9d, 0x37, 0x2b, 0xb6, 0x82,
-                                0x6b, 0xd8, 0x1d, 0x35, 0x42, 0xa4, 0x19, 0xd6};
-  EXPECT_THAT(actual, ElementsAreArray(expected));
-}
-
 }  // namespace
 }  // namespace internal
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END