Skip to content
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
12 changes: 12 additions & 0 deletions Makefile.include
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ RIOTMAKE ?= $(RIOTBASE)/makefiles
RIOTPKG ?= $(RIOTBASE)/pkg
RIOTPROJECT ?= $(shell git rev-parse --show-toplevel 2>/dev/null || pwd)
GITCACHE ?= $(RIOTBASE)/dist/tools/git/git-cache
FW_METADATA ?= $(RIOTBASE)/dist/tools/firmware_metadata
APPDIR ?= $(CURDIR)
BINDIRBASE ?= $(APPDIR)/bin
BINDIR ?= $(BINDIRBASE)/$(BOARD)
Expand Down Expand Up @@ -41,6 +42,7 @@ override RIOTMAKE := $(abspath $(RIOTMAKE))
override RIOTPKG := $(abspath $(RIOTPKG))
override RIOTPROJECT := $(abspath $(RIOTPROJECT))
override GITCACHE := $(abspath $(GITCACHE))
override FW_METADATA := $(abspath $(FW_METADATA))
override APPDIR := $(abspath $(APPDIR))
override BINDIRBASE := $(abspath $(BINDIRBASE))
override BINDIR := $(abspath $(BINDIR))
Expand Down Expand Up @@ -306,6 +308,16 @@ endif
endif
endif # BUILD_IN_DOCKER

firmware-metadata: all generate-metadata
BINFILE ?= $(ELFFILE:.elf=.bin)
$(Q)$(OBJCOPY) -O binary $(ELFFILE) $(BINFILE)
$(Q)$(FW_METADATA)/bin/generate-metadata $(BINFILE) $(VERSION) $(APPID) $(BINDIR)/firmware-metadata.bin

generate-metadata: $(FW_METADATA)/bin/generate-metadata

$(FW_METADATA)/bin/generate-metadata:
$(Q)env -i make -C $(FW_METADATA)

