From acf42ff3dbd3f59376af2d9a0172cebbaaeff7c7 Mon Sep 17 00:00:00 2001 From: Pavol Rusnak Date: Sat, 9 Nov 2019 11:32:19 +0000 Subject: [PATCH] storage: introduce storage_get_public_nocopy + use it in trezor.config.get for public fields --- .../extmod/modtrezorconfig/modtrezorconfig.c | 31 +++++++++++++++---- storage/storage.c | 23 ++++++++++++++ storage/storage.h | 2 ++ 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/core/embed/extmod/modtrezorconfig/modtrezorconfig.c b/core/embed/extmod/modtrezorconfig/modtrezorconfig.c index c72cc0d1c..3c6942481 100644 --- a/core/embed/extmod/modtrezorconfig/modtrezorconfig.c +++ b/core/embed/extmod/modtrezorconfig/modtrezorconfig.c @@ -193,13 +193,32 @@ STATIC mp_obj_t mod_trezorconfig_get(size_t n_args, const mp_obj_t *args) { if (len == 0) { return mp_const_empty_bytes; } - vstr_t vstr; - vstr_init_len(&vstr, len); - if (sectrue != storage_get(appkey, vstr.buf, vstr.len, &len)) { - vstr_clear(&vstr); - mp_raise_msg(&mp_type_RuntimeError, "Failed to get value from storage."); + + // public field + if (app & FLAG_PUBLIC) { + // let's not copy, use the value directly + const void *ptr; + uint16_t len; + if (sectrue != storage_get_public_nocopy(appkey, &ptr, &len)) { + mp_raise_msg(&mp_type_RuntimeError, "Failed to get value from storage."); + } + // create bytes object without copying the data, let's use the const pointer + mp_obj_str_t *o = m_new_obj(mp_obj_str_t); + o->base.type = &mp_type_bytes; + o->len = len; + o->hash = 0; // will be computed later if ever needed + o->data = (const byte *)ptr; + return MP_OBJ_FROM_PTR(o); + } else { + // we need to allocate and copy, because the value needs to be decrypted + vstr_t vstr; + vstr_init_len(&vstr, len); + if (sectrue != storage_get(appkey, vstr.buf, vstr.len, &len)) { + vstr_clear(&vstr); + mp_raise_msg(&mp_type_RuntimeError, "Failed to get value from storage."); + } + return mp_obj_new_str_from_vstr(&mp_type_bytes, &vstr); } - return mp_obj_new_str_from_vstr(&mp_type_bytes, &vstr); } STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorconfig_get_obj, 2, 3, mod_trezorconfig_get); diff --git a/storage/storage.c b/storage/storage.c index 5526262b4..ffa3611a9 100644 --- a/storage/storage.c +++ b/storage/storage.c @@ -981,6 +981,29 @@ secbool storage_get(const uint16_t key, void *val_dest, const uint16_t max_len, } } +/* + * Finds the data stored under key and writes its length to len. + * Returns a pointer to the data in val_ptr. + * Works only on public fields, it does not make sense to return encrypted data. + */ +secbool storage_get_public_nocopy(const uint16_t key, const void **val_ptr, + uint16_t *len) { + const uint8_t app = key >> 8; + // APP == 0 is reserved for PIN related values + if (sectrue != initialized || app == APP_STORAGE) { + return secfalse; + } + + // If the top bit of APP is set, then the value is not encrypted and can be + // read from a locked device. + if ((app & FLAG_PUBLIC) != 0) { + return norcow_get(key, val_ptr, len); + } else { + // it doesn't make sense to return encrypted data + return secfalse; + } +} + /* * Encrypts the data at val using cached_dek as the encryption key and stores * the ciphertext under key. diff --git a/storage/storage.h b/storage/storage.h index 05138fcbe..1607c20d2 100644 --- a/storage/storage.h +++ b/storage/storage.h @@ -54,6 +54,8 @@ secbool storage_change_pin(const uint32_t oldpin, const uint32_t newpin, const uint8_t *new_ext_salt); secbool storage_get(const uint16_t key, void *val, const uint16_t max_len, uint16_t *len); +secbool storage_get_public_nocopy(const uint16_t key, const void **val_ptr, + uint16_t *len); secbool storage_set(const uint16_t key, const void *val, uint16_t len); secbool storage_delete(const uint16_t key); secbool storage_set_counter(const uint16_t key, const uint32_t count);