Restores public server key and server address if missing #1300

technobly · 2017-04-16T18:46:47Z

Problem

There have been a number of community reports of devices losing their Public Server Key and/or Server Address. Typically this is due to a power loss or reset during a critical flash write operation.

Solution

Public Server Key and Server Address are baked into System Firmware with this PR, and will be restored in RAM before a connection is attempted if either is found to be missing.
This data will not be automatically written to FLASH when found missing to prevent further corruption.
A cloud event will generated by the device alerting the cloud to which error is currently being patched by system firmware. (currently this does not hit the user's event stream)
A subscription to a cloud event has been added to system firmware that will allow the user to restore the data to FLASH remotely when deemed appropriate. (currently there is no mechanism for the user to do this)
The TCP server address in PSK is NULL terminated (backup copy in system firmware matches)
The UDP server address in PSK is NOT NULL terminated (backup copy in system firmware will now include a NULL terminated server address).
pubkey and private_key arrays in system_cloud_internal.cpp were not initialized and would contain garbage wherever key data was not written to, they are not initiallized to 0xFF to mimick erased flash memory.
PR v2 dcd format and tests #1295 offers hardening and error checking to the FLASH read/write routines to help prevent this PR from being necessary, but just in case we will still be able to recover.

Steps to Test

All features have been manually tested on Photon/Electron/Core.
Use dfu-util to erase [D] and verify [U] data at the appropriate address
- Photon - dfu-util -d 2b04:d006 -a 1 -s 2082:768 -[D|U] blank.der
- Electron - dfu-util -d 2b04:d00a -a 1 -s 3298:320 -[D|U] blank.der
- Core - dfu-util -d 1d50:607f -a 1 -s 4096:768 -[D|U] blank.der
Change system subscription/publish event name from spark to spork and publish PUBLIC event spork/device/key/restore to restore PSK and Server Address.

Example App

This is an internal system change. No example application.

References

This will likely have a conflict with PR Support for Twilio SIMs by default in system firmware [ch1537] #1288 due to logging code where the public server address is read (prefer Support for Twilio SIMs by default in system firmware [ch1537] #1288's logging changes)
Related to PR v2 dcd format and tests #1295

Completeness

User is totes amazing for contributing!
Contributor has signed CLA (Info here)
Problem and Solution clearly stated
Run unit/integration/application tests on device
Added documentation (N/A)
Added to CHANGELOG.md after merging (add links to docs and issues)

Enhancement

[PR #1300] Restores public server key and server address if missing

m-mcgowan · 2017-04-16T19:01:10Z

hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp

@@ -272,6 +272,16 @@ void HAL_FLASH_Read_ServerAddress(ServerAddress* server_addr)
    parseServerAddressData(server_addr, (const uint8_t*)data, DCT_SERVER_ADDRESS_SIZE);
 }

+void HAL_FLASH_Write_ServerAddress(uint8_t *buf)


I suggest that the protocol be part of the call so that the definition is self-defined and unambiguous, rather than defaulting to the current config. It will make application code simpler too, since the app also knows what kind of key is being used.

m-mcgowan · 2017-04-16T19:01:16Z

hal/inc/ota_flash_hal.h

 /* Length in bytes of DER-encoded 1024-bit RSA private key */
 #define EXTERNAL_FLASH_CORE_PRIVATE_KEY_LENGTH		(612)

 void HAL_FLASH_Read_ServerAddress(ServerAddress *server_addr);
+void HAL_FLASH_Write_ServerAddress(uint8_t *buf);


could you make the pointer const please :-)

avtolstoy · 2017-05-04T02:47:54Z

system/src/system_cloud_internal.cpp

@@ -475,6 +478,24 @@ void SystemEvents(const char* name, const char* data)
                System.reset();
        }
    }
+    if (!strncmp(name, KEY_RESTORE_EVENT, strlen(KEY_RESTORE_EVENT))) {


What's the usecase for this? If your key/server address information was corrupted/removed then you won't be able to connect to server at all. And if you are already connected and you are able to receive this event from the server, it means that you probably have valid public key and server address 😅

The PSK and server address can be restored in RAM from system flash... so you can connect again. We decided not to automatically restore to DCT/DCD in case of conditions still present that may have caused the first corruption; perhaps they may further corrupt something else during the next flash write. We will track metrics on units reporting corruption, and in the future enable a way for users to send the restore event.

avtolstoy · 2017-05-04T02:54:28Z

system/inc/system_cloud.h

@@ -179,6 +197,58 @@ ProtocolFacade* system_cloud_protocol_instance(void);

 int spark_set_connection_property(unsigned property_id, unsigned data, void* datap, void* reserved);

+// minimal udp public server key
+const unsigned char backup_udp_public_server_key[] = {


These should be defined in a source file instead of a header.

Good point, I'll make that change.

…d use

avtolstoy · 2017-05-05T13:28:52Z

system/src/system_cloud_internal.cpp

@@ -68,7 +68,7 @@ static sock_handle_t sparkSocket = socket_handle_invalid();

 ProtocolFacade* sp;

-static Flags<ParticleKeyErrorFlag> particle_key_errors;
+static uint32_t particle_key_errors;


Just to be on a safe side I would initialize it to 0.

technobly added the enhancement label Apr 16, 2017

technobly added this to the 0.7.0 milestone Apr 16, 2017

technobly requested a review from m-mcgowan April 16, 2017 18:46

m-mcgowan reviewed Apr 18, 2017

View reviewed changes

avtolstoy requested changes May 4, 2017

View reviewed changes

technobly added 5 commits May 4, 2017 21:09

restores public server key and server address if missing

2ee1c1a

adds missing functions for GCC build

5926f04

makes pointers const, and lets system decide which protocol HAL shoul…

db00363

…d use

opt for simpler flag usage with system firmware

254db2c

move data objects to a source file

3d66a69

technobly force-pushed the feature/psk-servaddr-repair branch from 6199308 to 3d66a69 Compare May 5, 2017 02:13

technobly requested a review from avtolstoy May 5, 2017 02:15

avtolstoy approved these changes May 5, 2017

View reviewed changes

initialize particle_key_errors just to be safe

cad5abc

technobly merged commit 37116a5 into develop May 9, 2017

technobly deleted the feature/psk-servaddr-repair branch May 9, 2017 17:16

sergeuz mentioned this pull request Dec 1, 2022

Server key rotation #2570

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restores public server key and server address if missing #1300

Restores public server key and server address if missing #1300

technobly commented Apr 16, 2017 •

edited

Loading

m-mcgowan Apr 16, 2017

m-mcgowan Apr 16, 2017

avtolstoy May 4, 2017

technobly May 4, 2017

avtolstoy May 4, 2017

technobly May 4, 2017

avtolstoy May 5, 2017

technobly May 5, 2017

Restores public server key and server address if missing #1300

Restores public server key and server address if missing #1300

Conversation

technobly commented Apr 16, 2017 • edited Loading

Problem

Solution

Steps to Test

Example App

References

Completeness

Enhancement

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

technobly commented Apr 16, 2017 •

edited

Loading