Skip to content

Signed updates #3105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Dockerfile
.DS_Store
tools/dist/
tools/xtensa-lx106-elf/
Expand All @@ -8,11 +9,14 @@ exclude.txt
tools/sdk/lib/liblwip_src.a
tools/sdk/lwip/src/build
tools/sdk/lwip/src/liblwip_src.a
tests/host/bin
tools/sdk/ld/backup
tools/sdk/ld/eagle.app.v6.common.ld
tests/host/lcov/

tests/hosts/lcov/

*.o
*.gc??
*.gc??
*.pyc
*.gch

Expand Down
4 changes: 2 additions & 2 deletions cores/esp8266/MD5Builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ uint8_t hex_char_to_byte(uint8_t c){

void MD5Builder::begin(void){
memset(_buf, 0x00, 16);
MD5Init(&_ctx);
MD5_Init(&_ctx);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, suggest keeping ROM functions for performance reasons.

}

void MD5Builder::add(const uint8_t * data, const uint16_t len){
Expand Down Expand Up @@ -59,7 +59,7 @@ bool MD5Builder::addStream(Stream & stream, const size_t maxLen){
}

// Update MD5 with buffer payload
MD5Update(&_ctx, buf, numBytesRead);
MD5_Update(&_ctx, buf, numBytesRead);

yield(); // time for network streams

Expand Down
4 changes: 2 additions & 2 deletions cores/esp8266/MD5Builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@

#include <WString.h>
#include <Stream.h>
#include "md5.h"
#include <crypto/crypto.h>

class MD5Builder {
private:
md5_context_t _ctx;
MD5_CTX _ctx;
uint8_t _buf[16];
public:
void begin(void);
Expand Down
164 changes: 162 additions & 2 deletions cores/esp8266/Updater.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ UpdaterClass::UpdaterClass()
, _currentAddress(0)
, _command(U_FLASH)
{
#ifdef VERIFY_SIGNATURE
_ca_ctx = (CA_CERT_CTX *)malloc(sizeof(CA_CERT_CTX));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest allocating this only when the feature gets actually used.

#endif
}

void UpdaterClass::_reset() {
Expand Down Expand Up @@ -166,8 +169,52 @@ bool UpdaterClass::end(bool evenIfRemaining){
_size = progress();
}

_md5.calculate();
#ifdef VERIFY_SIGNATURE
// If this package has been signed correctly, the last uint32 is the size of the signature
// the second-last uint32 is the size of the certificate
ESP.flashRead(_startAddress + _size - sizeof(uint32_t), &_signatureLen, sizeof(uint32_t));
ESP.flashRead(_startAddress + _size - (2 * sizeof(uint32_t)), &_certificateLen, sizeof(uint32_t));
_signatureStartAddress = _startAddress + _size - (2 * sizeof(uint32_t)) - _signatureLen;
_certificateStartAddress = _signatureStartAddress - _certificateLen;

#ifdef DEBUG_UPDATER
DEBUG_UPDATER.printf("\n");
DEBUG_UPDATER.printf("[begin] _signatureLen: 0x%08X (%d)\n", _signatureLen, _signatureLen);
DEBUG_UPDATER.printf("[begin] _certificateLen: 0x%08X (%d)\n", _certificateLen, _certificateLen);
DEBUG_UPDATER.printf("[begin] _signatureStartAddress: 0x%08X (%d)\n", _signatureStartAddress, _signatureStartAddress);
DEBUG_UPDATER.printf("[begin] _certificateStartAddress: 0x%08X (%d)\n", _certificateStartAddress, _certificateStartAddress);
#endif

if(!_decryptMD5()) {
#ifdef DEBUG_UPDATER
DEBUG_UPDATER.printf("MD5 Decryption Failed.\n");
#endif
_reset();
return false;
}
#endif

if(_target_md5.length()) {
// If there is a target MD5 hash set, we now take the md5 hash of the binary
int bin_size = (int)_size;
#ifdef VERIFY_SIGNATURE
bin_size -= (int)(_signatureLen + _certificateLen + (2 * sizeof(uint32_t)));
#endif

uint8_t *bin_buffer = (uint8_t *)malloc(sizeof(uint8_t) * 32);
for(int i = 0; i < bin_size; i += 32) {
ESP.flashRead(_startAddress + i, (uint32_t *)bin_buffer, 32);

int read = bin_size - i;
if(read > 32) {
read = 32;
}

_md5.add(bin_buffer, read);
}
_md5.calculate();
free(bin_buffer);

if(_target_md5 != _md5.toString()){
_setError(UPDATE_ERROR_MD5);
_reset();
Expand Down Expand Up @@ -255,7 +302,6 @@ bool UpdaterClass::_writeBuffer(){
_setError(UPDATE_ERROR_WRITE);
return false;
}
_md5.add(_buffer, _bufferLen);
_currentAddress += _bufferLen;
_bufferLen = 0;
return true;
Expand Down Expand Up @@ -381,6 +427,120 @@ size_t UpdaterClass::writeStream(Stream &data) {
return written;
}

#ifdef VERIFY_SIGNATURE
int UpdaterClass::addCA(const uint8_t *cert, int *len) {
// TODO: Allow more than one CA
int res = x509_new(cert, len, &(_ca_ctx->cert[0]));

#ifdef DEBUG_UPDATER
if(res == X509_OK) {
DEBUG_UPDATER.printf("Loaded CA certificate. Common Name: %s\n", _ca_ctx->cert[0]->cert_dn[X509_COMMON_NAME]);
} else {
DEBUG_UPDATER.printf("Unable to load CA certificate: %i\n", res);
}
#endif

return res;
}

bool UpdaterClass::_loadCertificate(X509_CTX **ctx) {
size_t num_of_bits = sizeof(uint8_t) * _certificateLen;
uint8_t *cert = (uint8_t *)malloc(num_of_bits + (num_of_bits % 32)); // Round up to the next uint32_t boundary
ESP.flashRead(_certificateStartAddress, (uint32_t *)cert, num_of_bits);

int res = x509_new(cert, (int *)&_certificateLen, ctx);
free(cert);

if(res != X509_OK) {
#ifdef DEBUG_UPDATER
DEBUG_UPDATER.printf("Unable to load developer certificate: %i\n", res);
#endif
return false;
}
#ifdef DEBUG_UPDATER
DEBUG_UPDATER.printf("Loaded developer certificate. Common Name: %s\n", (*ctx)->cert_dn[X509_COMMON_NAME]);
#endif
return true;
}

bool UpdaterClass::_verifyCertificate(X509_CTX **ctx) {
int res = x509_verify(_ca_ctx, *ctx);

#ifdef DEBUG_UPDATER
if(res == X509_OK) {
DEBUG_UPDATER.printf("Developer certificate verified\n");
} else {
DEBUG_UPDATER.printf("Developer certificate not verified: %i\n", res);
}
#endif

return res == X509_OK;
}

bool UpdaterClass::_decryptSignature(X509_CTX **ctx, char **hash) {
size_t num_of_bits = sizeof(uint8_t) * _signatureLen;
uint8_t *sig = (uint8_t *)malloc(num_of_bits + (num_of_bits % 32)); // Round up to the next uint32_t boundary
ESP.flashRead(_signatureStartAddress, (uint32_t *)sig, num_of_bits);

//const int signature_size = (*ctx)->rsa_ctx->num_octets;
const int signature_size = _signatureLen; // In this version of the library, the num_octects is 0. Not sure why...
#ifdef DEBUG_UPDATER
DEBUG_UPDATER.printf("Size of output buffer: %i\n", signature_size);
#endif
uint8_t sig_data[signature_size];
int len = RSA_decrypt((*ctx)->rsa_ctx, (const uint8_t *)sig, sig_data, signature_size, 0);
free(sig);

if(len == -1) {
#ifdef DEBUG_UPDATER
DEBUG_UPDATER.printf("Decryption failed\n");
#endif
return false;
}

if(len < MD5_SIZE) {
#ifdef DEBUG_UPDATER
DEBUG_UPDATER.printf("Decryption failed: Signature is too short. Expected %i, got %i\n", MD5_SIZE, len);
#endif
return false;
}

#ifdef DEBUG_UPDATER
DEBUG_UPDATER.printf("Decryption successful.\n");
#endif

// Fetch the last part of the encrypted string - that is the MD5 hash, and then save the string
// version of it in to the hash pointer.
(*hash) = (char *)calloc((MD5_SIZE * 2) + 1, sizeof(char));
for(int i = 0; i < MD5_SIZE; i++) {
sprintf(*hash + (i * 2), "%02x", sig_data[len - MD5_SIZE + i]);
}

return true;
}

bool UpdaterClass::_decryptMD5() {
X509_CTX *ctx;

if(!_loadCertificate(&ctx)) {
return false;
}

if(!_verifyCertificate(&ctx)) {
return false;
}

char *hash;
if(!_decryptSignature(&ctx, &hash)) {
return false;
}

setMD5((const char *)hash);

return true;
}
#endif

void UpdaterClass::_setError(int error){
_error = error;
#ifdef DEBUG_UPDATER
Expand Down
24 changes: 24 additions & 0 deletions cores/esp8266/Updater.h
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
#ifndef ESP8266UPDATER_H
#define ESP8266UPDATER_H

#define VERIFY_SIGNATURE 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posted a note about using preprocessor macro as a feature switch above.


#include <Arduino.h>
#include <flash_utils.h>
#include <MD5Builder.h>

#ifdef VERIFY_SIGNATURE
#include <ssl/crypto_misc.h>
#include <crypto/crypto.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these headers in user-facing header file?

#endif

#define UPDATE_ERROR_OK (0)
#define UPDATE_ERROR_WRITE (1)
#define UPDATE_ERROR_ERASE (2)
Expand Down Expand Up @@ -141,13 +148,30 @@ class UpdaterClass {
return written;
}

#ifdef VERIFY_SIGNATURE
int addCA(const uint8_t *cert, int *len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to WiFiClientSecure, suggest exposing a helper template method which loads CA certificate from a file (Stream).

#endif

private:
void _reset();
bool _writeBuffer();

bool _verifyHeader(uint8_t data);
bool _verifyEnd();

#ifdef VERIFY_SIGNATURE
CA_CERT_CTX *_ca_ctx;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, i see now why the headers were needed. If you go for the approach with VerificationTraits interface (call it some other way if you like), you will not have to expose these internals to the users, introducing axTLS names into the sketch global namespace.

bool _loadCertificate(X509_CTX **ctx);
bool _verifyCertificate(X509_CTX **ctx);
bool _decryptSignature(X509_CTX **ctx, char **hash);
bool _decryptMD5();

uint32_t _certificateStartAddress;
uint32_t _certificateLen;
uint32_t _signatureStartAddress;
uint32_t _signatureLen;
#endif

void _setError(int error);

bool _async;
Expand Down
5 changes: 4 additions & 1 deletion tests/host/Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
BINARY_DIRECTORY := bin
LIB_DIRECTORY := lib
LCOV_DIRECTORY := lcov
OUTPUT_BINARY := $(BINARY_DIRECTORY)/host_tests
CORE_PATH := ../../cores/esp8266
SDK_PATH := ../../tools/sdk/include

# I wasn't able to build with clang when -coverage flag is enabled, forcing GCC on OS X
ifeq ($(shell uname -s),Darwin)
Expand Down Expand Up @@ -47,6 +49,7 @@ MOCK_C_FILES := $(addprefix common/,\
INC_PATHS += $(addprefix -I, \
common \
$(CORE_PATH) \
$(SDK_PATH) \
)

TEST_CPP_FILES := \
Expand Down Expand Up @@ -122,4 +125,4 @@ $(BINARY_DIRECTORY)/core.a: $(C_OBJECTS) $(CPP_OBJECTS_CORE)
ranlib -c $@

$(OUTPUT_BINARY): $(BINARY_DIRECTORY) $(CPP_OBJECTS_TESTS) $(BINARY_DIRECTORY)/core.a
$(CXX) $(LDFLAGS) $(CPP_OBJECTS_TESTS) $(BINARY_DIRECTORY)/core.a $(LIBS) -o $(OUTPUT_BINARY)
$(CXX) $(LDFLAGS) $(CPP_OBJECTS_TESTS) $(BINARY_DIRECTORY)/core.a $(LIB_DIRECTORY)/libaxtls.a $(LIBS) -o $(OUTPUT_BINARY)
Binary file added tests/host/lib/libaxtls.a
Binary file not shown.
Loading