From a718c2b37c4b2bcfe7254843065417c7b5e03eeb Mon Sep 17 00:00:00 2001
From: mikee47 <mike@sillyhouse.net>
Date: Mon, 11 Feb 2019 22:38:59 +0000
Subject: [PATCH 1/6] Improve memory management of SSL fingerprint data.

Add methods to SSLFingerprints structure to manage allocation/de-allocation of fingerprint data.
Create `SSLValidatorList` class for `TCPClient` and move relevant code into methods.

Take care to set `SSLFingerprints` members to `nullptr` when ownership relinquished (see `TcpClient::pinCertificate()`)

bugfix: In `TcpClient::onSslConnected`, the first matching validator causes checking to stop, which means the validator dataj for any subsequent fingerprints never gets released. This is fixed in the `SSLValidatorList` destructor.
---
 Sming/SmingCore/Network/Http/HttpRequest.cpp  |  6 +-
 Sming/SmingCore/Network/Http/HttpRequest.h    |  6 +-
 Sming/SmingCore/Network/HttpClient.cpp        |  2 +-
 Sming/SmingCore/Network/Ssl/SslFingerprints.h | 86 ++++++++++++++++++-
 Sming/SmingCore/Network/Ssl/SslValidator.cpp  | 51 +++++++++--
 Sming/SmingCore/Network/Ssl/SslValidator.h    | 26 +++++-
 Sming/SmingCore/Network/TcpClient.cpp         | 24 ++----
 Sming/SmingCore/Network/TcpClient.h           | 16 ++--
 samples/Basic_Ssl/app/application.cpp         | 20 ++---
 samples/HttpClient/app/application.cpp        | 18 ++--
 10 files changed, 190 insertions(+), 65 deletions(-)

diff --git a/Sming/SmingCore/Network/Http/HttpRequest.cpp b/Sming/SmingCore/Network/Http/HttpRequest.cpp
index b0a72e47fb..223d1fb5c9 100644
--- a/Sming/SmingCore/Network/Http/HttpRequest.cpp
+++ b/Sming/SmingCore/Network/Http/HttpRequest.cpp
@@ -28,7 +28,7 @@ HttpRequest::HttpRequest(const HttpRequest& value) : uri(value.uri)
 
 #ifdef ENABLE_SSL
 	sslOptions = value.sslOptions;
-	sslFingerprint = value.sslFingerprint;
+	sslFingerprints = value.sslFingerprints;
 	sslKeyCertPair = value.sslKeyCertPair;
 #endif
 }
@@ -163,9 +163,9 @@ String HttpRequest::toString()
 #ifdef ENABLE_SSL
 	content += F("> SSL options: ") + String(sslOptions) + '\n';
 	content +=
-		F("> SSL Cert Fingerprint Length: ") + String((sslFingerprint.certSha1 == nullptr) ? 0 : SHA1_SIZE) + '\n';
+		F("> SSL Cert Fingerprint Length: ") + String((sslFingerprints.certSha1 == nullptr) ? 0 : SHA1_SIZE) + '\n';
 	content +=
-		F("> SSL PK Fingerprint Length: ") + String((sslFingerprint.pkSha256 == nullptr) ? 0 : SHA256_SIZE) + '\n';
+		F("> SSL PK Fingerprint Length: ") + String((sslFingerprints.pkSha256 == nullptr) ? 0 : SHA256_SIZE) + '\n';
 	content += F("> SSL ClientCert Length: ") + String(sslKeyCertPair.getCertificateLength()) + '\n';
 	content += F("> SSL ClientCert PK Length: ") + String(sslKeyCertPair.getKeyLength()) + '\n';
 	content += '\n';
diff --git a/Sming/SmingCore/Network/Http/HttpRequest.h b/Sming/SmingCore/Network/Http/HttpRequest.h
index 15c3f48032..6da449d263 100644
--- a/Sming/SmingCore/Network/Http/HttpRequest.h
+++ b/Sming/SmingCore/Network/Http/HttpRequest.h
@@ -232,9 +232,9 @@ class HttpRequest
 	 *
 	 * @return bool  true of success, false or failure
 	 */
-	HttpRequest* pinCertificate(const SslFingerprints& fingerprints)
+	HttpRequest* pinCertificate(SslFingerprints& fingerprints)
 	{
-		sslFingerprint = fingerprints;
+		sslFingerprints = fingerprints;
 		return this;
 	}
 
@@ -284,7 +284,7 @@ class HttpRequest
 
 #ifdef ENABLE_SSL
 	uint32_t sslOptions = 0;
-	SslFingerprints sslFingerprint;
+	SslFingerprints sslFingerprints;
 	SslKeyCertPair sslKeyCertPair;
 #endif
 
diff --git a/Sming/SmingCore/Network/HttpClient.cpp b/Sming/SmingCore/Network/HttpClient.cpp
index 29770153f7..f0f29d651d 100644
--- a/Sming/SmingCore/Network/HttpClient.cpp
+++ b/Sming/SmingCore/Network/HttpClient.cpp
@@ -52,7 +52,7 @@ bool HttpClient::send(HttpRequest* request)
 			sslSessionIdPool[cacheKey] = new SslSessionId;
 		}
 		httpConnectionPool[cacheKey]->addSslOptions(request->getSslOptions());
-		httpConnectionPool[cacheKey]->pinCertificate(request->sslFingerprint);
+		httpConnectionPool[cacheKey]->pinCertificate(request->sslFingerprints);
 		httpConnectionPool[cacheKey]->setSslKeyCert(request->sslKeyCertPair);
 		httpConnectionPool[cacheKey]->sslSessionId = sslSessionIdPool[cacheKey];
 	}
diff --git a/Sming/SmingCore/Network/Ssl/SslFingerprints.h b/Sming/SmingCore/Network/Ssl/SslFingerprints.h
index 4554b3371f..e2b2391f17 100644
--- a/Sming/SmingCore/Network/Ssl/SslFingerprints.h
+++ b/Sming/SmingCore/Network/Ssl/SslFingerprints.h
@@ -14,15 +14,93 @@ enum SslFingerprintType {
 					   //    Only when the private key used to generate the certificate is used then that fingerprint
 };
 
-typedef struct {
+/** @brief Contains SSL fingerprint data
+ *  @note Lifetime as follows:
+ *  	- Constructed by application, using appropriate setXXX method;
+ *  	- Passed into HttpRequest by application, using pinCertificate method - request is then queued;
+ *  	- Passed into HttpConnection (TcpClient descendant) by HttpClient, using pinCertificate method
+ *  	- When certificate validated, memory is released
+ *
+ */
+struct SslFingerprints {
 	uint8_t* certSha1 = nullptr; // << certificate SHA1 fingerprint
 	uint8_t* pkSha256 = nullptr; // << public key SHA256 fingerprint
 
+	~SslFingerprints()
+	{
+		free();
+	}
+
 	void free()
 	{
-		delete certSha1;
+		delete[] certSha1;
 		certSha1 = nullptr;
-		delete pkSha256;
+		delete[] pkSha256;
 		pkSha256 = nullptr;
 	}
-} SslFingerprints;
+
+	void setSha1(const uint8_t* cert)
+	{
+		if(cert == nullptr) {
+			delete[] certSha1;
+			certSha1 = nullptr;
+		} else {
+			if(certSha1 == nullptr) {
+				certSha1 = new uint8_t[SHA1_SIZE];
+			}
+			memcpy(certSha1, cert, SHA1_SIZE);
+		}
+	}
+
+	void setSha256(const uint8_t* cert)
+	{
+		if(cert == nullptr) {
+			delete[] pkSha256;
+			pkSha256 = nullptr;
+		} else {
+			if(pkSha256 == nullptr) {
+				pkSha256 = new uint8_t[SHA256_SIZE];
+			}
+			memcpy(pkSha256, cert, SHA256_SIZE);
+		}
+	}
+
+	/** @brief Make copy of SHA1 certificate from data stored in flash
+	 *  @param cert
+	 */
+	void setSha1_P(const uint8_t* cert)
+	{
+		// Word-aligned and sized buffers don't need special handling
+		setSha1(cert);
+	}
+
+	/** @brief Make copy of SHA256 certificate from data stored in flash
+	 *  @param cert
+	 */
+	void setSha256_P(const uint8_t* cert)
+	{
+		// Word-aligned and sized buffers don't need special handling
+		setSha256(cert);
+	}
+
+	SslFingerprints& operator=(SslFingerprints& source)
+	{
+		delete[] certSha1;
+		certSha1 = source.certSha1;
+		source.certSha1 = nullptr;
+
+		delete[] pkSha256;
+		pkSha256 = source.pkSha256;
+		source.pkSha256 = nullptr;
+
+		return *this;
+	}
+
+	SslFingerprints& operator=(const SslFingerprints& source)
+	{
+		setSha1(source.certSha1);
+		setSha256(source.pkSha256);
+
+		return *this;
+	}
+};
diff --git a/Sming/SmingCore/Network/Ssl/SslValidator.cpp b/Sming/SmingCore/Network/Ssl/SslValidator.cpp
index f609b531fb..5907963c28 100644
--- a/Sming/SmingCore/Network/Ssl/SslValidator.cpp
+++ b/Sming/SmingCore/Network/Ssl/SslValidator.cpp
@@ -8,17 +8,16 @@
  *
  ****/
 
-#include "SslValidator.h"
-
 #ifdef ENABLE_SSL
 
