diff --git a/libraries/ESP8266WiFi/src/BearSSLHelpers.cpp b/libraries/ESP8266WiFi/src/BearSSLHelpers.cpp index 08549cc99a..cfdc6bf77b 100644 --- a/libraries/ESP8266WiFi/src/BearSSLHelpers.cpp +++ b/libraries/ESP8266WiFi/src/BearSSLHelpers.cpp @@ -651,21 +651,17 @@ namespace BearSSL { // ----- Public Key ----- PublicKey::PublicKey() { - _key = nullptr; } PublicKey::PublicKey(const char *pemKey) { - _key = nullptr; parse(pemKey); } PublicKey::PublicKey(const uint8_t *derKey, size_t derLen) { - _key = nullptr; parse(derKey, derLen); } PublicKey::PublicKey(Stream &stream, size_t size) { - _key = nullptr; auto buff = brssl::loadStream(stream, size); if (buff) { parse(buff, size); @@ -674,9 +670,6 @@ PublicKey::PublicKey(Stream &stream, size_t size) { } PublicKey::~PublicKey() { - if (_key) { - brssl::free_public_key(_key); - } } bool PublicKey::parse(const char *pemKey) { @@ -684,11 +677,8 @@ bool PublicKey::parse(const char *pemKey) { } bool PublicKey::parse(const uint8_t *derKey, size_t derLen) { - if (_key) { - brssl::free_public_key(_key); - _key = nullptr; - } - _key = brssl::read_public_key((const char *)derKey, derLen); + std::shared_ptr parsed (brssl::read_public_key((const char *)derKey, derLen), [](brssl::public_key *v){ brssl::free_public_key(v); } ); + _key = parsed; return _key ? true : false; } @@ -723,21 +713,17 @@ const br_ec_public_key *PublicKey::getEC() const { // ----- Private Key ----- PrivateKey::PrivateKey() { - _key = nullptr; } PrivateKey::PrivateKey(const char *pemKey) { - _key = nullptr; parse(pemKey); } PrivateKey::PrivateKey(const uint8_t *derKey, size_t derLen) { - _key = nullptr; parse(derKey, derLen); } PrivateKey::PrivateKey(Stream &stream, size_t size) { - _key = nullptr; auto buff = brssl::loadStream(stream, size); if (buff) { parse(buff, size); @@ -746,9 +732,6 @@ PrivateKey::PrivateKey(Stream &stream, size_t size) { } PrivateKey::~PrivateKey() { - if (_key) { - brssl::free_private_key(_key); - } } bool PrivateKey::parse(const char *pemKey) { @@ -756,11 +739,8 @@ bool PrivateKey::parse(const char *pemKey) { } bool PrivateKey::parse(const uint8_t *derKey, size_t derLen) { - if (_key) { - brssl::free_private_key(_key); - _key = nullptr; - } - _key = brssl::read_private_key((const char *)derKey, derLen); + std::shared_ptr parsed (brssl::read_private_key((const char *)derKey, derLen), [](brssl::private_key *v){ brssl::free_private_key(v); } ); + _key = parsed; return _key ? true : false; } @@ -795,30 +775,18 @@ const br_ec_private_key *PrivateKey::getEC() const { // ----- Certificate Lists ----- X509List::X509List() { - _count = 0; - _cert = nullptr; - _ta = nullptr; } X509List::X509List(const char *pemCert) { - _count = 0; - _cert = nullptr; - _ta = nullptr; append(pemCert); } X509List::X509List(const uint8_t *derCert, size_t derLen) { - _count = 0; - _cert = nullptr; - _ta = nullptr; append(derCert, derLen); } X509List::X509List(Stream &stream, size_t size) { - _count = 0; - _cert = nullptr; - _ta = nullptr; auto buff = brssl::loadStream(stream, size); if (buff) { append(buff, size); @@ -827,11 +795,13 @@ X509List::X509List(Stream &stream, size_t size) { } X509List::~X509List() { - brssl::free_certificates(_cert, _count); // also frees cert + brssl::free_certificates(_cert.get(), _count); // also frees cert for (size_t i = 0; i < _count; i++) { - brssl::free_ta_contents(&_ta[i]); + brssl::free_ta_contents(&_ta.get()[i]); } - free(_ta); + free(_ta.get()); + _cert.release(); + _ta.release(); } bool X509List::append(const char *pemCert) { @@ -846,47 +816,48 @@ bool X509List::append(const uint8_t *derCert, size_t derLen) { } // Add in the certificates - br_x509_certificate *saveCert = _cert; - _cert = (br_x509_certificate*)realloc(_cert, (numCerts + _count) * sizeof(br_x509_certificate)); - if (!_cert) { + auto newCert = (br_x509_certificate*)realloc(_cert.get(), (numCerts + _count) * sizeof(br_x509_certificate)); + if (!newCert) { free(newCerts); - _cert = saveCert; return false; } - memcpy(&_cert[_count], newCerts, numCerts * sizeof(br_x509_certificate)); + memcpy(&newCert[_count], newCerts, numCerts * sizeof(br_x509_certificate)); free(newCerts); + _cert.release(); + _cert.reset(newCert); // Build TAs for each certificate - br_x509_trust_anchor *saveTa = _ta; - _ta = (br_x509_trust_anchor*)realloc(_ta, (numCerts + _count) * sizeof(br_x509_trust_anchor)); - if (!_ta) { - _ta = saveTa; + auto newTa = (br_x509_trust_anchor*)realloc(_ta.get(), (numCerts + _count) * sizeof(br_x509_trust_anchor)); + if (!newTa) { return false; } for (size_t i = 0; i < numCerts; i++) { - br_x509_trust_anchor *newTa = brssl::certificate_to_trust_anchor(&_cert[_count + i]); + br_x509_trust_anchor *aTa = brssl::certificate_to_trust_anchor(&_cert.get()[_count + i]); if (newTa) { - _ta[_count + i ] = *newTa; - free(newTa); + newTa[_count + i ] = *aTa; + free(aTa); } else { return false; // OOM } } + _ta.release(); + _ta.reset(newTa); _count += numCerts; return true; } ServerSessions::~ServerSessions() { - if (_isDynamic && _store != nullptr) - delete _store; + if (!_isDynamic) { + _store.release(); + } } ServerSessions::ServerSessions(ServerSession *sessions, uint32_t size, bool isDynamic) : _size(sessions != nullptr ? size : 0), _store(sessions), _isDynamic(isDynamic) { if (_size > 0) - br_ssl_session_cache_lru_init(&_cache, (uint8_t*)_store, size * sizeof(ServerSession)); + br_ssl_session_cache_lru_init(&_cache, (uint8_t*)_store.get(), size * sizeof(ServerSession)); } const br_ssl_session_cache_class **ServerSessions::getCache() { @@ -961,9 +932,9 @@ extern "C" bool thunk_SigningVerifier_verify(PublicKey *_pubKey, UpdaterHashClas bool SigningVerifier::verify(UpdaterHashClass *hash, const void *signature, uint32_t signatureLen) { if (!_pubKey || !hash || !signature || signatureLen != length()) return false; #if !CORE_MOCK - return thunk_SigningVerifier_verify(_pubKey, hash, signature, signatureLen); + return thunk_SigningVerifier_verify(_pubKey.get(), hash, signature, signatureLen); #else - return SigningVerifier_verify(_pubKey, hash, signature, signatureLen); + return SigningVerifier_verify(_pubKey.get(), hash, signature, signatureLen); #endif } diff --git a/libraries/ESP8266WiFi/src/BearSSLHelpers.h b/libraries/ESP8266WiFi/src/BearSSLHelpers.h index 397a557522..64fa617519 100644 --- a/libraries/ESP8266WiFi/src/BearSSLHelpers.h +++ b/libraries/ESP8266WiFi/src/BearSSLHelpers.h @@ -26,6 +26,7 @@ #include #include #include +#include // Internal opaque structures, not needed by user applications namespace brssl { @@ -56,11 +57,8 @@ class PublicKey { const br_rsa_public_key *getRSA() const; const br_ec_public_key *getEC() const; - // Disable the copy constructor, we're pointer based - PublicKey(const PublicKey& that) = delete; - private: - brssl::public_key *_key; + std::shared_ptr _key = nullptr; }; // Holds either a single private RSA or EC key for use when BearSSL wants a secretkey. @@ -84,11 +82,8 @@ class PrivateKey { const br_rsa_private_key *getRSA() const; const br_ec_private_key *getEC() const; - // Disable the copy constructor, we're pointer based - PrivateKey(const PrivateKey& that) = delete; - private: - brssl::private_key *_key; + std::shared_ptr _key = nullptr; }; // Holds one or more X.509 certificates and associated trust anchors for @@ -114,19 +109,29 @@ class X509List { return _count; } const br_x509_certificate *getX509Certs() const { - return _cert; + return _cert.get(); } const br_x509_trust_anchor *getTrustAnchors() const { - return _ta; + return _ta.get(); } - // Disable the copy constructor, we're pointer based - X509List(const X509List& that) = delete; + // Enable move with the unique_ptr members + X509List& operator=(X509List&& that) { + if (this != &that) { + _count = that._count; + _cert.reset(that._cert.get()); + _ta.reset(that._ta.get()); + that._count = 0; + that._cert.release(); + that._ta.release(); + } + return *this; + } private: - size_t _count; - br_x509_certificate *_cert; - br_x509_trust_anchor *_ta; + size_t _count = 0; + std::unique_ptr _cert = nullptr; + std::unique_ptr _ta = nullptr; }; // Opaque object which wraps the BearSSL SSL session to make repeated connections @@ -177,9 +182,9 @@ class ServerSessions { const br_ssl_session_cache_class **getCache(); // Size of the store in sessions. - uint32_t _size; + uint32_t _size = 0; // Store where the information for the sessions are stored. - ServerSession *_store; + std::unique_ptr _store = nullptr; // Whether the store is dynamically allocated. // If this is true, the store needs to be freed in the destructor. bool _isDynamic; @@ -208,11 +213,17 @@ class SigningVerifier : public UpdaterVerifyClass { virtual bool verify(UpdaterHashClass *hash, const void *signature, uint32_t signatureLen) override; public: - SigningVerifier(PublicKey *pubKey) { _pubKey = pubKey; stack_thunk_add_ref(); } - ~SigningVerifier() { stack_thunk_del_ref(); } + SigningVerifier(PublicKey *pubKey) { + std::shared_ptr newKey (pubKey); + _pubKey = newKey; + stack_thunk_add_ref(); + } + ~SigningVerifier() { + stack_thunk_del_ref(); + } private: - PublicKey *_pubKey; + std::shared_ptr _pubKey = nullptr; }; // Stack thunked versions of calls