Converting to and from Base64 using OpenSSL API

Question

I am trying to get a working and good version of base64 conversions using OpenSSL. My alternatives were an implementation from an answer on SO and Boost (which I didn't chose because I read that is pretty broken).

I will need OpenSSL in my project and I thought it would be a good idea to use it for base64 too so I can get use with the API.

For start, let's just assume:

using ByteBuffer = std::vector<std::uint8_t>;

This is the header (redi is the main namespace of my project):

#ifndef REDI_BASE64_HPP
#define REDI_BASE64_HPP

#include "../bytebuffer.hpp"

namespace redi
{
namespace util
{
namespace base64
{

ByteBuffer encode(const ByteBuffer& data);
ByteBuffer encode(const std::string& str);
std::string encodeToString(const std::string& str);

ByteBuffer decode(const ByteBuffer& data);
ByteBuffer decode(const std::string& str);
std::string decodeToString(const ByteBuffer& data);
std::string decodeToString(const std::string& str);

} // namespace base64
} // namespace util
} // namespace redi


#endif // REDI_BASE64_HPP

and here is the source file:

#include <cassert>
#include <string>
#include <vector>
#include <openssl/bio.h>
#include <openssl/buffer.h>
#include <openssl/evp.h>
#include "base64.hpp"

namespace redi
{
namespace util
{
namespace base64
{
namespace
{

void rawBase64Encode(const std::uint8_t* input, std::size_t inputLen, char*& output, std::size_t& outputLen)
{
  BIO* bio;
  BIO* b64;
  BUF_MEM* bufferPtr;

  b64 = BIO_new(BIO_f_base64());
  bio = BIO_new(BIO_s_mem());
  bio = BIO_push(b64, bio);

  BIO_set_flags(bio, BIO_FLAGS_BASE64_NO_NL);
  BIO_write(bio, input, static_cast<int>(inputLen));
  BIO_flush(bio);
  BIO_get_mem_ptr(bio, std::addressof(bufferPtr));
  BIO_set_close(bio, BIO_NOCLOSE);

  output = bufferPtr->data;
  outputLen = bufferPtr->length;
  free(bufferPtr);

  BIO_free_all(bio);
}

std::size_t calcDecodeLength(const std::uint8_t* input, std::size_t len)
{
  std::size_t padding = 0;

  if (input[len - 1] == '=')
  {
    ++padding;
    if (input[len - 2] == '=') ++padding;
  }

  return len * 3 / 4 - padding;
}

template <typename T>
void rawBase64Decode(const std::uint8_t* input, std::size_t inputLen, T& container)
{
  BIO* bio;
  BIO* b64;
  container.resize(calcDecodeLength(input, inputLen));

  bio = BIO_new_mem_buf(input, -1);
  b64 = BIO_new(BIO_f_base64());
  bio = BIO_push(b64, bio);

  BIO_set_flags(bio, BIO_FLAGS_BASE64_NO_NL);
  int l = BIO_read(bio, std::addressof(container[0]), static_cast<int>(inputLen));
  assert(static_cast<std::size_t>(l) == container.size());

  BIO_free_all(bio);
}

template <typename T, typename K>
T decodeInternal(const K& data)
{
  T result;
  rawBase64Decode(reinterpret_cast<const std::uint8_t*>(data.data()), data.size(), result);

  return result;
}

template <typename T, typename K>
T encodeInternal(const K& data)
{
  char* ptr = nullptr;
  std::size_t len = 0;
  rawBase64Encode(reinterpret_cast<const std::uint8_t*>(data.data()), data.size(), ptr, len);

  T result(ptr, len);
  free(ptr);

  return result;
}

}

ByteBuffer encode(const ByteBuffer& data)
{
  return encodeInternal<ByteBuffer, ByteBuffer>(data);
}

ByteBuffer encode(const std::string& str)
{
  return encodeInternal<ByteBuffer, std::string>(str);
}

std::string encodeToString(const std::string& str)
{
  return encodeInternal<std::string, std::string>(str);
}

ByteBuffer decode(const ByteBuffer& data)
{
  return decodeInternal<ByteBuffer, ByteBuffer>(data);
}

ByteBuffer decode(const std::string& str)
{
  return decodeInternal<ByteBuffer, std::string>(str);
}

std::string decodeToString(const ByteBuffer& data)
{
  return decodeInternal<std::string, ByteBuffer>(data);
}

std::string decodeToString(const std::string& str)
{
  return decodeInternal<std::string, std::string>(str);
}

} // namespace base64
} // namespace util
} // namespace redi

All of this has been "combined" from answers on SO and example on gist.

I would like to know if is a good design, if I can make any optimization (example: avoid copying the char* into the ByteBuffer, for instance) in encode, and, last but not least, if I have any memory leaks.

Answer 1

I would like to know if is a good design

A few points about your design from a c++ perspective:

1. Usage of free functions instead of classes

From the point of OOP view, you should rather use classes than free functions to wrap / encapsulate the C API:

namespace redi
{
namespace util
{
namespace base64
{

    class Encoder {
    public:
        Encoder(const ByteBuffer& data);
        Encoder(const std::string& str);
        ByteBuffer operator(); // Do the encoding
        std::string to_string();
    };

    class Decoder {
    public:
        Decoder(const ByteBuffer& data);
        Decoder(const std::string& str);
        ByteBuffer operator(); // Do the decoding
        std::string to_string();
    };

} // namespace base64
} // namespace util
} // namespace redi

2. Prefer class specializations over introducing a namespace

Using classes as mentioned above you should prefer specializing classes (may be interface implementations) rather introducing a new namespace for a specific data category:

namespace redi
{
namespace util
{

    class Base64Encoder;
       // ^^^^^^
    class Base64Decoder;
       // ^^^^^^    
} // namespace util
} // namespace redi

Same for the function naming:

void rawBase64Encode(const std::uint8_t* input, std::size_t inputLen, char*& output, std::size_t& outputLen);

and

void rawBase64Decode(const std::uint8_t* input, std::size_t inputLen, T& container);

will become

void Base64Encoder::rawEncode();

and

void Base64Decoder::rawDecode();

as (probably private) class member functions.

3. Don't use using or typedef unnecessarily in published API's

The

using ByteBuffer = std::vector<std::uint8_t>;

obfuscates more, and leaves research efforts to API user, than it helps to save typing more code for the API provider.

Also I wouldn't provide that separately from the encoding / decoding features, but may be as a public typedef provided with classes.

4. In c++ encapsulate c-API handles

Your code all depends on a BIO* bio; API handle, at least internally

As mentioned above rather encapsulate that into a class. May be even into a common base class used for both, encoding and decoding:

 class Base64EncoderDecoderBase {
 protected:
      Bio* bio;
 };


 class Base64Encode : public Base64EncoderDecoderBase {
     // ...
 };

 class Base64Decode : public Base64EncoderDecoderBase {
     // ...
 };

5. Are you sure you need to use OpenSSL?

As suggested in this answer are you sure you really need the OpenSS Bio structure, or that could be left to a simpler solution?

6. You probably don't need to copy

As mentioned in my comment you probably don't need to have copies to interact with the OppenSSL library.
Using the data()members of these container classes directly, may be helpful to avoid that (not sure for the Bio* context though).