+#include "SslValidator.h"
+
 bool sslValidateCertificateSha1(SSL* ssl, void* data)
 {
-	uint8_t* hash = (uint8_t*)data;
+	uint8_t* hash = static_cast<uint8_t*>(data);
 	bool success = false;
-	if(hash != NULL) {
+	if(hash != nullptr) {
 		success = (ssl_match_fingerprint(ssl, hash) == 0);
-		delete[] hash;
 	}
 
 	return success;
@@ -26,11 +25,47 @@ bool sslValidateCertificateSha1(SSL* ssl, void* data)
 
 bool sslValidatePublicKeySha256(SSL* ssl, void* data)
 {
-	uint8_t* hash = (uint8_t*)data;
+	uint8_t* hash = static_cast<uint8_t*>(data);
 	bool success = false;
-	if(hash != NULL) {
+	if(hash != nullptr) {
 		success = (ssl_match_spki_sha256(ssl, hash) == 0);
-		delete[] hash;
+	}
+
+	return success;
+}
+
+/* SSLValidatorList */
+
+void SSLValidatorList::clear()
+{
+	// Ensure any remaining fingerprint data is released
+	for(unsigned i = 0; i < count(); ++i) {
+		delete[] static_cast<uint8_t*>(elementAt(i).data);
+	}
+	Vector::clear();
+}
+
+bool SSLValidatorList::validate(SSL* ssl)
+{
+	if(count() == 0) {
+		// No validators specified, always succeed
+		debug_d("SSL Validator: list empty, allow connection");
+		return true;
+	}
+
+	// Need a match against a fingerprint
+	bool success = false;
+	for(unsigned i = 0; i < count(); i++) {
+		auto& validator = elementAt(i);
+		if(validator.callback(ssl, validator.data)) {
+			debug_d("SSL validator: positive match");
+			success = true;
+			break;
+		}
+	}
+
+	if(!success) {
+		debug_d("SSL validator: NO match");
 	}
 
 	return success;
diff --git a/Sming/SmingCore/Network/Ssl/SslValidator.h b/Sming/SmingCore/Network/Ssl/SslValidator.h
index a44e9098af..7daf9910ed 100644
--- a/Sming/SmingCore/Network/Ssl/SslValidator.h
+++ b/Sming/SmingCore/Network/Ssl/SslValidator.h
@@ -16,12 +16,11 @@
 #ifndef _SMING_CORE_SSLVALIDATOR_H_
 #define _SMING_CORE_SSLVALIDATOR_H_
 
-#ifdef ENABLE_SSL
-
 #include "ssl/ssl.h"
 #include "ssl/tls1.h"
 
 #include <functional>
+#include "WVector.h"
 
 typedef std::function<bool(SSL* ssl, void* data)> SslValidatorCallback;
 
@@ -29,7 +28,28 @@ bool sslValidateCertificateSha1(SSL* ssl, void* data);
 
 bool sslValidatePublicKeySha256(SSL* ssl, void* data);
 
-#endif /* ENABLE_SSL */
+struct SSLValidator {
+	SslValidatorCallback callback;
+	void* data; ///< Fingerprint
+};
+
+class SSLValidatorList : private Vector<SSLValidator>
+{
+public:
+	~SSLValidatorList()
+	{
+		clear();
+	}
+
+	void add(SslValidatorCallback callback, void* data)
+	{
+		Vector::add(SSLValidator{callback, data});
+	}
+
+	bool validate(SSL* ssl);
+
+	void clear();
+};
 
 /** @} */
 #endif /* _SMING_CORE_SSLVALIDATOR_H_ */
diff --git a/Sming/SmingCore/Network/TcpClient.cpp b/Sming/SmingCore/Network/TcpClient.cpp
index a22b456551..1ee5862728 100644
--- a/Sming/SmingCore/Network/TcpClient.cpp
+++ b/Sming/SmingCore/Network/TcpClient.cpp
@@ -199,21 +199,9 @@ void TcpClient::onFinished(TcpClientState finishState)
 #ifdef ENABLE_SSL
 err_t TcpClient::onSslConnected(SSL* ssl)
 {
-	bool hasSuccess = (sslValidators.count() == 0);
-	for(unsigned i = 0; i < sslValidators.count(); i++) {
-		if(sslValidators[i](ssl, sslValidatorsData[i])) {
-			hasSuccess = true;
-			break;
-		}
-	}
-
-	return hasSuccess ? ERR_OK : ERR_ABRT;
-}
-
-void TcpClient::addSslValidator(SslValidatorCallback callback, void* data)
-{
-	sslValidators.addElement(callback);
-	sslValidatorsData.addElement(data);
+	err_t result = sslValidators.validate(ssl) ? ERR_OK : ERR_ABRT;
+	sslValidators.clear();
+	return result;
 }
 
 bool TcpClient::pinCertificate(const uint8_t* fingerprint, SslFingerprintType type)
@@ -235,20 +223,22 @@ bool TcpClient::pinCertificate(const uint8_t* fingerprint, SslFingerprintType ty
 		return false;
 	}
 
-	addSslValidator(callback, (void*)fingerprint);
+	addSslValidator(callback, const_cast<uint8_t*>(fingerprint));
 
 	return true;
 }
 
-bool TcpClient::pinCertificate(const SslFingerprints& fingerprints)
+bool TcpClient::pinCertificate(SslFingerprints& fingerprints)
 {
 	bool success = false;
 	if(fingerprints.certSha1 != nullptr) {
 		success = pinCertificate(fingerprints.certSha1, eSFT_CertSha1);
+		fingerprints.certSha1 = nullptr;
 	}
 
 	if(fingerprints.pkSha256 != nullptr) {
 		success = pinCertificate(fingerprints.pkSha256, eSFT_PkSha256);
+		fingerprints.pkSha256 = nullptr;
 	}
 
 	return success;
diff --git a/Sming/SmingCore/Network/TcpClient.h b/Sming/SmingCore/Network/TcpClient.h
index 25b1a4eaf6..67802c52fb 100644
--- a/Sming/SmingCore/Network/TcpClient.h
+++ b/Sming/SmingCore/Network/TcpClient.h
@@ -125,7 +125,10 @@ class TcpClient : public TcpConnection
 	 * 					   to delete it.
 	 *
 	 */
-	void addSslValidator(SslValidatorCallback callback, void* data = nullptr);
+	void addSslValidator(SslValidatorCallback callback, void* data = nullptr)
+	{
+		sslValidators.add(callback, data);
+	}
 
 	/**
 	 * @brief   Requires(pins) the remote SSL certificate to match certain fingerprints
@@ -149,20 +152,18 @@ class TcpClient : public TcpConnection
 	 * 			The SHA1 hash of the remote certificate will be calculated and compared with the given one.
 	 * 			Disadvantages: The hash needs to be updated every time the remote server updates its certificate
 	 *
-	 * @return bool  true of success, false or failure
+	 * @retval bool true on success, false on failure
 	 */
 	bool pinCertificate(const uint8_t* fingerprint, SslFingerprintType type);
 
 	/**
 	 * @brief	Requires(pins) the remote SSL certificate to match certain fingerprints
-	 *
 	 * @note	The data inside the fingerprints parameter is passed by reference
-	 *
 	 * @param	fingerprints - passes the certificate fingerprints by reference.
 	 *
-	 * @retval	bool  true of success, false or failure
+	 * @retval bool  true of success, false or failure
 	 */
-	bool pinCertificate(const SslFingerprints& fingerprints);
+	bool pinCertificate(SslFingerprints& fingerprints);
 #endif
 
 protected:
@@ -196,8 +197,7 @@ class TcpClient : public TcpConnection
 	uint16_t asyncTotalSent = 0;
 	uint16_t asyncTotalLen = 0;
 #ifdef ENABLE_SSL
-	Vector<SslValidatorCallback> sslValidators;
-	Vector<void*> sslValidatorsData;
+	SSLValidatorList sslValidators;
 #endif
 };
 
diff --git a/samples/Basic_Ssl/app/application.cpp b/samples/Basic_Ssl/app/application.cpp
index 7f0be13f9d..cf2e113f4d 100644
--- a/samples/Basic_Ssl/app/application.cpp
+++ b/samples/Basic_Ssl/app/application.cpp
@@ -88,35 +88,33 @@ int onDownload(HttpConnection& connection, bool success)
 
 void gotIP(IPAddress ip, IPAddress netmask, IPAddress gateway)
 {
-	const uint8_t googleSha1Fingerprint[] = {0x07, 0xf0, 0xb0, 0x8d, 0x41, 0xfb, 0xee, 0x6b, 0x34, 0xfb,
-											 0x9a, 0xd0, 0x9a, 0xa7, 0x73, 0xab, 0xcc, 0x8b, 0xb2, 0x64};
+	static const uint8_t googleSha1Fingerprint[] PROGMEM = {0x07, 0xf0, 0xb0, 0x8d, 0x41, 0xfb, 0xee, 0x6b, 0x34, 0xfb,
+															0x9a, 0xd0, 0x9a, 0xa7, 0x73, 0xab, 0xcc, 0x8b, 0xb2, 0x64};
 
-	const uint8_t googlePublicKeyFingerprint[] = {0xe7, 0x06, 0x09, 0xc7, 0xef, 0xb0, 0x69, 0xe8, 0x0a, 0xeb, 0x21,
-												  0x16, 0x4c, 0xd4, 0x2d, 0x86, 0x65, 0x09, 0x62, 0x37, 0xeb, 0x75,
-												  0x92, 0xaa, 0x10, 0x03, 0xe7, 0x99, 0x01, 0x9d, 0x9f, 0x0c};
+	static const uint8_t googlePublicKeyFingerprint[] PROGMEM = {
+		0xe7, 0x06, 0x09, 0xc7, 0xef, 0xb0, 0x69, 0xe8, 0x0a, 0xeb, 0x21, 0x16, 0x4c, 0xd4, 0x2d, 0x86,
+		0x65, 0x09, 0x62, 0x37, 0xeb, 0x75, 0x92, 0xaa, 0x10, 0x03, 0xe7, 0x99, 0x01, 0x9d, 0x9f, 0x0c};
 
 	debugf("Connected. Got IP: %s", ip.toString().c_str());
 
 	HttpRequest* request = new HttpRequest(URL("https://www.google.com/"));
 	request->setSslOptions(SSL_SERVER_VERIFY_LATER);
 
-	SslFingerprints fingerprint;
+	SslFingerprints fingerprints;
 
 	/*
 	 * The line below shows how to trust only a certificate that matches the SHA1 fingerprint.
 	 * When google changes their certificate the SHA1 fingerprint should not match any longer.
 	 */
-	fingerprint.certSha1 = new uint8_t[SHA1_SIZE];
-	memcpy(fingerprint.certSha1, googleSha1Fingerprint, SHA1_SIZE);
+	fingerprints.setSha1_P(googleSha1Fingerprint);
 
 	/*
 	* The line below shows how to trust only a certificate in which the public key matches the SHA256 fingerprint.
 	* When google changes the private key that they use in their certificate the SHA256 fingerprint should not match any longer.
 	*/
-	fingerprint.pkSha256 = new uint8_t[SHA256_SIZE];
-	memcpy(fingerprint.pkSha256, googlePublicKeyFingerprint, SHA256_SIZE);
+	fingerprints.setSha256_P(googlePublicKeyFingerprint);
 
-	request->pinCertificate(fingerprint);
+	request->pinCertificate(fingerprints);
 	request->onRequestComplete(onDownload);
 
 	downloadClient.send(request);
diff --git a/samples/HttpClient/app/application.cpp b/samples/HttpClient/app/application.cpp
index e9c1be5c8b..1fde711b95 100644
--- a/samples/HttpClient/app/application.cpp
+++ b/samples/HttpClient/app/application.cpp
@@ -98,24 +98,28 @@ void setSslFingerprints(HttpRequest* request)
 	// (See: ../Makefile-user.mk )
 	request->setSslOptions(SSL_SERVER_VERIFY_LATER);
 
-	const uint8_t sha1Fingerprint[] = {0xc5, 0xf9, 0xf0, 0x66, 0xc9, 0x0a, 0x21, 0x4a, 0xbc, 0x37,
-									   0xae, 0x6c, 0x48, 0xcc, 0x97, 0xa5, 0xc3, 0x35, 0x16, 0xdc};
+	static const uint8_t sha1Fingerprint[] PROGMEM = {0xc5, 0xf9, 0xf0, 0x66, 0xc9, 0x0a, 0x21, 0x4a, 0xbc, 0x37,
+													  0xae, 0x6c, 0x48, 0xcc, 0x97, 0xa5, 0xc3, 0x35, 0x16, 0xdc};
 
-	const uint8_t publicKeyFingerprint[] = {0x33, 0x47, 0xd1, 0x8a, 0xc8, 0x52, 0xd4, 0xd6, 0xd0, 0xa2, 0xcb,
-											0x3f, 0x4b, 0x54, 0x1f, 0x91, 0x64, 0x94, 0xa0, 0x9c, 0xa1, 0xe2,
-											0xf2, 0x4c, 0x68, 0xae, 0xc5, 0x27, 0x1c, 0x60, 0x83, 0xad};
+	static const uint8_t publicKeyFingerprint[] PROGMEM = {
+		0x33, 0x47, 0xd1, 0x8a, 0xc8, 0x52, 0xd4, 0xd6, 0xd0, 0xa2, 0xcb, 0x3f, 0x4b, 0x54, 0x1f, 0x91,
+		0x64, 0x94, 0xa0, 0x9c, 0xa1, 0xe2, 0xf2, 0x4c, 0x68, 0xae, 0xc5, 0x27, 0x1c, 0x60, 0x83, 0xad};
+
+	SslFingerprints fingerprints;
 
 	/*
 	 * The line below shows how to trust only a certificate in which the public key matches the SHA256 fingerprint.
 	 * When google changes the private key that they use in their certificate the SHA256 fingerprint should not match any longer.
 	 */
-	//	request->pinCertificate(publicKeyFingerprint, eSFT_PkSha256);
+	fingerprints.setSha256_P(publicKeyFingerprint);
 
 	/*
 	 * The line below shows how to trust only a certificate that matches the SHA1 fingerprint.
 	 * When google changes their certificate the SHA1 fingerprint should not match any longer.
 	 */
-	//	request->->pinCertificate(sha1Fingerprint, eSFT_CertSha1)
+	fingerprints.setSha1_P(sha1Fingerprint);
+
+	// request->pinCertificate(fingerprints);
 }
 
 void connectOk(IPAddress ip, IPAddress mask, IPAddress gateway)

From 6d992fe03385a2309ad363d2fe0e3346d828c8f2 Mon Sep 17 00:00:00 2001
From: mikee47 <mike@sillyhouse.net>
Date: Tue, 12 Feb 2019 15:08:17 +0000
Subject: [PATCH 2/6] Revise `Basic_Ssl` sample to use different web-page with
 fingerprint that doesn't change.

Add length checks to SslFingerprints.
Also change `HardwareSerial` to base on `ReadWriteStream` instead of `Stream`, allows incoming data to be streamed direct.
---
 .../SmingCore/Data/Stream/DataSourceStream.h  |  1 +
 Sming/SmingCore/HardwareSerial.h              | 21 +++++-
 Sming/SmingCore/Network/Ssl/SslFingerprints.h | 66 +++++++++++--------
 samples/Basic_Ssl/app/application.cpp         | 46 +++++++------
 samples/HttpClient/app/application.cpp        |  4 +-
 5 files changed, 86 insertions(+), 52 deletions(-)

diff --git a/Sming/SmingCore/Data/Stream/DataSourceStream.h b/Sming/SmingCore/Data/Stream/DataSourceStream.h
index ff0ca1194b..18e4bb1c6d 100644
--- a/Sming/SmingCore/Data/Stream/DataSourceStream.h
+++ b/Sming/SmingCore/Data/Stream/DataSourceStream.h
@@ -19,6 +19,7 @@
 enum StreamType {
 	eSST_Invalid,	///< Stream content not valid
 	eSST_Memory,	 ///< Memory data stream
+	eSST_Serial,	 ///< Serial port
 	eSST_File,		 ///< File data stream
 	eSST_Template,   ///< Template data stream
 	eSST_JsonObject, ///< JSON object data stream
diff --git a/Sming/SmingCore/HardwareSerial.h b/Sming/SmingCore/HardwareSerial.h
index 14093306dc..e0b65af553 100644
--- a/Sming/SmingCore/HardwareSerial.h
+++ b/Sming/SmingCore/HardwareSerial.h
@@ -14,7 +14,7 @@
 #define _HARDWARESERIAL_H_
 
 #include "WiringFrameworkDependencies.h"
-#include "Stream.h"
+#include "Data/Stream/ReadWriteStream.h"
 #include "Delegate.h"
 #include "espinc/uart.h"
 
@@ -88,7 +88,7 @@ enum SerialMode { SERIAL_FULL = UART_FULL, SERIAL_RX_ONLY = UART_RX_ONLY, SERIAL
 #endif
 
 /// Hardware serial class
-class HardwareSerial : public Stream
+class HardwareSerial : public ReadWriteStream
 {
 public:
 	/** @brief  Create instance of a hardware serial port object
@@ -101,6 +101,11 @@ class HardwareSerial : public Stream
 
 	~HardwareSerial();
 
+	virtual StreamType getStreamType() const
+	{
+		return eSST_Serial;
+	}
+
 	void setPort(int uartPort)
 	{
 		end();
@@ -239,11 +244,21 @@ class HardwareSerial : public Stream
 	 *  @note Although this shares the same name as the method in IDataSourceStream,
 	 *  behaviour is different because in effect the 'seek' position is changed by this call.
 	 */
-	virtual uint16_t readMemoryBlock(char* buf, size_t max_len)
+	virtual uint16_t readMemoryBlock(char* buf, int max_len)
 	{
 		return uart_read(uart, buf, max_len);
 	}
 
+	virtual bool seek(int len)
+	{
+		return false;
+	}
+
+	virtual bool isFinished()
+	{
+		return false;
+	}
+
 	/** @brief  Read a character from serial port without removing from input buffer
      *  @retval int Character read from serial port or -1 if buffer empty
      *  @note   The character remains in serial port input buffer
diff --git a/Sming/SmingCore/Network/Ssl/SslFingerprints.h b/Sming/SmingCore/Network/Ssl/SslFingerprints.h
index e2b2391f17..32d78f940f 100644
--- a/Sming/SmingCore/Network/Ssl/SslFingerprints.h
+++ b/Sming/SmingCore/Network/Ssl/SslFingerprints.h
@@ -39,50 +39,35 @@ struct SslFingerprints {
 		pkSha256 = nullptr;
 	}
 
-	void setSha1(const uint8_t* cert)
+	bool setSha1(const uint8_t* cert, unsigned length)
 	{
-		if(cert == nullptr) {
-			delete[] certSha1;
-			certSha1 = nullptr;
-		} else {
-			if(certSha1 == nullptr) {
-				certSha1 = new uint8_t[SHA1_SIZE];
-			}
-			memcpy(certSha1, cert, SHA1_SIZE);
-		}
+		return setValue(certSha1, SHA1_SIZE, cert, length);
 	}
 
-	void setSha256(const uint8_t* cert)
+	bool setSha256(const uint8_t* cert, unsigned length)
 	{
-		if(cert == nullptr) {
-			delete[] pkSha256;
-			pkSha256 = nullptr;
-		} else {
-			if(pkSha256 == nullptr) {
-				pkSha256 = new uint8_t[SHA256_SIZE];
-			}
-			memcpy(pkSha256, cert, SHA256_SIZE);
-		}
+		return setValue(pkSha256, SHA256_SIZE, cert, length);
 	}
 
 	/** @brief Make copy of SHA1 certificate from data stored in flash
 	 *  @param cert
 	 */
-	void setSha1_P(const uint8_t* cert)
+	bool setSha1_P(const uint8_t* cert, unsigned length)
 	{
 		// Word-aligned and sized buffers don't need special handling
-		setSha1(cert);
+		return setSha1(cert, length);
 	}
 
 	/** @brief Make copy of SHA256 certificate from data stored in flash
 	 *  @param cert
 	 */
-	void setSha256_P(const uint8_t* cert)
+	bool setSha256_P(const uint8_t* cert, unsigned length)
 	{
 		// Word-aligned and sized buffers don't need special handling
-		setSha256(cert);
+		return setSha256(cert, length);
 	}
 
+	/** @brief Moves values out of source */
 	SslFingerprints& operator=(SslFingerprints& source)
 	{
 		delete[] certSha1;
@@ -96,11 +81,40 @@ struct SslFingerprints {
 		return *this;
 	}
 
+	/** @brief Make copy of values from source */
 	SslFingerprints& operator=(const SslFingerprints& source)
 	{
-		setSha1(source.certSha1);
-		setSha256(source.pkSha256);
+		setSha1(source.certSha1, SHA1_SIZE);
+		setSha256(source.pkSha256, SHA256_SIZE);
 
 		return *this;
 	}
+
+private:
+	/** @brief Internal method to set a fingerprint
+	 *  @param value Reference to fingerprint value in this structure
+	 *  @param length Required length for value
+	 *  @param newValue
+	 *  @param newLength
+	 *  @retval bool true on success, false on invalid length or memory allocation failure
+	 */
+	bool setValue(uint8_t*& value, unsigned length, const uint8_t* newValue, unsigned newLength)
+	{
+		if(newValue == nullptr || newLength == 0) {
+			delete[] value;
+			value = nullptr;
+			return true;
+		} else if(newLength != length) {
+			return false;
+		} else {
+			if(value == nullptr) {
+				value = new uint8_t[length];
+				if(value == nullptr) {
+					return false;
+				}
+			}
+			memcpy(value, newValue, length);
+			return true;
+		}
+	}
 };
diff --git a/samples/Basic_Ssl/app/application.cpp b/samples/Basic_Ssl/app/application.cpp
index cf2e113f4d..c7f9841dfe 100644
--- a/samples/Basic_Ssl/app/application.cpp
+++ b/samples/Basic_Ssl/app/application.cpp
@@ -9,6 +9,7 @@
 
 #include <user_config.h>
 #include <SmingCore/SmingCore.h>
+#include "Data/HexString.h"
 
 // If you want, you can define WiFi settings globally in Eclipse Environment Variables
 #ifndef WIFI_SSID
@@ -22,17 +23,14 @@ HttpClient downloadClient;
 /* Debug SSL functions */
 void displaySessionId(SSL* ssl)
 {
-	int i;
 	const uint8_t* session_id = ssl_get_session_id(ssl);
-	int sess_id_size = ssl_get_session_id_size(ssl);
+	unsigned sess_id_size = ssl_get_session_id_size(ssl);
 
-	if(sess_id_size > 0) {
+	if(sess_id_size != 0) {
 		debugf("-----BEGIN SSL SESSION PARAMETERS-----");
-		for(i = 0; i < sess_id_size; i++) {
-			m_printf("%02x", session_id[i]);
-		}
-
-		debugf("\n-----END SSL SESSION PARAMETERS-----");
+		m_puts(makeHexString(session_id, sess_id_size).c_str());
+		m_putc('\n');
+		debugf("-----END SSL SESSION PARAMETERS-----");
 	}
 }
 
@@ -60,7 +58,7 @@ void displayCipher(SSL* ssl)
 		break;
 
 	default:
-		m_printf("Unknown - %d", ssl_get_cipher_id(ssl));
+		m_printf("Unknown - %u", ssl_get_cipher_id(ssl));
 		break;
 	}
 
@@ -70,7 +68,6 @@ void displayCipher(SSL* ssl)
 int onDownload(HttpConnection& connection, bool success)
 {
 	debugf("Got response code: %d", connection.getResponseCode());
-	debugf("Got content starting with: %s", connection.getResponseString().substring(0, 50).c_str());
 	debugf("Success: %d", success);
 
 	SSL* ssl = connection.getSsl();
@@ -88,35 +85,42 @@ int onDownload(HttpConnection& connection, bool success)
 
 void gotIP(IPAddress ip, IPAddress netmask, IPAddress gateway)
 {
-	static const uint8_t googleSha1Fingerprint[] PROGMEM = {0x07, 0xf0, 0xb0, 0x8d, 0x41, 0xfb, 0xee, 0x6b, 0x34, 0xfb,
-															0x9a, 0xd0, 0x9a, 0xa7, 0x73, 0xab, 0xcc, 0x8b, 0xb2, 0x64};
+	// Use the Gibson Research fingerprints web page as an example. Unlike Google, the fingerprints don't change!
+	static const uint8_t grcSha1Fingerprint[] PROGMEM = {0x15, 0x9A, 0x76, 0xC5, 0xAE, 0xF4, 0x90, 0x15, 0x79, 0xE6,
+														 0xA4, 0x99, 0x96, 0xC1, 0xD6, 0xA1, 0xD9, 0x3B, 0x07, 0x43};
 
-	static const uint8_t googlePublicKeyFingerprint[] PROGMEM = {
-		0xe7, 0x06, 0x09, 0xc7, 0xef, 0xb0, 0x69, 0xe8, 0x0a, 0xeb, 0x21, 0x16, 0x4c, 0xd4, 0x2d, 0x86,
-		0x65, 0x09, 0x62, 0x37, 0xeb, 0x75, 0x92, 0xaa, 0x10, 0x03, 0xe7, 0x99, 0x01, 0x9d, 0x9f, 0x0c};
+	static const uint8_t grcPublicKeyFingerprint[] PROGMEM = {
+		0xEB, 0xA0, 0xFE, 0x70, 0xFE, 0xCB, 0xF8, 0xA8, 0x7A, 0xB9, 0x1D, 0xAC, 0x1E, 0xAC, 0xA0, 0xF6,
+		0x62, 0xCB, 0xCD, 0xE4, 0x16, 0x72, 0xE6, 0xBC, 0x82, 0x9B, 0x32, 0x39, 0x43, 0x15, 0x76, 0xD4};
 
 	debugf("Connected. Got IP: %s", ip.toString().c_str());
 
-	HttpRequest* request = new HttpRequest(URL("https://www.google.com/"));
+	HttpRequest* request = new HttpRequest(URL(F("https://www.grc.com/fingerprints.htm")));
 	request->setSslOptions(SSL_SERVER_VERIFY_LATER);
 
+	/*
+	 * GET probably won't work as sites tend to use 16K blocks which we can't handle,
+	 * so just fetch the header and leave it at that. To return actual data requires a web server
+	 * configured to use smaller encrytion blocks, e.g. 4K.
+	 */
+	request->setMethod(HTTP_HEAD);
+
 	SslFingerprints fingerprints;
 
 	/*
 	 * The line below shows how to trust only a certificate that matches the SHA1 fingerprint.
-	 * When google changes their certificate the SHA1 fingerprint should not match any longer.
 	 */
-	fingerprints.setSha1_P(googleSha1Fingerprint);
+	fingerprints.setSha1_P(grcSha1Fingerprint, sizeof(grcSha1Fingerprint));
 
 	/*
 	* The line below shows how to trust only a certificate in which the public key matches the SHA256 fingerprint.
-	* When google changes the private key that they use in their certificate the SHA256 fingerprint should not match any longer.
 	*/
-	fingerprints.setSha256_P(googlePublicKeyFingerprint);
+	fingerprints.setSha256_P(grcPublicKeyFingerprint, sizeof(grcPublicKeyFingerprint));
 
 	request->pinCertificate(fingerprints);
 	request->onRequestComplete(onDownload);
 
+	request->setResponseStream(&Serial);
 	downloadClient.send(request);
 }
 
@@ -129,7 +133,7 @@ void init()
 {
 	Serial.begin(SERIAL_BAUD_RATE);
 	Serial.systemDebugOutput(true); // Allow debug print to serial
-	Serial.println("Ready for SSL tests");
+	Serial.println(F("Ready for SSL tests"));
 
 	// Setup the WIFI connection
 	WifiStation.enable(true);
diff --git a/samples/HttpClient/app/application.cpp b/samples/HttpClient/app/application.cpp
index 1fde711b95..d4a43a148a 100644
--- a/samples/HttpClient/app/application.cpp
+++ b/samples/HttpClient/app/application.cpp
@@ -111,13 +111,13 @@ void setSslFingerprints(HttpRequest* request)
 	 * The line below shows how to trust only a certificate in which the public key matches the SHA256 fingerprint.
 	 * When google changes the private key that they use in their certificate the SHA256 fingerprint should not match any longer.
 	 */
-	fingerprints.setSha256_P(publicKeyFingerprint);
+	fingerprints.setSha256_P(publicKeyFingerprint, sizeof(publicKeyFingerprint));
 
 	/*
 	 * The line below shows how to trust only a certificate that matches the SHA1 fingerprint.
 	 * When google changes their certificate the SHA1 fingerprint should not match any longer.
 	 */
-	fingerprints.setSha1_P(sha1Fingerprint);
+	fingerprints.setSha1_P(sha1Fingerprint, sizeof(sha1Fingerprint));
 
 	// request->pinCertificate(fingerprints);
 }

From 9989ef9af1c9f3e548010a720617b6b4c8bed1a2 Mon Sep 17 00:00:00 2001
From: mikee47 <mike@sillyhouse.net>
Date: Wed, 13 Feb 2019 10:25:03 +0000
Subject: [PATCH 3/6] Review changes

* Additional checks in `SslFingerprints` assignment operators
* Output debug error if `SslFingerprint::setXXX` fails
* Update fingerprints in `HttpClient` sample and enable checking
---
 .../SmingCore/Data/Stream/DataSourceStream.h  |  6 ++--
 Sming/SmingCore/HardwareSerial.h              |  5 ---
 Sming/SmingCore/Network/Ssl/SslFingerprints.h | 21 +++++++-----
 Sming/SmingCore/Network/TcpClient.h           |  2 +-
 samples/HttpClient/app/application.cpp        | 33 +++++++++----------
 5 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/Sming/SmingCore/Data/Stream/DataSourceStream.h b/Sming/SmingCore/Data/Stream/DataSourceStream.h
index 18e4bb1c6d..2aaf3d9cdb 100644
--- a/Sming/SmingCore/Data/Stream/DataSourceStream.h
+++ b/Sming/SmingCore/Data/Stream/DataSourceStream.h
@@ -19,7 +19,6 @@
 enum StreamType {
 	eSST_Invalid,	///< Stream content not valid
 	eSST_Memory,	 ///< Memory data stream
-	eSST_Serial,	 ///< Serial port
 	eSST_File,		 ///< File data stream
 	eSST_Template,   ///< Template data stream
 	eSST_JsonObject, ///< JSON object data stream
@@ -45,7 +44,10 @@ class IDataSourceStream : public Stream
      *  @retval StreamType The stream type.
      *  @todo   Return value of IDataSourceStream:getStreamType base class function should be of type StreamType, e.g. eSST_User
      */
-	virtual StreamType getStreamType() const = 0;
+	virtual StreamType getStreamType() const
+	{
+		return eSST_Unknown;
+	}
 
 	/** @brief Determine if the stream object contains valid data
 	 *  @retval bool true if valid, false if invalid
diff --git a/Sming/SmingCore/HardwareSerial.h b/Sming/SmingCore/HardwareSerial.h
index e0b65af553..0265fad34c 100644
--- a/Sming/SmingCore/HardwareSerial.h
+++ b/Sming/SmingCore/HardwareSerial.h
@@ -101,11 +101,6 @@ class HardwareSerial : public ReadWriteStream
 
 	~HardwareSerial();
 
-	virtual StreamType getStreamType() const
-	{
-		return eSST_Serial;
-	}
-
 	void setPort(int uartPort)
 	{
 		end();
diff --git a/Sming/SmingCore/Network/Ssl/SslFingerprints.h b/Sming/SmingCore/Network/Ssl/SslFingerprints.h
index 32d78f940f..a6ca862399 100644
--- a/Sming/SmingCore/Network/Ssl/SslFingerprints.h
+++ b/Sming/SmingCore/Network/Ssl/SslFingerprints.h
@@ -70,13 +70,15 @@ struct SslFingerprints {
 	/** @brief Moves values out of source */
 	SslFingerprints& operator=(SslFingerprints& source)
 	{
-		delete[] certSha1;
-		certSha1 = source.certSha1;
-		source.certSha1 = nullptr;
+		if(this != &source) {
+			delete[] certSha1;
+			certSha1 = source.certSha1;
+			source.certSha1 = nullptr;
 
-		delete[] pkSha256;
-		pkSha256 = source.pkSha256;
-		source.pkSha256 = nullptr;
+			delete[] pkSha256;
+			pkSha256 = source.pkSha256;
+			source.pkSha256 = nullptr;
+		}
 
 		return *this;
 	}
@@ -84,8 +86,10 @@ struct SslFingerprints {
 	/** @brief Make copy of values from source */
 	SslFingerprints& operator=(const SslFingerprints& source)
 	{
-		setSha1(source.certSha1, SHA1_SIZE);
-		setSha256(source.pkSha256, SHA256_SIZE);
+		if(this != &source) {
+			setSha1(source.certSha1, SHA1_SIZE);
+			setSha256(source.pkSha256, SHA256_SIZE);
+		}
 
 		return *this;
 	}
@@ -105,6 +109,7 @@ struct SslFingerprints {
 			value = nullptr;
 			return true;
 		} else if(newLength != length) {
+			debug_e("ERROR! Invalid fingerprint length");
 			return false;
 		} else {
 			if(value == nullptr) {
diff --git a/Sming/SmingCore/Network/TcpClient.h b/Sming/SmingCore/Network/TcpClient.h
index 67802c52fb..5c481e7555 100644
--- a/Sming/SmingCore/Network/TcpClient.h
+++ b/Sming/SmingCore/Network/TcpClient.h
@@ -161,7 +161,7 @@ class TcpClient : public TcpConnection
 	 * @note	The data inside the fingerprints parameter is passed by reference
 	 * @param	fingerprints - passes the certificate fingerprints by reference.
 	 *
-	 * @retval bool  true of success, false or failure
+	 * @retval bool  true on success, false on failure
 	 */
 	bool pinCertificate(SslFingerprints& fingerprints);
 #endif
diff --git a/samples/HttpClient/app/application.cpp b/samples/HttpClient/app/application.cpp
index d4a43a148a..19588f9bc6 100644
--- a/samples/HttpClient/app/application.cpp
+++ b/samples/HttpClient/app/application.cpp
@@ -92,34 +92,33 @@ int onDownload(HttpConnection& connection, bool success)
 
 void setSslFingerprints(HttpRequest* request)
 {
-	// SSL validation: If you want to check the remote server certificate against a fingerprint,
-	// you can also add the lines below
-	// Note: SSL is not compiled by default. In our example we set the ENABLE_SSL directive to 1
-	// (See: ../Makefile-user.mk )
+	/*
+	 * SSL validation: We check the remote server certificate against a fingerprint
+	 * Note that fingerprints _may_ change, in which case these need to be updated.
+	 *
+	 * Note: SSL is not compiled by default. In our example we set the ENABLE_SSL directive to 1
+	 * (See: ../Makefile-user.mk )
+	 */
 	request->setSslOptions(SSL_SERVER_VERIFY_LATER);
 
-	static const uint8_t sha1Fingerprint[] PROGMEM = {0xc5, 0xf9, 0xf0, 0x66, 0xc9, 0x0a, 0x21, 0x4a, 0xbc, 0x37,
-													  0xae, 0x6c, 0x48, 0xcc, 0x97, 0xa5, 0xc3, 0x35, 0x16, 0xdc};
+	// These are the fingerprints for httpbin.org
+	static const uint8_t sha1Fingerprint[] PROGMEM = {0x2B, 0xF0, 0x48, 0x9D, 0x78, 0xB4, 0xDE, 0xE9, 0x69, 0xE2,
+													  0x73, 0xE0, 0x14, 0xD0, 0xDC, 0xCC, 0xA8, 0xD8, 0x3B, 0x40};
 
 	static const uint8_t publicKeyFingerprint[] PROGMEM = {
-		0x33, 0x47, 0xd1, 0x8a, 0xc8, 0x52, 0xd4, 0xd6, 0xd0, 0xa2, 0xcb, 0x3f, 0x4b, 0x54, 0x1f, 0x91,
-		0x64, 0x94, 0xa0, 0x9c, 0xa1, 0xe2, 0xf2, 0x4c, 0x68, 0xae, 0xc5, 0x27, 0x1c, 0x60, 0x83, 0xad};
+		0xE3, 0x88, 0xC4, 0x0A, 0x2A, 0x99, 0x8F, 0xA4, 0x8C, 0x38, 0x4E, 0xE7, 0xCB, 0x4F, 0x8B, 0x99,
+		0x19, 0x48, 0x63, 0x9A, 0x2E, 0xD6, 0x05, 0x7D, 0xB1, 0xD3, 0x56, 0x6C, 0xC0, 0x7E, 0x74, 0x1A};
 
 	SslFingerprints fingerprints;
 
-	/*
-	 * The line below shows how to trust only a certificate in which the public key matches the SHA256 fingerprint.
-	 * When google changes the private key that they use in their certificate the SHA256 fingerprint should not match any longer.
-	 */
+	// Trust only a certificate in which the public key matches the SHA256 fingerprint...
 	fingerprints.setSha256_P(publicKeyFingerprint, sizeof(publicKeyFingerprint));
 
-	/*
-	 * The line below shows how to trust only a certificate that matches the SHA1 fingerprint.
-	 * When google changes their certificate the SHA1 fingerprint should not match any longer.
-	 */
+	// ... or a certificate that matches the SHA1 fingerprint.
 	fingerprints.setSha1_P(sha1Fingerprint, sizeof(sha1Fingerprint));
 
-	// request->pinCertificate(fingerprints);
+	// Attached fingerprints to request for validation
+	request->pinCertificate(fingerprints);
 }
 
 void connectOk(IPAddress ip, IPAddress mask, IPAddress gateway)

From 16732c414a268cf60b1a45eb63ef2dd639b9fccc Mon Sep 17 00:00:00 2001
From: mikee47 <mike@sillyhouse.net>
Date: Wed, 13 Feb 2019 12:25:58 +0000
Subject: [PATCH 4/6] Move `pinCertificate` code into SslValidator

Validator callback still responsible for de-allocating memory, but can be called with ssl = nullptr
---
 Sming/SmingCore/Network/Ssl/SslFingerprints.h | 25 +++++-
 Sming/SmingCore/Network/Ssl/SslValidator.cpp  | 78 ++++++++++++++-----
 Sming/SmingCore/Network/Ssl/SslValidator.h    | 44 ++++++++---
 Sming/SmingCore/Network/TcpClient.cpp         | 49 ------------
 Sming/SmingCore/Network/TcpClient.h           | 57 +++++++-------
 5 files changed, 141 insertions(+), 112 deletions(-)

diff --git a/Sming/SmingCore/Network/Ssl/SslFingerprints.h b/Sming/SmingCore/Network/Ssl/SslFingerprints.h
index a6ca862399..5ab09e8c17 100644
--- a/Sming/SmingCore/Network/Ssl/SslFingerprints.h
+++ b/Sming/SmingCore/Network/Ssl/SslFingerprints.h
@@ -5,13 +5,28 @@
  * All files of the Sming Core are provided under the LGPL v3 license.
  ****/
 
+#ifndef SMINGCORE_NETWORK_SSLFINGERPRINTS_H_
+#define SMINGCORE_NETWORK_SSLFINGERPRINTS_H_
+
 #include "ssl/ssl.h"
 
+/**
+ * @brief SSL Certificate fingerprint type
+ */
 enum SslFingerprintType {
-	eSFT_CertSha1 = 0, // << Fingerprint based on the SHA1 value of the certificate.
-					   //     Every time a certificate is renewed this value will change.
-	eSFT_PkSha256,	 // << Fingerprint based on the SHA256 value of the public key subject in the certificate.
-					   //    Only when the private key used to generate the certificate is used then that fingerprint
+	/* The SHA1 hash of the entire certificate. This changes on each certificate renewal so needs
+	 * to be updated every time the remote server updates its certificate.
+	 * Advantages: Takes less time to verify than SHA256
+	 * Disadvantages: Likely to change periodically
+	 */
+	eSFT_CertSha1 = 0, ///< Fingerprint based on the SHA1 value of the certificate
+
+	/* For HTTP public key pinning (RFC7469), the SHA-256 hash of the Subject Public Key Info
+	 * (which usually only changes when the public key changes) is used.
+	 * Advantages: Doesn't change frequently
+	 * Disadvantages: Takes more time (in ms) to verify.
+	 */
+	eSFT_PkSha256, // << Fingerprint based on the SHA256 value of the Public Key Subject in the certificate
 };
 
 /** @brief Contains SSL fingerprint data
@@ -123,3 +138,5 @@ struct SslFingerprints {
 		}
 	}
 };
+
+#endif // SMINGCORE_NETWORK_SSLFINGERPRINTS_H_
diff --git a/Sming/SmingCore/Network/Ssl/SslValidator.cpp b/Sming/SmingCore/Network/Ssl/SslValidator.cpp
index 5907963c28..9201da9c47 100644
--- a/Sming/SmingCore/Network/Ssl/SslValidator.cpp
+++ b/Sming/SmingCore/Network/Ssl/SslValidator.cpp
@@ -12,23 +12,29 @@
 
 #include "SslValidator.h"
 
-bool sslValidateCertificateSha1(SSL* ssl, void* data)
+static bool sslValidateCertificateSha1(SSL* ssl, void* data)
 {
 	uint8_t* hash = static_cast<uint8_t*>(data);
 	bool success = false;
 	if(hash != nullptr) {
-		success = (ssl_match_fingerprint(ssl, hash) == 0);
+		if(ssl != nullptr) {
+			success = (ssl_match_fingerprint(ssl, hash) == 0);
+		}
+		delete[] hash;
 	}
 
 	return success;
 }
 
-bool sslValidatePublicKeySha256(SSL* ssl, void* data)
+static bool sslValidatePublicKeySha256(SSL* ssl, void* data)
 {
 	uint8_t* hash = static_cast<uint8_t*>(data);
 	bool success = false;
 	if(hash != nullptr) {
-		success = (ssl_match_spki_sha256(ssl, hash) == 0);
+		if(ssl != nullptr) {
+			success = (ssl_match_spki_sha256(ssl, hash) == 0);
+		}
+		delete[] hash;
 	}
 
 	return success;
@@ -36,39 +42,73 @@ bool sslValidatePublicKeySha256(SSL* ssl, void* data)
 
 /* SSLValidatorList */
 
-void SSLValidatorList::clear()
-{
-	// Ensure any remaining fingerprint data is released
-	for(unsigned i = 0; i < count(); ++i) {
-		delete[] static_cast<uint8_t*>(elementAt(i).data);
-	}
-	Vector::clear();
-}
-
 bool SSLValidatorList::validate(SSL* ssl)
 {
-	if(count() == 0) {
+	if(ssl != nullptr && count() == 0) {
 		// No validators specified, always succeed
 		debug_d("SSL Validator: list empty, allow connection");
 		return true;
 	}
 
-	// Need a match against a fingerprint
+	/*
+	 * We only need one match for a successful result, but we call all the validators anyway to
+	 * ensure their data is released.
+	 */
 	bool success = false;
 	for(unsigned i = 0; i < count(); i++) {
-		auto& validator = elementAt(i);
-		if(validator.callback(ssl, validator.data)) {
+		auto& validator = operator[](i);
+		// If we've already succeeded, then just release validator data without checking
+		if(validator.callback(success ? nullptr : ssl, validator.data)) {
 			debug_d("SSL validator: positive match");
 			success = true;
-			break;
 		}
+		// Callback will have released data so pointer no longer valid
+		validator.data = nullptr;
 	}
 
-	if(!success) {
+	if(ssl != nullptr && !success) {
 		debug_d("SSL validator: NO match");
 	}
 
 	return success;
 }
 
+bool SSLValidatorList::add(const uint8_t* fingerprint, SslFingerprintType type)
+{
+	SslValidatorCallback callback = nullptr;
+	switch(type) {
+	case eSFT_CertSha1:
+		callback = sslValidateCertificateSha1;
+		break;
+	case eSFT_PkSha256:
+		callback = sslValidatePublicKeySha256;
+		break;
+	default:
+		debug_d("Unsupported SSL certificate fingerprint type");
+	}
+
+	if(!callback) {
+		delete[] fingerprint;
+		return false;
+	}
+
+	return add(callback, const_cast<uint8_t*>(fingerprint));
+}
+
+bool SSLValidatorList::add(SslFingerprints& fingerprints)
+{
+	bool success = false;
+	if(fingerprints.certSha1 != nullptr) {
+		success = add(fingerprints.certSha1, eSFT_CertSha1);
+		fingerprints.certSha1 = nullptr;
+	}
+
+	if(fingerprints.pkSha256 != nullptr) {
+		success = add(fingerprints.pkSha256, eSFT_PkSha256);
+		fingerprints.pkSha256 = nullptr;
+	}
+
+	return success;
+}
+
 #endif /* ENABLE_SSL */
diff --git a/Sming/SmingCore/Network/Ssl/SslValidator.h b/Sming/SmingCore/Network/Ssl/SslValidator.h
index 7daf9910ed..d2c00a470b 100644
--- a/Sming/SmingCore/Network/Ssl/SslValidator.h
+++ b/Sming/SmingCore/Network/Ssl/SslValidator.h
@@ -22,15 +22,20 @@
 #include <functional>
 #include "WVector.h"
 
-typedef std::function<bool(SSL* ssl, void* data)> SslValidatorCallback;
-
-bool sslValidateCertificateSha1(SSL* ssl, void* data);
+#include "SslFingerprints.h"
 
-bool sslValidatePublicKeySha256(SSL* ssl, void* data);
+/** @brief Validator callback function
+ *  @param ssl Contains certificate to validate (may be NULL)
+ *  @param data Data for the callback to use
+ *  @retval bool true if validation succeeded
+ *  @note Callback must ALWAYS release any allocate memory before returning.
+ *  If called with ssl = NULL then just release memory and return false.
+ */
+typedef std::function<bool(SSL* ssl, void* data)> SslValidatorCallback;
 
 struct SSLValidator {
 	SslValidatorCallback callback;
-	void* data; ///< Fingerprint
+	void* data; ///< Callback-specific data, e.g. fingerprint to compare against
 };
 
 class SSLValidatorList : private Vector<SSLValidator>
@@ -38,17 +43,36 @@ class SSLValidatorList : private Vector<SSLValidator>
 public:
 	~SSLValidatorList()
 	{
-		clear();
+		// Make sure memory gets released
+		validate(nullptr);
 	}
 
-	void add(SslValidatorCallback callback, void* data)
+	bool add(SslValidatorCallback callback, void* data)
 	{
-		Vector::add(SSLValidator{callback, data});
+		return Vector::add(SSLValidator{callback, data});
 	}
 
-	bool validate(SSL* ssl);
+	/**
+	 * @brief	Add a standard fingerprint validator
+	 * @param	fingerprint	The fingerprint data against which the match should be performed.
+	 * 						Must be allocated on the heap and will be deleted after use.
+	 * @param	type	The fingerprint type - see SslFingerprintType for details.
+	 * @retval	bool	true on success, false on failure
+	 */
+	bool add(const uint8_t* fingerprint, SslFingerprintType type);
+
+	/**
+	 * @brief	Add validators for standard fingerprints
+	 * @param	fingerprints Will be invalid after returning as data is moved rather than copied
+	 * @retval	bool true on success, false on failure
+	 */
+	bool add(SslFingerprints& fingerprints);
 
-	void clear();
+	/** @brief Used to validate certificate by invoking each validator callback until successful
+	 *  @param ssl When called with nullptr will simply de-allocate any validator memory
+	 * @retval bool  true on success, false on failure
+	 */
+	bool validate(SSL* ssl);
 };
 
 /** @} */
diff --git a/Sming/SmingCore/Network/TcpClient.cpp b/Sming/SmingCore/Network/TcpClient.cpp
index 1ee5862728..03e8f9ba0c 100644
--- a/Sming/SmingCore/Network/TcpClient.cpp
+++ b/Sming/SmingCore/Network/TcpClient.cpp
@@ -195,52 +195,3 @@ void TcpClient::onFinished(TcpClientState finishState)
 		completed(*this, state == eTCS_Successful);
 	}
 }
-
-#ifdef ENABLE_SSL
-err_t TcpClient::onSslConnected(SSL* ssl)
-{
-	err_t result = sslValidators.validate(ssl) ? ERR_OK : ERR_ABRT;
-	sslValidators.clear();
-	return result;
-}
-
-bool TcpClient::pinCertificate(const uint8_t* fingerprint, SslFingerprintType type)
-{
-	SslValidatorCallback callback = nullptr;
-	switch(type) {
-	case eSFT_CertSha1:
-		callback = sslValidateCertificateSha1;
-		break;
-	case eSFT_PkSha256:
-		callback = sslValidatePublicKeySha256;
-		break;
-	default:
-		debug_d("Unsupported SSL certificate fingerprint type");
-	}
-
-	if(!callback) {
-		delete[] fingerprint;
-		return false;
-	}
-
-	addSslValidator(callback, const_cast<uint8_t*>(fingerprint));
-
-	return true;
-}
-
-bool TcpClient::pinCertificate(SslFingerprints& fingerprints)
-{
-	bool success = false;
-	if(fingerprints.certSha1 != nullptr) {
-		success = pinCertificate(fingerprints.certSha1, eSFT_CertSha1);
-		fingerprints.certSha1 = nullptr;
-	}
-
-	if(fingerprints.pkSha256 != nullptr) {
-		success = pinCertificate(fingerprints.pkSha256, eSFT_PkSha256);
-		fingerprints.pkSha256 = nullptr;
-	}
-
-	return success;
-}
-#endif
diff --git a/Sming/SmingCore/Network/TcpClient.h b/Sming/SmingCore/Network/TcpClient.h
index 5c481e7555..5ca7d84820 100644
--- a/Sming/SmingCore/Network/TcpClient.h
+++ b/Sming/SmingCore/Network/TcpClient.h
@@ -119,51 +119,44 @@ class TcpClient : public TcpConnection
 #ifdef ENABLE_SSL
 	/**
 	 * @brief Allows setting of multiple SSL validators after a successful handshake
-	 * @param SslValidatorCallback callback
-	 * @param void* data - The data that should be passed to the callback.
-	 * 					   The callback will cast the data to the correct type and take care
-	 * 					   to delete it.
+	 * @param callback The callback function to be invoked on validation
+	 * @param data The data to pass to the callback
+	 * @note The callback is responsible for releasing the data if appropriate.
+	 * See SslValidatorCallback for further details.
 	 *
+	 * @retval bool true on success, false on failure
 	 */
-	void addSslValidator(SslValidatorCallback callback, void* data = nullptr)
+	bool addSslValidator(SslValidatorCallback callback, void* data = nullptr)
 	{
-		sslValidators.add(callback, data);
+		return sslValidators.add(callback, data);
 	}
 
 	/**
-	 * @brief   Requires(pins) the remote SSL certificate to match certain fingerprints
-	 * 			Check if SHA256 hash of Subject Public Key Info matches the one given.
-	 * @note    For HTTP public key pinning (RFC7469), the SHA-256 hash of the
-	 * 		    Subject Public Key Info (which usually only changes when the public key changes)
-	 * 		    is used rather than the SHA-1 hash of the entire certificate
-	 * 		    (which will change on each certificate renewal).
-	 * @param const uint8_t *finterprint - the fingeprint data against which the match should be performed
-	 * 									   The fingerprint will be deleted after use and should
-	 * 									   not be reused outside of this method
-	 * @param SslFingerprintType type - the fingerprint type
-	 * @note    Type: eSFT_PkSha256
-	 * 			For HTTP public key pinning (RFC7469), the SHA-256 hash of the
-	 * 		    Subject Public Key Info (which usually only changes when the public key changes)
-	 * 		    is used rather than the SHA-1 hash of the entire certificate
-	 * 		    (which will change on each certificate renewal).
-	 * 		    Advantages: The
-	 * 		    Disadvantages: Takes more time (in ms) to verify.
-	 * @note    Type: eSFT_CertSha1
-	 * 			The SHA1 hash of the remote certificate will be calculated and compared with the given one.
-	 * 			Disadvantages: The hash needs to be updated every time the remote server updates its certificate
+	 * @brief Requires (pins) the remote SSL certificate to match certain fingerprints
+	 * @param fingerprint	The fingerprint data against which the match should be performed.
+	 * 						Must be allocated on the heap and will be deleted after use.
+	 * 						Do not re-use outside of this method.
+	 * @param type			The fingerprint type - see SslFingerprintType for details.
 	 *
 	 * @retval bool true on success, false on failure
 	 */
-	bool pinCertificate(const uint8_t* fingerprint, SslFingerprintType type);
+	bool pinCertificate(const uint8_t* fingerprint, SslFingerprintType type)
+	{
+		return sslValidators.add(fingerprint, type);
+	}
 
 	/**
-	 * @brief	Requires(pins) the remote SSL certificate to match certain fingerprints
+	 * @brief	Requires (pins) the remote SSL certificate to match certain fingerprints
 	 * @note	The data inside the fingerprints parameter is passed by reference
 	 * @param	fingerprints - passes the certificate fingerprints by reference.
 	 *
 	 * @retval bool  true on success, false on failure
 	 */
-	bool pinCertificate(SslFingerprints& fingerprints);
+	bool pinCertificate(SslFingerprints& fingerprints)
+	{
+		return sslValidators.add(fingerprints);
+	}
+
 #endif
 
 protected:
@@ -175,7 +168,11 @@ class TcpClient : public TcpConnection
 	virtual void onFinished(TcpClientState finishState);
 
 #ifdef ENABLE_SSL
-	virtual err_t onSslConnected(SSL* ssl);
+	virtual err_t onSslConnected(SSL* ssl)
+	{
+		return sslValidators.validate(ssl) ? ERR_OK : ERR_ABRT;
+	}
+
 #endif
 
 	void pushAsyncPart();

From fd0f801811083c98dbbedb186e48084812bdcf0e Mon Sep 17 00:00:00 2001
From: mikee47 <mike@sillyhouse.net>
Date: Wed, 13 Feb 2019 14:48:46 +0000
Subject: [PATCH 5/6] Rename `SSLValidatorList` -> `SslValidatorList`,
 `SSLValidator` -> `SslValidator`

---
 Sming/SmingCore/Network/Ssl/SslValidator.cpp | 8 ++++----
 Sming/SmingCore/Network/Ssl/SslValidator.h   | 8 ++++----
 Sming/SmingCore/Network/TcpClient.h          | 2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Sming/SmingCore/Network/Ssl/SslValidator.cpp b/Sming/SmingCore/Network/Ssl/SslValidator.cpp
index 9201da9c47..aba3198073 100644
--- a/Sming/SmingCore/Network/Ssl/SslValidator.cpp
+++ b/Sming/SmingCore/Network/Ssl/SslValidator.cpp
@@ -40,9 +40,9 @@ static bool sslValidatePublicKeySha256(SSL* ssl, void* data)
 	return success;
 }
 
-/* SSLValidatorList */
+/* SslValidatorList */
 
-bool SSLValidatorList::validate(SSL* ssl)
+bool SslValidatorList::validate(SSL* ssl)
 {
 	if(ssl != nullptr && count() == 0) {
 		// No validators specified, always succeed
@@ -73,7 +73,7 @@ bool SSLValidatorList::validate(SSL* ssl)
 	return success;
 }
 
-bool SSLValidatorList::add(const uint8_t* fingerprint, SslFingerprintType type)
+bool SslValidatorList::add(const uint8_t* fingerprint, SslFingerprintType type)
 {
 	SslValidatorCallback callback = nullptr;
 	switch(type) {
@@ -95,7 +95,7 @@ bool SSLValidatorList::add(const uint8_t* fingerprint, SslFingerprintType type)
 	return add(callback, const_cast<uint8_t*>(fingerprint));
 }
 
-bool SSLValidatorList::add(SslFingerprints& fingerprints)
+bool SslValidatorList::add(SslFingerprints& fingerprints)
 {
 	bool success = false;
 	if(fingerprints.certSha1 != nullptr) {
diff --git a/Sming/SmingCore/Network/Ssl/SslValidator.h b/Sming/SmingCore/Network/Ssl/SslValidator.h
index d2c00a470b..4eaca7ff69 100644
--- a/Sming/SmingCore/Network/Ssl/SslValidator.h
+++ b/Sming/SmingCore/Network/Ssl/SslValidator.h
@@ -33,15 +33,15 @@
  */
 typedef std::function<bool(SSL* ssl, void* data)> SslValidatorCallback;
 
-struct SSLValidator {
+struct SslValidator {
 	SslValidatorCallback callback;
 	void* data; ///< Callback-specific data, e.g. fingerprint to compare against
 };
 
-class SSLValidatorList : private Vector<SSLValidator>
+class SslValidatorList : private Vector<SslValidator>
 {
 public:
-	~SSLValidatorList()
+	~SslValidatorList()
 	{
 		// Make sure memory gets released
 		validate(nullptr);
@@ -49,7 +49,7 @@ class SSLValidatorList : private Vector<SSLValidator>
 
 	bool add(SslValidatorCallback callback, void* data)
 	{
-		return Vector::add(SSLValidator{callback, data});
+		return Vector::add(SslValidator{callback, data});
 	}
 
 	/**
diff --git a/Sming/SmingCore/Network/TcpClient.h b/Sming/SmingCore/Network/TcpClient.h
index 5ca7d84820..60bcb6a176 100644
--- a/Sming/SmingCore/Network/TcpClient.h
+++ b/Sming/SmingCore/Network/TcpClient.h
@@ -194,7 +194,7 @@ class TcpClient : public TcpConnection
 	uint16_t asyncTotalSent = 0;
 	uint16_t asyncTotalLen = 0;
 #ifdef ENABLE_SSL
-	SSLValidatorList sslValidators;
+	SslValidatorList sslValidators;
 #endif
 };
 

From ed1ce289be1e990415354327dc3ce722d9f46ad6 Mon Sep 17 00:00:00 2001
From: mikee47 <mike@sillyhouse.net>
Date: Wed, 13 Feb 2019 21:31:12 +0000
Subject: [PATCH 6/6] Accept invalid length of fingerprint to ensure validation
 fails

Move SslFingerprint code out of header
Add check for flash memory in `SslFingerprint::setValue`
---
 .../SmingCore/Network/Ssl/SslFingerprints.cpp | 87 ++++++++++++++++++
 Sming/SmingCore/Network/Ssl/SslFingerprints.h | 88 +++++--------------
 Sming/SmingCore/Network/Ssl/SslValidator.cpp  |  2 +-
 3 files changed, 111 insertions(+), 66 deletions(-)
 create mode 100644 Sming/SmingCore/Network/Ssl/SslFingerprints.cpp

diff --git a/Sming/SmingCore/Network/Ssl/SslFingerprints.cpp b/Sming/SmingCore/Network/Ssl/SslFingerprints.cpp
new file mode 100644
index 0000000000..879a267e38
--- /dev/null
+++ b/Sming/SmingCore/Network/Ssl/SslFingerprints.cpp
@@ -0,0 +1,87 @@
+/****
+ * Sming Framework Project - Open Source framework for high efficiency native ESP8266 development.
+ * Created 2015 by Skurydin Alexey
+ * http://github.com/anakod/Sming
+ * All files of the Sming Core are provided under the LGPL v3 license.
+ ****/
+
+#ifdef ENABLE_SSL
+
+#include "SslFingerprints.h"
+#include <user_config.h>
+#include "flashmem.h"
+
+static inline bool isFlashPtr(const uint8_t* ptr)
+{
+	auto addr = reinterpret_cast<uint32_t>(ptr);
+	return addr >= INTERNAL_FLASH_START_ADDRESS;
+}
+
+static inline void freeValue(const uint8_t*& ptr)
+{
+	delete[] ptr;
+	ptr = nullptr;
+}
+
+void SslFingerprints::free()
+{
+	freeValue(certSha1);
+	freeValue(pkSha256);
+}
+
+bool SslFingerprints::setValue(const uint8_t*& value, unsigned requiredLength, const uint8_t* newValue,
+							   unsigned newLength)
+{
+	if(newValue == nullptr || newLength == 0) {
+		freeValue(value);
+		return true;
+	} else {
+		if(newLength != requiredLength) {
+			debug_w("Warning: Invalid fingerprint length");
+			// Copy data anyway to prevent false positive validation
+		}
+		if(value == nullptr) {
+			value = new uint8_t[requiredLength];
+			if(value == nullptr) {
+				return false;
+			}
+		}
+		// If new value is longer than buffer, copy short
+		unsigned length = std::min(newLength, requiredLength);
+		// Behave properly when source is flash memory and length is wrong or buffers misaligned
+		if(isFlashPtr(newValue)) {
+			memcpy_P(const_cast<uint8_t*>(value), newValue, length);
+		} else {
+			memcpy(const_cast<uint8_t*>(value), newValue, length);
+		}
+		return true;
+	}
+}
+
+SslFingerprints& SslFingerprints::operator=(SslFingerprints& source)
+{
+	if(this != &source) {
+		freeValue(certSha1);
+		certSha1 = source.certSha1;
+		source.certSha1 = nullptr;
+
+		freeValue(pkSha256);
+		pkSha256 = source.pkSha256;
+		source.pkSha256 = nullptr;
+	}
+
+	return *this;
+}
+
+/** @brief Make copy of values from source */
+SslFingerprints& SslFingerprints::operator=(const SslFingerprints& source)
+{
+	if(this != &source) {
+		setSha1(source.certSha1, SHA1_SIZE);
+		setSha256(source.pkSha256, SHA256_SIZE);
+	}
+
+	return *this;
+}
+
+#endif // ENABLE_SSL
diff --git a/Sming/SmingCore/Network/Ssl/SslFingerprints.h b/Sming/SmingCore/Network/Ssl/SslFingerprints.h
index 5ab09e8c17..df361a46ca 100644
--- a/Sming/SmingCore/Network/Ssl/SslFingerprints.h
+++ b/Sming/SmingCore/Network/Ssl/SslFingerprints.h
@@ -38,105 +38,63 @@ enum SslFingerprintType {
  *
  */
 struct SslFingerprints {
-	uint8_t* certSha1 = nullptr; // << certificate SHA1 fingerprint
-	uint8_t* pkSha256 = nullptr; // << public key SHA256 fingerprint
+	const uint8_t* certSha1 = nullptr; // << certificate SHA1 fingerprint
+	const uint8_t* pkSha256 = nullptr; // << public key SHA256 fingerprint
 
 	~SslFingerprints()
 	{
 		free();
 	}
 
-	void free()
-	{
-		delete[] certSha1;
-		certSha1 = nullptr;
-		delete[] pkSha256;
-		pkSha256 = nullptr;
-	}
+	void free();
 
+	/** @brief Set the SHA1 fingerprint
+	 *  @param cert data to copy
+	 *  @param length for checking
+	 *  @retval bool false on length check or allocation failure
+	 */
 	bool setSha1(const uint8_t* cert, unsigned length)
 	{
 		return setValue(certSha1, SHA1_SIZE, cert, length);
 	}
 
-	bool setSha256(const uint8_t* cert, unsigned length)
+	/** @brief Make copy of SHA1 certificate from data stored in flash */
+	bool setSha1_P(const uint8_t* cert, unsigned length)
 	{
-		return setValue(pkSha256, SHA256_SIZE, cert, length);
+		return setValue(certSha1, SHA1_SIZE, cert, length);
 	}
 
-	/** @brief Make copy of SHA1 certificate from data stored in flash
-	 *  @param cert
+	/** @brief Set the SHA256 fingerprint
+	 *  @param cert data to copy
+	 *  @param length for checking
+	 *  @retval bool false on length check or allocation failure
 	 */
-	bool setSha1_P(const uint8_t* cert, unsigned length)
+	bool setSha256(const uint8_t* cert, unsigned length)
 	{
-		// Word-aligned and sized buffers don't need special handling
-		return setSha1(cert, length);
+		return setValue(pkSha256, SHA256_SIZE, cert, length);
 	}
 
-	/** @brief Make copy of SHA256 certificate from data stored in flash
-	 *  @param cert
-	 */
+	/** @brief Make copy of SHA256 certificate from data stored in flash */
 	bool setSha256_P(const uint8_t* cert, unsigned length)
 	{
-		// Word-aligned and sized buffers don't need special handling
-		return setSha256(cert, length);
+		return setValue(pkSha256, SHA256_SIZE, cert, length);
 	}
 
 	/** @brief Moves values out of source */
-	SslFingerprints& operator=(SslFingerprints& source)
-	{
-		if(this != &source) {
-			delete[] certSha1;
-			certSha1 = source.certSha1;
-			source.certSha1 = nullptr;
-
-			delete[] pkSha256;
-			pkSha256 = source.pkSha256;
-			source.pkSha256 = nullptr;
-		}
-
-		return *this;
-	}
+	SslFingerprints& operator=(SslFingerprints& source);
 
 	/** @brief Make copy of values from source */
-	SslFingerprints& operator=(const SslFingerprints& source)
-	{
-		if(this != &source) {
-			setSha1(source.certSha1, SHA1_SIZE);
-			setSha256(source.pkSha256, SHA256_SIZE);
-		}
-
-		return *this;
-	}
+	SslFingerprints& operator=(const SslFingerprints& source);
 
 private:
 	/** @brief Internal method to set a fingerprint
 	 *  @param value Reference to fingerprint value in this structure
-	 *  @param length Required length for value
+	 *  @param requiredLength Expected length for value
 	 *  @param newValue
 	 *  @param newLength
 	 *  @retval bool true on success, false on invalid length or memory allocation failure
 	 */
-	bool setValue(uint8_t*& value, unsigned length, const uint8_t* newValue, unsigned newLength)
-	{
-		if(newValue == nullptr || newLength == 0) {
-			delete[] value;
-			value = nullptr;
-			return true;
-		} else if(newLength != length) {
-			debug_e("ERROR! Invalid fingerprint length");
-			return false;
-		} else {
-			if(value == nullptr) {
-				value = new uint8_t[length];
-				if(value == nullptr) {
-					return false;
-				}
-			}
-			memcpy(value, newValue, length);
-			return true;
-		}
-	}
+	bool setValue(const uint8_t*& value, unsigned requiredLength, const uint8_t* newValue, unsigned newLength);
 };
 
 #endif // SMINGCORE_NETWORK_SSLFINGERPRINTS_H_
diff --git a/Sming/SmingCore/Network/Ssl/SslValidator.cpp b/Sming/SmingCore/Network/Ssl/SslValidator.cpp
index aba3198073..8644e0f0c2 100644
--- a/Sming/SmingCore/Network/Ssl/SslValidator.cpp
+++ b/Sming/SmingCore/Network/Ssl/SslValidator.cpp
@@ -87,7 +87,7 @@ bool SslValidatorList::add(const uint8_t* fingerprint, SslFingerprintType type)
 		debug_d("Unsupported SSL certificate fingerprint type");
 	}
 
-	if(!callback) {
+	if(callback == nullptr) {
 		delete[] fingerprint;
 		return false;
 	}