Skip to content
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

Implement AT$NVM, AT$APKACCESS, and AT$DISUART #117

Closed
wants to merge 6 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
14 changes: 13 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,18 @@ DEBUG_PORT ?= 1
# 2 - PB6 (pin 39). Arduino MKRWAN1310 boards use this pin.
TCXO_PIN ?= 1

# Specific target MKR1310, add a feature to deactivate the serial port based on
# PB12 GPIO. This is because Arduino team used same pins for serial line and SPI
# lines. As a consequence, Serial line activity prevent to use the SPI port and the
# onboarded SerialFlash.
# The Arduino documentation recommands to maintain the modem in a reset mode but this
# will make the LoRaWAN session lost, so this approach is not working. With the ability
# to deactivate the serail line on request, this may solve the problem even if the right
# solution should be to redesign this board.
# 0 - Disable this specific feature
# 1 - Enable this specific feature with control line PB12 set to 0 (pull-up on the line)
MKR1310 ?= 0

################################################################################
# You shouldn't need to edit the text below under normal circumstances. #
################################################################################
Expand Down Expand Up @@ -306,7 +318,7 @@ CFLAGS += -DUSE_FULL_LL_DRIVER

CFLAGS += -DDEFAULT_UART_BAUDRATE=$(DEFAULT_UART_BAUDRATE)

CFLAGS += -DTCXO_PIN=$(TCXO_PIN)
CFLAGS += -DTCXO_PIN=$(TCXO_PIN) -DMKR1310=$(MKR1310)

# Extra flags to be only applied when we compile the souce files from the lib
# subdirectory. Since that sub-directory contains third-party code, disable some
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