..compiler-check:
@command -v $(CC) >/dev/null 2>&1 || \
{ $(COLOR_ECHO) \
Expand Down
19 changes: 19 additions & 0 deletions dist/tools/firmware_metadata/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
RIOTBASE := ../../..

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this line should be:

RIOTBASE ?= $(CURDIR)/../../..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think since dist/tools/ is a path that will always exists in any clone of RIOT, we don't really need to define $(CURDIR), and so the relative path will always be the same.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok you are right.

RIOT_INCLUDE = $(RIOTBASE)/sys/include
SHA256_DIR := $(RIOTBASE)/sys/hashes
SHA256_INCLUDE := $(RIOT_INCLUDE)/hashes
METADATA_SRC := generate-metadata.c $(SHA256_DIR)/sha256.c
METADATA_HDR := $(RIOT_INCLUDE)/hashes/sha256.h

CFLAGS += -g -O3 -Wall -Wextra -pedantic -std=c99

all: bin/generate-metadata

bin/:
mkdir -p bin

bin/generate-metadata: $(METADATA_HDR) $(METADATA_SRC) | bin/
$(CC) $(CFLAGS) -I$(RIOT_INCLUDE) $(METADATA_SRC) -o $@

clean:
rm -rf bin/generate-metadata
36 changes: 36 additions & 0 deletions dist/tools/firmware_metadata/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Metadata generator for firmware verification
This program generates a binary file containing a metadata structure as

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe add an empty line after title.

Quick question: does the file only contain the metadata or the metadata + the initial firmware ? If it's the second case, it should be more explicit. Otherwise, one can easily understand that the generated binary file only contain the struct.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe add an empty line after title.

What this will change?

uick question: does the file only contain the metadata or the metadata + the initial firmware ? If it's the second case, it should be more explicit. Otherwise, one can easily understand that the generated binary file only contain the struct.

It is the first case.

follows:

```c
typedef struct firmware_metadata {
uint8_t hash[SHA256_DIGEST_LENGTH]; /**< SHA256 Hash of firmware image */
uint8_t shash[SIGN_LEN]; /**< Signed SHA256 */
uint16_t version; /**< Integer representing firmware version */
uint32_t size; /**< Size of firmware image */
uint32_t appid; /**< Integer representing the application ID */
} firmware_metadata_t;
```

This structure is filled with the data obtained from the firmware.

## Usage
To use it, call `generate-metadata` with the following arguments:

```console
./generate-metadata <firmware.bin> <version> <appid> <output-path>
```

Where:

_\<firmware.bin\>\:_ The firmware in binary format

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe use <firmware.bin> instead so it is better readable in raw text?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't get exactly what you mean...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think @kaspar030 means "use ` ` around <firmware.bin>". +1 on my side


_\<version\>\:_ The firmware version in 16-bit HEX format

_\<appid\>_\: ID for the application in 32-bit HEX format

_\<output-path/filename.bin\>_\: The path and name of the metadata binary file

If the operation succeeds, the results are printed out and a binary called
"firmware-metadata.bin" is written, if no _output-path_ option is specified.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is confusing as first sight.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What's the confusion?

Otherwise, the file is written to the given path with the given name.
153 changes: 153 additions & 0 deletions dist/tools/firmware_metadata/generate-metadata.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
/*
* Copyright (c) 2016, Mark Solters <msolters@gmail.com>.
* 2016, Inria
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
* 3. Neither the name of the Institute nor the names of its contributors
* may be used to endorse or promote products derived from this software
* without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE INSTITUTE AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE INSTITUTE OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
*/

/**
* @ingroup FW
* @file
* @brief Meta-data generation for FW images
*
* @author Mark Solters <msolters@gmail.com>
* @author Francisco Acosta <francisco.acosta@inria.fr>
*/

#include <stdio.h>
#include <stdint.h>
#include <string.h>
#include <sys/resource.h>

#include "hashes/sha256.h"

#define FW_METADATA_BIN "firmware-metadata.bin"

typedef struct firmware_metadata {
uint8_t hash[SHA256_DIGEST_LENGTH]; /**< SHA256 Hash of firmware image */
uint8_t shash[SHA256_DIGEST_LENGTH]; /**< Signed SHA256 */
uint16_t version; /**< Integer representing firmware version */
uint32_t size; /**< Size of firmware image */
uint32_t appid; /**< Integer representing the application ID */
} firmware_metadata_t;

/* Input firmware .bin file */
FILE *firmware_bin;

/* Output metadata .bin file */
FILE *metadata_bin;

uint32_t firmware_size = 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could be declared static

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does it matter for a native C program? In that case all the variables should be declared static in this program.


int main(int argc, char *argv[])
{
firmware_metadata_t metadata;
sha256_context_t firmware_sha256;
uint8_t output_buffer[sizeof(firmware_metadata_t)];
int bytes_read = 0;
uint8_t firmware_buffer[1024];
char firmware_metadata_path[128];

if (argc < 4) {
puts("Usage: generate-metadata <BINFILE> <VERSION> <APPID> [output path]");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/output path/output filename/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I still think output path is ok...

return -1;
}

if (argv[4]) {
strcpy(firmware_metadata_path, argv[4]);
}
else {
strcpy(firmware_metadata_path, "firmware-metadata.bin");
}

/* Open the firmware .bin file */
if (!(firmware_bin = fopen(argv[1], "r"))) {
printf("Error: No binary file found!\n");
return -1;
}

sha256_init(&firmware_sha256);

while ((bytes_read = fread(firmware_buffer, 1, sizeof(firmware_buffer), firmware_bin))) {
sha256_update(&firmware_sha256, firmware_buffer, bytes_read);
firmware_size += bytes_read;
}
sha256_final(&firmware_sha256, metadata.hash);

printf("Firmware bytes read: %u\n", firmware_size);

/* Close the .bin file. */
fclose(firmware_bin);

/*
* Fill with 0 the signed hash, as it's not yet implemented
*/
for (unsigned long i = 0; i < sizeof(metadata.shash); i++) {
metadata.shash[i] = 0;
}

/* Generate FW image metadata */
metadata.size = firmware_size;
sscanf(argv[2], "%xu", (unsigned int *)&(metadata.version));
sscanf(argv[3], "%xu", &(metadata.appid));
memcpy(output_buffer, (uint8_t*)&metadata, sizeof(firmware_metadata_t));

printf("Firmware Size: %d\n", metadata.size);
printf("Firmware Version: %#x\n", metadata.version);
printf("Firmware APPID: %#x\n", metadata.appid);
printf("Firmware HASH: ");
for (unsigned long i = 0; i < sizeof(metadata.hash); i++) {
printf("%02x ", metadata.hash[i]);
}
printf("\n");
printf("Firmware signed HASH: ");
for (unsigned long i = 0; i < sizeof(metadata.shash); i++) {
printf("%02x ", metadata.shash[i]);
}
printf("\n");

/* Open the output firmware .bin file */
metadata_bin = fopen(firmware_metadata_path, "w");

/* Write the metadata */
printf("Metadata size: %lu\n", sizeof(firmware_metadata_t));
fwrite(output_buffer, sizeof(output_buffer), 1, metadata_bin);

/* 0xff spacing until firmware binary starts */
uint8_t blank_buffer[256 - sizeof(firmware_metadata_t)];

for (unsigned long b = 0; b < sizeof(blank_buffer); b++) {
blank_buffer[b] = 0xff;
}

fwrite(blank_buffer, sizeof(blank_buffer), 1, metadata_bin);

/* Close the metadata file */
fclose(metadata_bin);

return 0;
}