dfu: Remove support for the Metadata Store Proposal

This was never adopted, and firmware now indicates the license in the metainfo
file rather than the DFU file itself.
This commit is contained in:
Richard Hughes 2019-10-09 15:51:25 +01:00
parent 5d064d92e0
commit 7301d06f23
10 changed files with 0 additions and 596 deletions

View File

@ -1,172 +0,0 @@
DFU File Format - Metadata Store Proposal
=========================================
Introduction
------------
The DFU specification version 1.1 defines some target-specific data that can
optionally be included in the DFU file to ease firmware deployment.
These include items such as the runtime vendor, product and device release,
but nothing else and with no provision for extra metadata.
The DFU file specification does not specify any additional data structures
allowing vendors to include additional metadata, although does provide the
chance to include future additional data fields in the header in a backwards and
forwards compatible way by increasing the `header_len` value.
All software reading and writing DFU-format files should already be reading the
footer length from the file, rather than assuming a fixed footer length of 0x10
bytes.
This ensures that only the raw device firmware is sent to the device and not any
additional data fields added in future versions of the DFU specification.
There are valid reasons why we would want to add additional metadata into the
distributed DFU file. Reasons are listed as follows:
* Legal compliance, to tag a file with copyright and licensing information
* Business-specific metadata, for instance the SHA-1 git commit for the source
* Cryptographic information such as a SHA-256 hash or a detached GPG signature
Although the original authors of the specification allowed for future additions
to the specification, they only allowed us to extend the footer by 239 bytes as
the `header_len` value is specified as just one byte, and 16 bytes are already
specified by the specification.
This would explain why some vendors are using vendor-specific file prefix data
segments, for instance the DfuSe prefix specification from ST.
This specification is not aiming to expand or standardize the various
incompatible vendor-specific prefix specifications, but tries to squeeze the
additional metadata into the existing DFU footer space which is compatible with
all existing DFU-compliant software.
Specification
-------------
An additional structure would be present after the binary firmware data, and
notionally contained within the DFU footer itself, although specified as a
seporate object.
The representation in memory and on disk would be as follows:
uint16 signature='MD'
uint8 number_of_keys
uint8 key(n)_length
... key(n) (no NUL)
uint8 value(n)_length
... value(n) (no NUL)
<existing DFU footer>
If there are no metadata keys being set, it is expected that the metadata table
signature is not be written to the file, and that the footer should be again
0x10 bytes in length.
The signature of `MD` should also be checked before attempting to parse the
metadata store structure to ensure other vendor-specific extensions are not
already in use.
The key and value fields should be parsed as UTF-8, although in the pursuit of
space minimisation ASCII values are preferred where possible.
Example
-------
The following table shows an example firmware file with the payload 'DATA' set
with vendor 0x1234, product 0xABCD and no metadata table.
neg.
offset description byte
----------------------------
13 firmware'D' 0x44
12 firmware'A' 0x41
11 firmware'T' 0x54
10 firmware'A' 0x44
0f bcdDevice 0xFF
0e bcdDevice 0xFF
0d idProduct 0xCD
0c idProduct 0xAB
0b idVendor 0x34
0a idVendor 0x12
09 bcdDFU 0x00
08 bcdDFU 0x01
07 ucDfuSig'U' 0x55
06 ucDfuSig'F' 0x46
05 ucDfuSig'D' 0x44
04 bLength 0x10
03 dwCRC 0x52
02 dwCRC 0xB4
01 dwCRC 0xE5
00 dwCRC 0xCE
The following table shows a second firmware file with the same payload but
with the addition of a metadata table with a single metadata pair of `test=val`:
neg.
offset description byte
----------------------------
1f firmware'D' 0x44
1e firmware'A' 0x41
1d firmware'T' 0x54
1c firmware'A' 0x44
1b ucMdSig'M' 0x4D
1a ucMdSig'D' 0x44
19 bMdLength 0x01
18 bKeyLen 0x04
17 KeyData't' 0x74
16 KeyData'e' 0x65
15 KeyData's' 0x73
14 KeyData't' 0x74
13 bValueLen 0x03
12 ValueData'v' 0x76
11 ValueData'a' 0x61
10 ValueData'l' 0x6c
0f bcdDevice 0xFF
0e bcdDevice 0xFF
0d idProduct 0xCD
0c idProduct 0xAB
0b idVendor 0x34
0a idVendor 0x12
09 bcdDFU 0x00
08 bcdDFU 0x01
07 ucDfuSig'U' 0x55
06 ucDfuSig'F' 0x46
05 ucDfuSig'D' 0x44
04 bLength 0x1C
03 dwCRC 0x1B
02 dwCRC 0x25
01 dwCRC 0x6D
00 dwCRC 0xF5
Conclusions
-----------
The metadata store proposal allows us to store a small amount of metadata
inside the DFU file footer.
If the original specification had included just one more byte for the footer
length (for instance a `uint16`, rather than a `uint8`) type then I would have
proposed a key type allowing integers, IEEE floating point, and strings, and
also made the number of keys and the length of keys much larger.
Working with what we've been given, we can support a useful easy-to-parse
extension that allows us to solve some of the real-world problems vendors are
facing when trying to distribute firmware files for devices that support
in-the-field device firmware upgrading.
Several deliberate compomises have been made to this proposal due to the
restricted space available:
* The metadata table signature is just two bytes
* The number of keys is limited to just 59 pairs
* Keys are limited to just 233 chars maximum
* Values are limited to just 233 chars maximum
* Types are limited to just strings (which can includes empty strings)
* Strings are written without a `NUL` trailing byte
* The metadata table uses variable offsets rather than fixed sizes
The key-value length in particular leads to some other best practices:
* A value for the 'License' key should be in SPDX format, NOT the full licence
* A value for the 'Copyright' key should just be the company name
The author is not already aware of any vendors using this additional data area,
but would be willing to work with any vendors who have implemented a similar
proprietary extension already or are planning to do so.

