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

drivers/at86rf2xx; spi security module (AES) #14113

Merged
merged 2 commits into from
Aug 12, 2020
Merged
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
6 changes: 6 additions & 0 deletions drivers/at86rf2xx/Makefile
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
# exclude submodule sources from *.c wildcard source selection
SRC := $(filter-out aes_spi.c,$(wildcard *.c))

# enable submodules
SUBMODULES := 1

include $(RIOTBASE)/Makefile.base
350 changes: 350 additions & 0 deletions drivers/at86rf2xx/aes_spi.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,350 @@
/* Copyright (C) 2020 Otto-von-Guericke-Universität Magdeburg
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup drivers_at86rf2xx
* @{
*
* @file
* @brief Implementation of at86rf2xx SPI security module (AES)
*
* @author Fabian Hüßler <fabian.huessler@ovgu.de>
* @}
*/

#include "xtimer.h"
#include "periph/spi.h"
#include "at86rf2xx_aes.h"

#define ENABLE_DEBUG (0)
#include "debug.h"

#define AES_DEBUG(...) DEBUG("[at86rf2xx_aes_spi]: "__VA_ARGS__)

#define AT86RF2XX_CMD_SRAM_READ (0b00000000)
#define AT86RF2XX_CMD_SRAM_WRITE (0b01000000)

static inline
void at86rf2xx_spi_get_bus(const at86rf2xx_t *dev)
{
spi_acquire(dev->params.spi, dev->params.cs_pin, SPI_MODE_0, dev->params.spi_clk);
}

static inline
void at86rf2xx_spi_release_bus(const at86rf2xx_t *dev)
{
spi_release(dev->params.spi);
}
Comment on lines +31 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

You could move those to at86rf2xx_internal.h if you want.
But might also be done in a later patch. I was wondering why you didn't just use at86rf2xx_sram_read() and friends, but I guess the problem there is that they will all acquire and release the bus.

Only recently I have learned that this is a bad idea (and even more so after the recent power optimizations after release() on sam0), so the right courser of action would be to convert the driver to not acquire() and release() the bus, just like you did here.

But that would be another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but I guess the problem there is that they will all acquire and release the bus.

Yes.
And further at86rf2xx_sram_read() writes dummy bytes to the transceiver, while
at86rf2xx_sram_write() does not care what it reads from the transceiver.
For fast SPI, you read the result from the last block operation, while you write the next block.


static inline
uint8_t _aes_status(at86rf2xx_t *dev)
Copy link
Member

Choose a reason for hiding this comment

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