The aim of this project is to develop open source LoRaWAN modem for the [Type ABZ](https://github.com/hardwario/lora-modem-abz/wiki/Type-ABZ-Modules) wireless module by Murata. The firmware provides an AT command interface backward compatible with Murata Modem, Murata's proprietary LoRaWAN firmware. The firmware can be used on all Type ABZ variants with an open (user-reprogrammable) microcontroller. Binary firmware images are provided for HARDWARIO [LoRa Module](https://shop.hardwario.com/lora-module/), Arduino [MKR WAN 1300](https://store-usa.arduino.cc/products/arduino-mkr-wan-1300-lora-connectivity), and [MKR WAN 1310](https://store.arduino.cc/products/arduino-mkr-wan-1310) boards.

## Main Features
## Main Features

* Support for [LoRaWAN 1.0.4](https://resources.lora-alliance.org/technical-specifications/ts001-1-0-4-lorawan-l2-1-0-4-specification), [LoRaWAN 1.1](https://resources.lora-alliance.org/technical-specifications/lorawan-specification-v1-1), and regional parameters [RP2-1.0.3](https://resources.lora-alliance.org/technical-specifications/rp2-1-0-3-lorawan-regional-parameters)
* Based on the most recent version (unreleased 4.7.0) of [LoRaMac-node](https://github.com/Lora-net/LoRaMac-node)
Expand Down
92 changes: 89 additions & 3 deletions src/cmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -489,11 +489,22 @@ static void set_appskey(atci_param_t *param)

static void get_appkey(void)
{
atci_print("+OK=");
atci_print_buffer_as_hex(find_key(APP_KEY), SE_KEY_SIZE);
EOL();
if ( sysconf.appkey_readable ) {
atci_print("+OK=");
atci_print_buffer_as_hex(find_key(APP_KEY), SE_KEY_SIZE);
EOL();
} else {
abort(ERR_UNSUPPORTED);
}
}

static void protect_appkey(atci_param_t *param)
{
if (param != NULL) abort(ERR_PARAM);
sysconf.appkey_readable = 0;
sysconf_modified = true;
OK_();
}

static void set_appkey_10(atci_param_t *param)
{
Expand Down Expand Up @@ -2055,6 +2066,76 @@ static void get_session(void)
}



// Read and Write a User data into the NVM area
// AT$NVM=0 read data at address 0
// AT$NVM=0,223 write 223 at address 0
static void nvm_userdata(atci_param_t *param) {
uint32_t adr, value;
if (param == NULL) abort(ERR_PARAM);
if (!atci_param_get_uint(param, &adr)) abort(ERR_PARAM);
if (adr >= USER_NVM_MAX_SIZE) abort(ERR_PARAM);
if (param->offset < param->length) {
if (!atci_param_is_comma(param)) abort(ERR_PARAM);
if (!atci_param_get_uint(param, &value)) abort(ERR_PARAM);
if (value >= 256) abort(ERR_PARAM);
user_nvm.values[adr] = value;
user_nvm_process();
OK_();
} else {
OK("%d",user_nvm.values[adr]);
}
}

#if MKR1310 == 1
// The MKR1310 is using the same wires for SPI and UART
// To be able to use the embedded SPI Flash, we need to switch UART Off
// As uart is used for waking the device up when going into sleep mode
// we will disable the sleep mode too during this period of time
// Uart will be resumed by setting GPIO B12 to LOW (it has a pull-up on the line)
static uint8_t __prev_sleep;
static uint8_t __uart_disable = 0;
static void disable_uart(atci_param_t *param) {
if (param != NULL) abort(ERR_PARAM);
// simpler to add everything in a single place,
// make sure these lines are INPUT to not conflict with external signals from SPI
GPIO_InitTypeDef cfg = {
.Mode = GPIO_MODE_INPUT,
.Pull = GPIO_NOPULL
};
gpio_init(GPIOB, GPIO_PIN_12, &cfg);
gpio_init(GPIOB, GPIO_PIN_13, &cfg);
gpio_init(GPIOB, GPIO_PIN_14, &cfg);
gpio_init(GPIOB, GPIO_PIN_15, &cfg);

// No need to deactivate uart if the wakeup signal is true
int v = gpio_read(GPIOB, GPIO_PIN_12);
if ( v == 0 ) abort(ERR_PARAM);
OK_();

lpuart_disable();
__prev_sleep = sysconf.sleep;
__uart_disable = 1;
sysconf.sleep = 0; // prevent the device to go deep sleep and never waked up due to serial line deactivation
}

// this is monitoring the PB12 line and restart the UART when to comes low
void process_uart_wakeup(void) {
// nothing to do
if ( __uart_disable == 0 ) return;

int v = gpio_read(GPIOB, GPIO_PIN_12);
if ( v == 0 ) {
// restart is requested
lpuart_enable();
sysconf.sleep = __prev_sleep;
__uart_disable = 0;
OK_();
}
}

#endif

static const atci_command_t cmds[] = {
{"+UART", NULL, set_uart, get_uart, NULL, "Configure UART interface"},
{"+VER", NULL, NULL, get_version_comp, NULL, "Firmware version and build time"},
Expand Down Expand Up @@ -2125,6 +2206,11 @@ static const atci_command_t cmds[] = {
{"$SESSION", NULL, NULL, get_session, NULL, "Get network session information"},
{"$CW", cw, NULL, NULL, NULL, "Start continuous carrier wave transmission"},
{"$CM", cm, NULL, NULL, NULL, "Start continuous modulated FSK transmission"},
{"$NVM", nvm_userdata, NULL, NULL, NULL, "Write / Read userdata to/from 64 bytes of NVM"},
{"$APKACCESS", protect_appkey, NULL, NULL, NULL, "Protect AppKey against read access"},
Copy link
Collaborator

@janakj janakj Nov 5, 2022

Choose a reason for hiding this comment

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

This is an interesting idea, but as currently implemented this would only provide minimal (accidental) protection. As long as you have access to USART1 or USART2 on the Type ABZ module, which all devices that I have seen have to support firmware updates, you can always access any of the keys stored in EEPROM.

One way to do it is to flash a modified version of the firmware into the modem, or even just an older version of this firmware. Another method is to switch the STM32 inside Type ABZ into a bootloader mode and then you can read the EEPROM from there. This firmware currently does not lock down the Type ABZ module in any way, so these access methods are always possible.

Also, the AppKey is only used in the OTAA mode to derive the AppSKey. In the APB mode, the modem directly uses the AppSKey. Since we support both modes in the firmware, we would need to lock both keys.

It might be a good idea to think about how we could provide more protection for the various security keys, but perhaps we should do that in a separate PR or discussion thread. If you don't mind, I would like to keep this PR for the user NVM feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, this should also applied to AppSkey and NetSkey

Copy link
Collaborator

Choose a reason for hiding this comment

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

If that's not secret, what's your use case for AT$APKACCESS? Did you have any specific attacks or threats in mind?

Copy link
Contributor Author

@disk91 disk91 Nov 16, 2022

Choose a reason for hiding this comment

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

The idea is to protect the keys and make access to the most private one "AppKey" as much complex as possible. As the modem is saving it into the eeprom you don't need to store it in the MCU it is really great, it can be removed from the MCU firmware with a dynamic loading, perfect.
But it's really easy to send an AT command direectly on the modem line from an external system to extract it. So if the AT command can be blocked, the only way is to attack the eeprom, this is more complex. To go to the next step, you can take a look at my software secure store for keys in my abz sdk on my github.
Here I don't want to implement a such security level but it's just to avoid a so simple way to extract keys.

Honestly, i'm not really think about a real protection way with this command, but it's to avoid all the "security researchers" and other script kiddies mocking on IoT because they discovered how to send a bit on a serial line wire... so it's "best practice" to make a such minimal protection on credentials.

#if MKR1310 == 1
{"$DISUART", disable_uart, NULL, NULL, NULL, "Disable UART"},
#endif
ATCI_COMMAND_CLAC,
ATCI_COMMAND_HELP};

Expand Down
4 changes: 4 additions & 0 deletions src/cmd.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,8 @@ void cmd_ans(unsigned int margin, unsigned int gwcnt);
#define cmd_print atci_print
#define cmd_printf atci_printf

#if MKR1310 == 1
void process_uart_wakeup(void);
#endif

#endif // _CMD_H
12 changes: 12 additions & 0 deletions src/lpuart.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ void lpuart_init(unsigned int baudrate)
}



static void init_gpio(void)
{
GPIO_InitTypeDef gpio = {
Expand Down Expand Up @@ -168,6 +169,17 @@ static void deinit_gpio(void)
}


void lpuart_disable(void) {
lpuart_flush();
HAL_UART_DMAPause(&port);
deinit_gpio();
}

void lpuart_enable() {
init_gpio();
HAL_UART_DMAResume(&port);
}

void HAL_UART_MspInit(UART_HandleTypeDef *port)
{
/* Enable peripherals and GPIO Clocks */
Expand Down
14 changes: 14 additions & 0 deletions src/lpuart.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,18 @@ void lpuart_before_stop(void);
void lpuart_after_stop(void);


/*! @brief Disable LPUART1 Gpio for MKRWAN1310
*
* This function is specific to MKRWAN1310, it disable the lpuart pins to allow sharing the
* wires on the board with the SPI to use the onboarded flash.
*/
void lpuart_disable(void);

/*! @brief Reinit LPUART1 Gpio for MKRWAN1310
*
* This function is specific to MKRWAN1310, it reinit the lpuart pins to allow sharing the
* wires on the board with the SPI to use the onboarded flash.
*/
void lpuart_enable(void);

#endif /* __LPUART_H__ */
4 changes: 3 additions & 1 deletion src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ int main(void)
cmd_event(CMD_EVENT_MODULE, CMD_MODULE_BOOT);

while (1) {
#if MKR1310 == 1
process_uart_wakeup();
#endif
cmd_process();
lrw_process();
sysconf_process();
Expand Down Expand Up @@ -91,7 +94,6 @@ int main(void)
}

enable_irq();

// Invoke lrw_process as the first thing after waking up to give the MAC
// a chance to timestamp incoming downlink as quickly as possible.
lrw_process();
Expand Down
42 changes: 38 additions & 4 deletions src/nvm.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "nvm.h"
#include <assert.h>
#include <string.h>
#include <strings.h>
#include <stm/include/stm32l072xx.h>
#include <loramac-node/src/mac/LoRaMacTypes.h>
#include <loramac-node/src/mac/LoRaMac.h>
Expand All @@ -11,7 +12,7 @@
#include "part.h"
#include "utils.h"

#define NUMBER_OF_PARTS 8
#define NUMBER_OF_PARTS 9


/* The following partition sizes have been derived from the in-memory size of
Expand All @@ -28,8 +29,9 @@
#define MAC2_PART_SIZE 512
#define SE_PART_SIZE 512
#define REGION1_PART_SIZE 32
#define REGION2_PART_SIZE 1536
#define REGION2_PART_SIZE 1310
#define CLASSB_PART_SIZE 32
#define USER_NVM_PART_SIZE 72


// Make sure each data structure fits into its fixed-size partition
Expand All @@ -41,6 +43,7 @@ static_assert(sizeof(SecureElementNvmData_t) <= SE_PART_SIZE, "SecureElement NVM
static_assert(sizeof(RegionNvmDataGroup1_t) <= REGION1_PART_SIZE, "RegionGroup1 NVM data too long");
static_assert(sizeof(RegionNvmDataGroup2_t) <= REGION2_PART_SIZE, "RegionGroup2 NVM data too long");
static_assert(sizeof(LoRaMacClassBNvmData_t) <= CLASSB_PART_SIZE, "ClassB NVM data too long");
static_assert(sizeof(user_nvm_t) <= USER_NVM_PART_SIZE, "User NVM data too long");


// And also make sure that NVM data fits into the EEPROM twice. This is
Expand All @@ -53,7 +56,8 @@ static_assert(
SE_PART_SIZE +
REGION1_PART_SIZE +
REGION2_PART_SIZE +
CLASSB_PART_SIZE
CLASSB_PART_SIZE +
USER_NVM_PART_SIZE
<= (DATA_EEPROM_BANK2_END - DATA_EEPROM_BASE + 1 - PART_TABLE_SIZE(NUMBER_OF_PARTS)) / 2,
"NVM data does not fit into a single EEPROM bank");

Expand All @@ -71,6 +75,8 @@ static part_block_t nvm = {

struct nvm_parts nvm_parts;

user_nvm_t user_nvm = { 0 };

sysconf_t sysconf = {
.uart_baudrate = DEFAULT_UART_BAUDRATE,
.uart_timeout = 1000,
Expand All @@ -79,7 +85,8 @@ sysconf_t sysconf = {
.sleep = 1,
.device_class = CLASS_A,
.unconfirmed_retransmissions = 1,
.confirmed_retransmissions = 8
.confirmed_retransmissions = 8,
.appkey_readable = 1
};

bool sysconf_modified;
Expand Down Expand Up @@ -147,6 +154,11 @@ void nvm_init(void)
nvm_parts.classb.dsc->size != CLASSB_PART_SIZE)
goto retry;

if ((part_find(&nvm_parts.user, &nvm, "user") &&
part_create(&nvm_parts.user, &nvm, "user", USER_NVM_PART_SIZE)) ||
nvm_parts.user.dsc->size != USER_NVM_PART_SIZE)
goto retry;

size_t size;
const uint8_t *p = part_mmap(&size, &nvm_parts.sysconf);
if (check_block_crc(p, sizeof(sysconf))) {
Expand All @@ -155,6 +167,17 @@ void nvm_init(void)
} else {
log_debug("Invalid system configuration checksum, using defaults");
}

p = part_mmap(&size, &nvm_parts.user);
if (check_block_crc(p, sizeof(user_nvm))) {
log_debug("Restoring user data from NVM");
memcpy(&user_nvm, p, sizeof(user_nvm));
} else {
log_debug("Invalid user data checksum, using defaults");
user_nvm.magic = USER_NVM_MAGIC;
bzero(user_nvm.values,USER_NVM_MAX_SIZE);
}

return;

retry:
Expand Down Expand Up @@ -192,3 +215,14 @@ void sysconf_process(void)
sysconf_modified = false;
}


void user_nvm_process(void)
{

if (update_block_crc(&user_nvm, sizeof(user_nvm))) {
log_debug("Saving user data to NVM");
if (!part_write(&nvm_parts.user, 0, &user_nvm, sizeof(user_nvm)))
log_error("Error while writing user data to NVM");
}

}
16 changes: 16 additions & 0 deletions src/nvm.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ typedef struct sysconf
*/
uint8_t confirmed_retransmissions;

/* This is allowing to read the appKey from the serial line of not
* once set to false (0) it will not be possible to retrieve the appKey
*/
uint8_t appkey_readable:1;

uint32_t crc32;
} sysconf_t;

Expand All @@ -66,18 +71,29 @@ struct nvm_parts {
part_t region1;
part_t region2;
part_t classb;
part_t user;
};

#define USER_NVM_MAX_SIZE 64 // maximum allows values inside the User Nvm area
#define USER_NVM_MAGIC 0xD15C9101
typedef struct user_nvm_s {
uint32_t magic;
uint8_t values[USER_NVM_MAX_SIZE];
uint32_t crc32;
} user_nvm_t;


extern struct nvm_parts nvm_parts;
extern sysconf_t sysconf;
extern bool sysconf_modified;
extern uint16_t nvm_flags;
extern user_nvm_t user_nvm;

void nvm_init(void);

int nvm_erase(void);

void sysconf_process(void);
void user_nvm_process(void);

#endif // _NVM_H_