View File

@ -127,9 +127,6 @@ typedef enum {
DFU_VERSION_LAST
} DfuVersion;
#define DFU_METADATA_KEY_LICENSE "License"
#define DFU_METADATA_KEY_COPYRIGHT "Copyright"
const gchar *dfu_state_to_string (DfuState state);
const gchar *dfu_status_to_string (DfuStatus status);
const gchar *dfu_version_to_string (DfuVersion version);

View File

@ -35,7 +35,6 @@
static void dfu_firmware_finalize (GObject *object);
typedef struct {
GHashTable *metadata;
GPtrArray *images;
guint16 vid;
guint16 pid;
@ -61,7 +60,6 @@ dfu_firmware_init (DfuFirmware *firmware)
priv->pid = 0xffff;
priv->release = 0xffff;
priv->images = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);
priv->metadata = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
}
static void
@ -71,7 +69,6 @@ dfu_firmware_finalize (GObject *object)
DfuFirmwarePrivate *priv = GET_PRIVATE (firmware);
g_ptr_array_unref (priv->images);
g_hash_table_destroy (priv->metadata);
G_OBJECT_CLASS (dfu_firmware_parent_class)->finalize (object);
}
@ -417,68 +414,6 @@ dfu_firmware_parse_file (DfuFirmware *firmware, GFile *file,
return dfu_firmware_parse_data (firmware, bytes, flags, error);
}
/**
* dfu_firmware_get_metadata:
* @firmware: a #DfuFirmware
* @key: metadata string key
*
* Gets metadata from the store with a specific key.
*
* Return value: the metadata value, or %NULL for unset
**/
const gchar *
dfu_firmware_get_metadata (DfuFirmware *firmware, const gchar *key)
{
DfuFirmwarePrivate *priv = GET_PRIVATE (firmware);
return g_hash_table_lookup (priv->metadata, key);
}
/**
* dfu_firmware_get_metadata_table:
* @firmware: a #DfuFirmware
*
* Gets all metadata from the store.
*
* Return value: (transfer none): the metadata hash table
**/
GHashTable *
dfu_firmware_get_metadata_table (DfuFirmware *firmware)
{
DfuFirmwarePrivate *priv = GET_PRIVATE (firmware);
return priv->metadata;
}
/**
* dfu_firmware_set_metadata:
* @firmware: a #DfuFirmware
* @key: metadata string key
* @value: metadata string value
*
* Sets a metadata value with a specific key.
**/
void
dfu_firmware_set_metadata (DfuFirmware *firmware, const gchar *key, const gchar *value)
{
DfuFirmwarePrivate *priv = GET_PRIVATE (firmware);
g_debug ("adding metadata %s=%s", key, value);
g_hash_table_insert (priv->metadata, g_strdup (key), g_strdup (value));
}
/**
* dfu_firmware_remove_metadata:
* @firmware: a #DfuFirmware
* @key: metadata string key
*
* Removes a metadata item from the store
**/
void
dfu_firmware_remove_metadata (DfuFirmware *firmware, const gchar *key)
{
DfuFirmwarePrivate *priv = GET_PRIVATE (firmware);
g_debug ("removing metadata %s", key);
g_hash_table_remove (priv->metadata, key);
}
static gboolean
dfu_firmware_check_acceptable_for_format (DfuFirmware *firmware, GError **error)
{
@ -602,7 +537,6 @@ dfu_firmware_to_string (DfuFirmware *firmware)
DfuImage *image;
GString *str;
g_autofree gchar *release_str = NULL;
g_autoptr(GList) keys = NULL;
g_return_val_if_fail (DFU_IS_FIRMWARE (firmware), NULL);
@ -617,15 +551,6 @@ dfu_firmware_to_string (DfuFirmware *firmware)
dfu_firmware_format_to_string (priv->format),
priv->format);
/* print metadata */
keys = g_hash_table_get_keys (priv->metadata);
for (GList *l = keys; l != NULL; l = l->next) {
const gchar *key = l->data;
const gchar *value;
value = g_hash_table_lookup (priv->metadata, key);
g_string_append_printf (str, "metadata: %s=%s\n", key, value);
}
/* print images */
for (guint i = 0; i < priv->images->len; i++) {
g_autofree gchar *tmp = NULL;

View File

@ -25,7 +25,6 @@ struct _DfuFirmwareClass
* @DFU_FIRMWARE_PARSE_FLAG_NONE: No flags set
* @DFU_FIRMWARE_PARSE_FLAG_NO_CRC_TEST: Do not verify the CRC
* @DFU_FIRMWARE_PARSE_FLAG_NO_VERSION_TEST: Do not verify the DFU version
* @DFU_FIRMWARE_PARSE_FLAG_NO_METADATA: Do not read the metadata table
*
* The optional flags used for parsing.
**/
@ -33,7 +32,6 @@ typedef enum {
DFU_FIRMWARE_PARSE_FLAG_NONE = 0,
DFU_FIRMWARE_PARSE_FLAG_NO_CRC_TEST = (1 << 0),
DFU_FIRMWARE_PARSE_FLAG_NO_VERSION_TEST = (1 << 1),
DFU_FIRMWARE_PARSE_FLAG_NO_METADATA = (1 << 2),
/*< private >*/
DFU_FIRMWARE_PARSE_FLAG_LAST
} DfuFirmwareParseFlags;
@ -99,12 +97,3 @@ gboolean dfu_firmware_write_file (DfuFirmware *firmware,
GFile *file,
GError **error);
gchar *dfu_firmware_to_string (DfuFirmware *firmware);
GHashTable *dfu_firmware_get_metadata_table(DfuFirmware *firmware);
const gchar *dfu_firmware_get_metadata (DfuFirmware *firmware,
const gchar *key);
void dfu_firmware_set_metadata (DfuFirmware *firmware,
const gchar *key,
const gchar *value);
void dfu_firmware_remove_metadata (DfuFirmware *firmware,
const gchar *key);

View File

@ -10,7 +10,6 @@
#include "dfu-element.h"
#include "dfu-format-dfu.h"
#include "dfu-format-metadata.h"
#include "dfu-format-dfuse.h"
#include "dfu-format-raw.h"
#include "dfu-image.h"
@ -205,14 +204,6 @@ dfu_firmware_from_dfu (DfuFirmware *firmware,
return FALSE;
}
/* parse the optional metadata segment */
if ((flags & DFU_FIRMWARE_PARSE_FLAG_NO_METADATA) == 0) {
gsize offset = len - ftr->len;
g_autoptr(GBytes) md = g_bytes_new (&data[offset], ftr->len);
if (!dfu_firmware_from_metadata (firmware, md, flags, error))
return FALSE;
}
/* parse DfuSe prefix */
contents = g_bytes_new_from_bytes (bytes, 0, len - ftr->len);
if (dfu_firmware_get_format (firmware) == DFU_FIRMWARE_FORMAT_DFUSE)
@ -237,27 +228,16 @@ dfu_firmware_add_footer (DfuFirmware *firmware, GBytes *contents, GError **error
{
DfuFirmwareFooter *ftr;
const guint8 *data_bin;
const guint8 *data_md;
gsize length_bin = 0;
gsize length_md = 0;
guint32 crc_new;
guint8 *buf;
g_autoptr(GBytes) metadata_table = NULL;
/* get any file metadata */
metadata_table = dfu_firmware_to_metadata (firmware, error);
if (metadata_table == NULL)
return NULL;
data_md = g_bytes_get_data (metadata_table, &length_md);
/* add the raw firmware data */
data_bin = g_bytes_get_data (contents, &length_bin);
buf = g_malloc0 (length_bin + length_md + 0x10);
memcpy (buf + 0, data_bin, length_bin);
/* add the metadata table */
memcpy (buf + length_bin, data_md, length_md);
/* set up LE footer */
ftr = (DfuFirmwareFooter *) (buf + length_bin + length_md);
ftr->release = GUINT16_TO_LE (dfu_firmware_get_release (firmware));

View File

@ -1,210 +0,0 @@
/*
* Copyright (C) 2015-2016 Richard Hughes <richard@hughsie.com>
*
* SPDX-License-Identifier: LGPL-2.1+
*/
#include "config.h"
#include <string.h>
#include "fu-common.h"
#include "dfu-element.h"
#include "dfu-format-metadata.h"
#include "dfu-image.h"
#include "fwupd-error.h"
/**
* dfu_firmware_from_metadata: (skip)
* @firmware: a #DfuFirmware
* @bytes: data to parse
* @flags: some #DfuFirmwareParseFlags
* @error: a #GError, or %NULL
*
* Unpacks into a firmware object from metadata data.
*
* The representation in memory is as follows:
*
* uint16 signature='MD'
* uint8 number_of_keys
* uint8 number_of_keys
* uint8 key(n)_length
* ... key(n) (no NUL)
* uint8 value(n)_length
* ... value(n) (no NUL)
* <existing DFU footer>
* Returns: %TRUE for success
**/
gboolean
dfu_firmware_from_metadata (DfuFirmware *firmware,
GBytes *bytes,
DfuFirmwareParseFlags flags,
GError **error)
{
const guint8 *data;
gsize data_length;
guint idx = 2;
guint kvlen;
guint number_keys;
/* not big enough */
data = g_bytes_get_data (bytes, &data_length);
if (data_length <= 0x10)
return TRUE;
/* signature invalid */
if (memcmp (data, "MD", 2) != 0)
return TRUE;
/* parse key=value store */
number_keys = data[idx++];
for (guint i = 0; i < number_keys; i++) {
g_autofree gchar *key = NULL;
g_autofree gchar *value = NULL;
/* parse key */
kvlen = data[idx++];
if (kvlen > 233) {
g_set_error (error,
FWUPD_ERROR,
FWUPD_ERROR_INTERNAL,
"metadata table corrupt, key=%u",
kvlen);
return FALSE;
}
if (idx + kvlen + 0x10 > data_length) {
g_set_error (error,
FWUPD_ERROR,
FWUPD_ERROR_INTERNAL,
"metadata table corrupt, k-kvlen=%u",
kvlen);
return FALSE;
}
key = g_strndup ((const gchar *) data + idx, kvlen);
idx += kvlen;
/* parse value */
kvlen = data[idx++];
if (kvlen > 233) {
g_set_error (error,
FWUPD_ERROR,
FWUPD_ERROR_INTERNAL,
"metadata table corrupt, value=%u",
kvlen);
return FALSE;
}
if (idx + kvlen + 0x10 > data_length) {
g_set_error (error,
FWUPD_ERROR,
FWUPD_ERROR_INTERNAL,
"metadata table corrupt, v-kvlen=%u",
kvlen);
return FALSE;
}
value = g_strndup ((const gchar *) data + idx, kvlen);
idx += kvlen;
dfu_firmware_set_metadata (firmware, key, value);
}
return TRUE;
}
/**
* dfu_firmware_to_metadata: (skip)
* @firmware: a #DfuFirmware
* @error: a #GError, or %NULL
*
* Packs metadata firmware
*
* Returns: (transfer full): the packed data
**/
GBytes *
dfu_firmware_to_metadata (DfuFirmware *firmware, GError **error)
{
GHashTable *metadata;
guint8 mdbuf[239];
guint idx = 0;
guint number_keys;
g_autoptr(GList) keys = NULL;
/* no metadata */
metadata = dfu_firmware_get_metadata_table (firmware);
if (g_hash_table_size (metadata) == 0)
return g_bytes_new (NULL, 0);
/* check the number of keys */
keys = g_hash_table_get_keys (metadata);
number_keys = g_list_length (keys);
if (number_keys > 59) {
g_set_error (error,
FWUPD_ERROR,
FWUPD_ERROR_NOT_SUPPORTED,
"too many metadata keys (%u)",
number_keys);
return NULL;
}
/* write the signature */
mdbuf[idx++] = 'M';
mdbuf[idx++] = 'D';
mdbuf[idx++] = (guint8) number_keys;
for (GList *l = keys; l != NULL; l = l->next) {
const gchar *key;
const gchar *value;
guint key_len;
guint value_len;
/* check key and value length */
key = l->data;
key_len = (guint) strlen (key);
if (key_len > 233) {
g_set_error (error,
FWUPD_ERROR,
FWUPD_ERROR_NOT_SUPPORTED,
"metadata key too long: %s",
key);
return NULL;
}
value = g_hash_table_lookup (metadata, key);
value_len = (guint) strlen (value);
if (value_len > 233) {
g_set_error (error,
FWUPD_ERROR,
FWUPD_ERROR_NOT_SUPPORTED,
"value too long: %s",
value);
return NULL;
}
/* do we still have space? */
if (idx + key_len + value_len + 2 > sizeof(mdbuf)) {
g_set_error (error,
FWUPD_ERROR,
FWUPD_ERROR_NOT_SUPPORTED,
"not enough space in metadata table, "
"already used %u bytes", idx);
return NULL;
}
/* write the key */
mdbuf[idx++] = (guint8) key_len;
if (!fu_memcpy_safe (mdbuf, sizeof(mdbuf), idx, /* dst */
(const guint8 *) key, key_len, 0x0, /* src */
key_len, error))
return NULL;
idx += key_len;
/* write the value */
mdbuf[idx++] = (guint8) value_len;
if (!fu_memcpy_safe (mdbuf, sizeof(mdbuf), idx, /* dst */
(const guint8 *) value, value_len, 0x0, /* src */
value_len, error))
return NULL;
idx += value_len;
}
g_debug ("metadata table was %u/%" G_GSIZE_FORMAT " bytes",
idx, sizeof(mdbuf));
return g_bytes_new (mdbuf, idx);
}

View File

@ -1,19 +0,0 @@
/*
* Copyright (C) 2015-2016 Richard Hughes <richard@hughsie.com>
*
* SPDX-License-Identifier: LGPL-2.1+
*/
#pragma once
#include <glib-object.h>
#include <gio/gio.h>
#include "dfu-firmware.h"
GBytes *dfu_firmware_to_metadata (DfuFirmware *firmware,
GError **error);
gboolean dfu_firmware_from_metadata (DfuFirmware *firmware,
GBytes *bytes,
DfuFirmwareParseFlags flags,
GError **error);

View File

@ -224,43 +224,6 @@ dfu_firmware_dfuse_func (void)
g_unsetenv ("DFU_SELF_TEST_IMAGE_MEMCPY_NAME");
}
static void
dfu_firmware_metadata_func (void)
{
gboolean ret;
g_autofree gchar *filename = NULL;
g_autoptr(DfuFirmware) firmware = NULL;
g_autoptr(GBytes) roundtrip_orig = NULL;
g_autoptr(GBytes) roundtrip = NULL;
g_autoptr(GError) error = NULL;
g_autoptr(GFile) file = NULL;
/* load a DFU firmware with a metadata table */
filename = dfu_test_get_filename ("metadata.dfu");
g_assert (filename != NULL);
file = g_file_new_for_path (filename);
firmware = dfu_firmware_new ();
ret = dfu_firmware_parse_file (firmware, file,
DFU_FIRMWARE_PARSE_FLAG_NONE,
&error);
g_assert_no_error (error);
g_assert (ret);
g_assert_cmpint (dfu_firmware_get_size (firmware), ==, 6);
g_assert_cmpstr (dfu_firmware_get_metadata (firmware, "key"), ==, "value");
g_assert_cmpstr (dfu_firmware_get_metadata (firmware, "???"), ==, NULL);
/* can we roundtrip without losing data */
roundtrip_orig = dfu_self_test_get_bytes_for_file (file, &error);
g_assert_no_error (error);
g_assert (roundtrip_orig != NULL);
roundtrip = dfu_firmware_write_data (firmware, &error);
g_assert_no_error (error);
g_assert (roundtrip != NULL);
ret = fu_common_bytes_compare (roundtrip, roundtrip_orig, &error);
g_assert_no_error (error);
g_assert_true (ret);
}
static gchar *
dfu_target_sectors_to_string (DfuTarget *target)
{
@ -533,7 +496,6 @@ main (int argc, char **argv)
g_test_add_func ("/dfu/firmware{raw}", dfu_firmware_raw_func);
g_test_add_func ("/dfu/firmware{dfu}", dfu_firmware_dfu_func);
g_test_add_func ("/dfu/firmware{dfuse}", dfu_firmware_dfuse_func);
g_test_add_func ("/dfu/firmware{metadata}", dfu_firmware_metadata_func);
return g_test_run ();
}

View File

@ -823,47 +823,6 @@ dfu_tool_set_address (DfuToolPrivate *priv, gchar **values, GError **error)
return dfu_firmware_write_file (firmware, file, error);
}
static gboolean
dfu_tool_set_metadata (DfuToolPrivate *priv, gchar **values, GError **error)
{
g_autoptr(DfuFirmware) firmware = NULL;
g_autoptr(GFile) file = NULL;
/* check args */
if (g_strv_length (values) < 3) {
g_set_error_literal (error,
FWUPD_ERROR,
FWUPD_ERROR_INTERNAL,
"Invalid arguments, expected FILE KEY VALUE"
" -- e.g. `firmware.dfu Licence GPL-2.0+");
return FALSE;
}
/* open */
file = g_file_new_for_path (values[0]);
firmware = dfu_firmware_new ();
if (!dfu_firmware_parse_file (firmware, file,
DFU_FIRMWARE_PARSE_FLAG_NONE,
error)) {
return FALSE;
}
/* doesn't make sense for non-DFU */
if (dfu_firmware_get_format (firmware) == DFU_FIRMWARE_FORMAT_RAW) {
g_set_error (error,
FWUPD_ERROR,
FWUPD_ERROR_INTERNAL,
"Only possible on DFU/DfuSe images, try convert");
return FALSE;
}
/* set metadata */
dfu_firmware_set_metadata (firmware, values[1], values[2]);
/* write out new file */
return dfu_firmware_write_file (firmware, file, error);
}
static gboolean
dfu_tool_set_alt_setting (DfuToolPrivate *priv, gchar **values, GError **error)
{
@ -2117,12 +2076,6 @@ main (int argc, char *argv[])
/* TRANSLATORS: command description */
_("Watch DFU devices being hotplugged"),
dfu_tool_watch);
dfu_tool_add (priv->cmd_array,
"set-metadata",
"FILE KEY VALUE",
/* TRANSLATORS: command description */
_("Sets metadata on a firmware file"),
dfu_tool_set_metadata);
dfu_tool_add (priv->cmd_array,
"replace-data",
NULL,

View File

@ -14,7 +14,6 @@ dfu = static_library(
'dfu-firmware.c',
'dfu-format-dfu.c',
'dfu-format-dfuse.c',
'dfu-format-metadata.c',
'dfu-format-raw.c',
'dfu-image.c',
'dfu-patch.c',