can't this be replaced with the at86rf2xx_sram_xxx functions (from 'at86rf2xx_internal.c`)?

Copy link
Contributor

Choose a reason for hiding this comment

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

{
spi_transfer_byte(dev->params.spi, dev->params.cs_pin, true,
AT86RF2XX_CMD_SRAM_READ);
spi_transfer_byte(dev->params.spi, dev->params.cs_pin, true,
AT86RF2XX_REG__AES_STATUS);
return spi_transfer_byte(dev->params.spi, dev->params.cs_pin, false, 0);
}

static inline
void _aes_wait_for_result(at86rf2xx_t *dev)
{
xtimer_usleep(AT86RF2XX_AES_DELAY_US);
uint8_t status = _aes_status(dev);
/*
If this assert fires, there probably is an implementation error.
The error bit is set before the transceiver has processed a data block.
There are two cases:
1. The delay between initiating an AES operation and sending the next cfg
to AT86RF2XX_REG__AES_CTRL was too short. Meaning the transceiver
did not have enough time to process the current block.
2. Less then 16 bytes of data have been sent to the transceiver.

Both should not occur in the code.
*/
assert(!(status & AT86RF2XX_AES_STATUS_MASK__AES_ER));
while (!(status & AT86RF2XX_AES_STATUS_MASK__AES_DONE)) {
AES_DEBUG("status: %02x\n", status);
status = _aes_status(dev);
}
Comment on lines +70 to +73
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we still get the AT86RF2XX_AES_STATUS_MASK__AES_ER bit set here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No the error bit is set before the transceiver has processed a data block. There are two cases:

  1. The delay between initiating an AES operation and sending the next cfg to AT86RF2XX_REG__AES_CTRL was too short. Meaning the transceiver did not have enough time to process the current block.
  2. Less then 16 bytes of data have been sent to the transceiver. This never happens in the code, so this case cannot arrive.

Copy link
Contributor

Choose a reason for hiding this comment

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

Better add that as a comment so in the future this question doesn't come up again :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually the first case should not arrive either because I am polling if the operation is complete.
I could make an assert() that the error bit is never set and change the return types of all functions to be void because that was the only thing that I expected that could go wrong. Every time I witnessed the error bit was set, there was a mistake in the code.
Or there may be something that I am not aware of and the current implementation would be a little safer.
But I don´t think so. Even when I reduce AT86RF2XX_AES_DELAY_US, the error is not set because the polling is safe.
So I would like to turn it into an assert()

}

static inline
void _aes_open_read(at86rf2xx_t *dev, uint8_t addr)
Copy link
Member

Choose a reason for hiding this comment

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

ditto

{
spi_transfer_byte(dev->params.spi, dev->params.cs_pin, true,
AT86RF2XX_CMD_SRAM_READ);
spi_transfer_byte(dev->params.spi, dev->params.cs_pin, true,
addr);
}

static inline
void _aes_open_write(at86rf2xx_t *dev, uint8_t addr)
Copy link
Member

Choose a reason for hiding this comment

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

ditto

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 cannot reuse them because, they acquire / release the bus in each call and at86rf2xx_sram_write() ignores what is read from the transceiver, which is important for fast SPI.

Copy link
Member

Choose a reason for hiding this comment

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

hmmm I would say that these are "presentation layer" functions (not specific to AES). Maybe they can be added as generic functions? What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generally I agree, they could be moved to at86rf2xx_internal because they are not related to the security module. But I am not sure if I should do this because nothing else uses it, so far.

On the one side it touches the actual driver, and I think this should be done, if some day someone decides to modify the at86rf2xx SPI functions in a way that they do not acquire / release the SPI bus for every call. Right now I see no benefit of moving them.

On the other side it doesn´t harm because it is a small change.

I would keep them in place for now.

{
spi_transfer_byte(dev->params.spi, dev->params.cs_pin, true,
AT86RF2XX_CMD_SRAM_WRITE);
spi_transfer_byte(dev->params.spi, dev->params.cs_pin, true,
addr);
}

static inline
void _aes_transfer_bytes(at86rf2xx_t *dev, bool cont, const void* out,
void* in, size_t len)
{
spi_transfer_bytes(dev->params.spi, dev->params.cs_pin, cont, out,
in, len);
}

static inline
void _aes_save_key(at86rf2xx_t *dev, uint8_t cfg, uint8_t key[AT86RF2XX_AES_BLOCK_SIZE])
{
_aes_open_write(dev, AT86RF2XX_REG__AES_CTRL);
_aes_transfer_bytes(dev, false, &cfg, NULL, sizeof(cfg));
_aes_open_read(dev, AT86RF2XX_REG__AES_KEY_START);
_aes_transfer_bytes(dev, false, NULL, key, AT86RF2XX_AES_KEY_LENGTH);
}

static inline
void _aes_transfer_block(at86rf2xx_t *dev, uint8_t cfg, uint8_t mirror,
const aes_block_t src, aes_block_t dst)
{
/*
cfg:
value which tells the AES engine what kind of data is coming in
mirror:
must be the same value as cfg but depending on whether the bit
AT86RF2XX_AES_CTRL_MIRROR_MASK__AES_REQUEST is set, the transceiver
will process the incoming block, or not
src:
current data block of 16 bytes to be sent to the AES engine
dst:
if not NULL, dst stores the processed data block of the
block that has been sent to the AES engine most recently
*/

/* access SRAM register AES_CTRL for writing */
_aes_open_write(dev, AT86RF2XX_REG__AES_CTRL);
/* MOSI: send configuration to the AES_CTRL register */
_aes_transfer_bytes(dev, true, &cfg, NULL, sizeof(cfg));
/* MOSI: send first byte of the current block (block_i) */
_aes_transfer_bytes(dev, true, src, NULL, 1);
/* MOSI: send the last 15 bytes of block_i */
/* MISO: get the first 15 bytes of the most recently processed block (block_i-1) */
_aes_transfer_bytes(dev, true, src + 1, dst, AT86RF2XX_AES_BLOCK_SIZE - 1);
/* MOSI: send the mirrored cfg value and initiate the processing of block_i (or not) */
/* MISO: get the last byte of block_i-1 */
_aes_transfer_bytes(dev, false, &mirror,
dst ? dst + AT86RF2XX_AES_BLOCK_SIZE - 1 : NULL, 1);
}

void at86rf2xx_aes_key_read_encrypt(at86rf2xx_t *dev,
uint8_t key[AT86RF2XX_AES_KEY_LENGTH])
{
uint8_t cfg = AT86RF2XX_AES_CTRL_AES_MODE__KEY |
AT86RF2XX_AES_CTRL_AES_DIR__ENC;
at86rf2xx_spi_get_bus(dev);
_aes_save_key(dev, cfg, key);
at86rf2xx_spi_release_bus(dev);
}

void at86rf2xx_aes_key_write_encrypt(at86rf2xx_t *dev,
const uint8_t key[AT86RF2XX_AES_KEY_LENGTH])
{
uint8_t cfg = AT86RF2XX_AES_CTRL_AES_MODE__KEY |
AT86RF2XX_AES_CTRL_AES_DIR__ENC;
at86rf2xx_spi_get_bus(dev);
_aes_open_write(dev, AT86RF2XX_REG__AES_CTRL);
_aes_transfer_bytes(dev, true, &cfg, NULL, sizeof(cfg));
_aes_transfer_bytes(dev, false, key, NULL, AT86RF2XX_AES_KEY_LENGTH);
at86rf2xx_spi_release_bus(dev);
}

void at86rf2xx_aes_key_read_decrypt(at86rf2xx_t *dev,
uint8_t key[AT86RF2XX_AES_KEY_LENGTH])
{
uint8_t cfg = AT86RF2XX_AES_CTRL_AES_MODE__KEY |
AT86RF2XX_AES_CTRL_AES_DIR__DEC;
at86rf2xx_spi_get_bus(dev);
_aes_save_key(dev, cfg, key);
at86rf2xx_spi_release_bus(dev);
}

void at86rf2xx_aes_key_write_decrypt(at86rf2xx_t *dev,
const uint8_t key[AT86RF2XX_AES_KEY_LENGTH])
{
uint8_t cfg = AT86RF2XX_AES_CTRL_AES_MODE__KEY |
AT86RF2XX_AES_CTRL_AES_DIR__DEC;
at86rf2xx_spi_get_bus(dev);
_aes_open_write(dev, AT86RF2XX_REG__AES_CTRL);
_aes_transfer_bytes(dev, true, &cfg, NULL, sizeof(cfg));
_aes_transfer_bytes(dev, false, key, NULL, AT86RF2XX_AES_KEY_LENGTH);
at86rf2xx_spi_release_bus(dev);
}

void at86rf2xx_aes_ecb_encrypt(at86rf2xx_t *dev,
aes_block_t *cipher,
uint8_t key[AT86RF2XX_AES_BLOCK_SIZE],
const aes_block_t *plain,
uint8_t nblocks)
{
if (!nblocks) {
return;
}
uint8_t cfg = AT86RF2XX_AES_CTRL_AES_MODE__ECB |
AT86RF2XX_AES_CTRL_AES_DIR__ENC;
uint8_t mirror = cfg | AT86RF2XX_AES_CTRL_MIRROR_MASK__AES_REQUEST;
Copy link
Contributor

Choose a reason for hiding this comment

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

What's that mirror about?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mirror must be the same value as the cfg and is transfered to the slave to start an AES operation, if AT86RF2XX_AES_CTRL_MIRROR_MASK__AES_REQUEST is set.

at86rf2xx_spi_get_bus(dev);
_aes_transfer_block(dev, cfg, mirror, plain[0], NULL);
_aes_wait_for_result(dev);
if (key) {
cfg = AT86RF2XX_AES_CTRL_AES_MODE__KEY |
AT86RF2XX_AES_CTRL_AES_DIR__DEC;
_aes_save_key(dev, cfg, key);
}
cfg = AT86RF2XX_AES_CTRL_AES_MODE__ECB |
AT86RF2XX_AES_CTRL_AES_DIR__ENC;
for (unsigned i = 1; i < nblocks; i++) {
_aes_transfer_block(dev, cfg, mirror, plain[i],
cipher ? cipher[i - 1] : NULL);
_aes_wait_for_result(dev);
}

/* send dummy bytes to get the last block of cipher text */
mirror &= ~AT86RF2XX_AES_CTRL_MIRROR_MASK__AES_REQUEST;
_aes_transfer_block(dev, cfg, mirror, plain[0],
cipher ? cipher[nblocks - 1] : NULL);
at86rf2xx_spi_release_bus(dev);
}

void at86rf2xx_aes_ecb_decrypt(at86rf2xx_t *dev,
aes_block_t *plain,
uint8_t key[AT86RF2XX_AES_BLOCK_SIZE],
const aes_block_t *cipher,
uint8_t nblocks)
{
if (!nblocks) {
return;
}
uint8_t cfg = AT86RF2XX_AES_CTRL_AES_MODE__ECB |
AT86RF2XX_AES_CTRL_AES_DIR__DEC;
uint8_t mirror = cfg | AT86RF2XX_AES_CTRL_MIRROR_MASK__AES_REQUEST;
at86rf2xx_spi_get_bus(dev);
_aes_transfer_block(dev, cfg, mirror, cipher[0], NULL);
_aes_wait_for_result(dev);
if (key) {
cfg = AT86RF2XX_AES_CTRL_AES_MODE__KEY |
AT86RF2XX_AES_CTRL_AES_DIR__ENC;
_aes_save_key(dev, cfg, key);
}
cfg = AT86RF2XX_AES_CTRL_AES_MODE__ECB |
AT86RF2XX_AES_CTRL_AES_DIR__DEC;
for (unsigned i = 1; i < nblocks; i++) {
_aes_transfer_block(dev, cfg, mirror, cipher[i],
plain ? plain[i - 1] : NULL);
_aes_wait_for_result(dev);
}

/* send dummy bytes to get the last block of plain text */
mirror &= ~AT86RF2XX_AES_CTRL_MIRROR_MASK__AES_REQUEST;
_aes_transfer_block(dev, cfg, mirror, cipher[0],
plain ? plain[nblocks - 1] : NULL);
at86rf2xx_spi_release_bus(dev);
}

void at86rf2xx_aes_cbc_encrypt(at86rf2xx_t *dev,
aes_block_t *cipher,
uint8_t key[AT86RF2XX_AES_BLOCK_SIZE],
uint8_t iv[AT86RF2XX_AES_BLOCK_SIZE],
const aes_block_t *plain,
uint8_t nblocks)
{
if (!nblocks) {
return;
}
uint8_t cfg = AT86RF2XX_AES_CTRL_AES_MODE__ECB |
AT86RF2XX_AES_CTRL_AES_DIR__ENC;
uint8_t mirror = cfg | AT86RF2XX_AES_CTRL_MIRROR_AES_REQUEST__START;
/* The first block has to be ECB encrypted because there is no
cipher result to be XOR´ed from the last round.
Instead an "initial vector" is XOR´ed to the first block
of plain text. */
uint8_t first[AT86RF2XX_AES_BLOCK_SIZE];
for (unsigned i = 0; i < AT86RF2XX_AES_BLOCK_SIZE; i++) {
first[i] = plain[0][i] ^ iv[i];
}
at86rf2xx_spi_get_bus(dev);
_aes_transfer_block(dev, cfg, mirror, first, NULL);
_aes_wait_for_result(dev);
if (key) {
cfg = AT86RF2XX_AES_CTRL_AES_MODE__KEY |
AT86RF2XX_AES_CTRL_AES_DIR__DEC;
_aes_save_key(dev, cfg, key);
}
cfg = AT86RF2XX_AES_CTRL_AES_MODE__CBC |
AT86RF2XX_AES_CTRL_AES_DIR__ENC;
mirror = cfg | AT86RF2XX_AES_CTRL_MIRROR_AES_REQUEST__START;
for (unsigned i = 1; i < nblocks; i++) {
_aes_transfer_block(dev, cfg, mirror, plain[i],
cipher ? cipher[i - 1] : NULL);
_aes_wait_for_result(dev);
}

/* send dummy bytes to get the last block of cipher text */
uint8_t *mac = cipher ? cipher[nblocks - 1] : iv;
mirror &= ~AT86RF2XX_AES_CTRL_MIRROR_MASK__AES_REQUEST;
_aes_transfer_block(dev, cfg, mirror, plain[0], mac);
at86rf2xx_spi_release_bus(dev);
}

void at86rf2xx_aes_cbc_decrypt(at86rf2xx_t *dev,
aes_block_t *plain,
uint8_t key[AT86RF2XX_AES_BLOCK_SIZE],
uint8_t iv[AT86RF2XX_AES_BLOCK_SIZE],
const aes_block_t *cipher,
uint8_t nblocks)
{
if (!nblocks) {
return;
}
uint8_t cfg = AT86RF2XX_AES_CTRL_AES_MODE__ECB |
AT86RF2XX_AES_CTRL_AES_DIR__DEC;
uint8_t mirror = cfg | AT86RF2XX_AES_CTRL_MIRROR_AES_REQUEST__START;
at86rf2xx_spi_get_bus(dev);
_aes_transfer_block(dev, cfg, mirror, cipher[0], NULL);
_aes_wait_for_result(dev);
if (key) {
cfg = AT86RF2XX_AES_CTRL_AES_MODE__KEY |
AT86RF2XX_AES_CTRL_AES_DIR__ENC;
_aes_save_key(dev, cfg, key);
}
const uint8_t *xor = iv;
cfg = AT86RF2XX_AES_CTRL_AES_MODE__ECB |
AT86RF2XX_AES_CTRL_AES_DIR__DEC;
for (unsigned i = 1; i < nblocks; i++) {
_aes_transfer_block(dev, cfg, mirror, cipher[i],
plain ? plain[i - 1] : NULL);
_aes_wait_for_result(dev);
if (plain) {
for (unsigned j = 0; j < AT86RF2XX_AES_BLOCK_SIZE; j++) {
plain[i - 1][j] ^= xor[j];
}
xor = cipher[i - 1];
}
}

/* send dummy bytes to get the last block of plain text */
uint8_t *mac = plain ? plain[nblocks - 1] : iv;
mirror &= ~AT86RF2XX_AES_CTRL_MIRROR_MASK__AES_REQUEST;
_aes_transfer_block(dev, cfg, mirror, cipher[0], mac);
if (plain) {
for (unsigned j = 0; j < AT86RF2XX_AES_BLOCK_SIZE; j++) {
plain[nblocks - 1][j] ^= xor[j];
}
}

at86rf2xx_spi_release_bus(dev);
}
Loading