| Message ID | 20190401143001.32679-1-s.hauer@pengutronix.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Richard Weinberger |
| Headers | show |
| Series | ubifs: support offline signed images | expand |
----- Ursprüngliche Mail ----- > Von: "Sascha Hauer" <s.hauer@pengutronix.de> > An: "linux-mtd" <linux-mtd@lists.infradead.org> > CC: "richard" <richard@nod.at>, kernel@pengutronix.de, "Sascha Hauer" <s.hauer@pengutronix.de> > Gesendet: Montag, 1. April 2019 16:30:01 > Betreff: [PATCH] ubifs: support offline signed images > HMACs can only be generated on the system the UBIFS image is running on. Because at mkfs.ubifs time you don't have the key which might be hardware specific, right? > To support offline signed images we add a PKCS#7 signature to the UBIFS > image which can be created by mkfs.ubifs. > > Both the master node and the superblock need to be authenticated, during > normal runtime both are protected with HMACs. For offline signature > support however only a single signature is desired. We add a signature > covering the superblock node directly behind it. To protect the master > node a hash of the master node is added to the superblock which is used > when the master node doesn't contain a HMAC. > > Transition to a read/write filesystem is also supported. During > transition first the master node is rewritten with a HMAC (implicitly, > it is written anyway as the FS is marked dirty). Afterwards the > superblock is rewritten with a HMAC. Once after the image has been > mounted read/write it is HMAC only, the signature is no longer required > or even present on the filesystem. > > In an offline signed image the master node is authenticated by the > superblock. In a transition to r/w we have to make sure that the master > node is rewritten before the superblock node. In this case the master > node gets a HMAC and its authenticity no longer depends on the > superblock node. There are some cases in which the current code first > writes the superblock node though, so with this patch writing of the > superblock node is delayed until the master node is written. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > fs/ubifs/Kconfig | 1 + > fs/ubifs/auth.c | 81 ++++++++++++++++++++++++++++++++++++++++++ > fs/ubifs/master.c | 54 ++++++++++++++++++++++++---- > fs/ubifs/sb.c | 49 ++++++++++++------------- > fs/ubifs/super.c | 41 ++++++++++++++++----- > fs/ubifs/ubifs-media.h | 21 ++++++++++- > fs/ubifs/ubifs.h | 4 +++ > 7 files changed, 211 insertions(+), 40 deletions(-) > > diff --git a/fs/ubifs/Kconfig b/fs/ubifs/Kconfig > index 9da2f135121b..3893668d5dae 100644 > --- a/fs/ubifs/Kconfig > +++ b/fs/ubifs/Kconfig > @@ -78,6 +78,7 @@ config UBIFS_FS_AUTHENTICATION > bool "UBIFS authentication support" > depends on KEYS > select CRYPTO_HMAC > + select SYSTEM_DATA_VERIFICATION > help > Enable authentication support for UBIFS. This feature offers protection > against offline changes for both data and metadata of the filesystem. > diff --git a/fs/ubifs/auth.c b/fs/ubifs/auth.c > index 5bf5fd08879e..cff5820ee54b 100644 > --- a/fs/ubifs/auth.c > +++ b/fs/ubifs/auth.c > @@ -10,10 +10,12 @@ > */ > > #include <linux/crypto.h> > +#include <linux/verification.h> > #include <crypto/hash.h> > #include <crypto/sha.h> > #include <crypto/algapi.h> > #include <keys/user-type.h> > +#include <keys/asymmetric-type.h> > > #include "ubifs.h" > > @@ -217,6 +219,66 @@ int __ubifs_node_check_hash(const struct ubifs_info *c, > const void *node, > return 0; > } > > +/** > + * ubifs_sb_verify_signature - verify the signature of a superblock > + * @c: UBIFS file-system description object > + * @sup: The superblock node > + * > + * To support offline signed images the superblock can be signed with a > + * PKCS#7 signature. The signature is placed directly behind the superblock > + * node in an ubifs_sig_node. > + * > + * Returns 0 when the signature can be successfully verified or a negative > + * error code if not. > + */ > +int ubifs_sb_verify_signature(struct ubifs_info *c, > + const struct ubifs_sb_node *sup) > +{ > + int err; > + struct ubifs_scan_leb *sleb; > + struct ubifs_scan_node *snod; > + const struct ubifs_sig_node *signode; > + > + err = ubifs_leb_read(c, UBIFS_SB_LNUM, c->sbuf, 0, c->leb_size, 1); This line looks wrong here, leftover from old code? :) > + sleb = ubifs_scan(c, UBIFS_SB_LNUM, UBIFS_SB_NODE_SZ, c->sbuf, 0); > + if (IS_ERR(sleb)) { > + err = PTR_ERR(sleb); > + return err; > + } > + > + if (sleb->nodes_cnt == 0) { > + ubifs_err(c, "Unable to find signature node"); > + err = -EINVAL; > + goto out_destroy; > + } > + > + snod = list_entry(sleb->nodes.next, struct ubifs_scan_node, list); list_first_entry()? > + if (snod->type != UBIFS_SIG_NODE) { > + ubifs_err(c, "Signature node is of wrong type"); > + err = -EINVAL; > + goto out_destroy; > + } > + > + signode = snod->node; > + > + err = verify_pkcs7_signature(sup, sizeof(struct ubifs_sb_node), > + signode->sig, le32_to_cpu(signode->len), signode->len is not verified, please embed a check on the length. > + NULL, VERIFYING_UNSPECIFIED_SIGNATURE, > + NULL, NULL); > + > + if (err) > + ubifs_err(c, "Failed to verify signature"); > + else > + ubifs_msg(c, "Successfully verified super block signature"); While this is good news, is it really worth telling the user every time? > +out_destroy: > + ubifs_scan_destroy(sleb); > + > + return err; > +} > + > /** > * ubifs_init_authentication - initialize UBIFS authentication support > * @c: UBIFS file-system description object > @@ -499,3 +561,22 @@ int ubifs_hmac_wkm(struct ubifs_info *c, u8 *hmac) > return err; > return 0; > } > + > +/* > + * ubifs_hmac_zero - test if a HMAC is zero > + * @c: UBIFS file-system description object > + * @hmac: the HMAC to test > + * > + * This function tests if a HMAC is zero and returns true if it is > + * and false otherwise. > + */ > +bool ubifs_hmac_zero(struct ubifs_info *c, const u8 *hmac) > +{ > + int i; > + > + for (i = 0; i < c->hmac_desc_len; i++) > + if (hmac[i] != 0) > + return false; > + > + return true; > +} Isn't there an helper available? Maybe based on memcmp()? > diff --git a/fs/ubifs/master.c b/fs/ubifs/master.c > index 5ea51bbd14c7..5655414a76a7 100644 > --- a/fs/ubifs/master.c > +++ b/fs/ubifs/master.c > @@ -60,6 +60,40 @@ int ubifs_compare_master_node(struct ubifs_info *c, void *m1, > void *m2) > return 0; > } > > +/* mst_node_check_hash - Check hash of a master node > + * @c: UBIFS file-system description object > + * @mst: The master node > + * @expected: The expected hash of the master node > + * > + * This checks the hash of a master node against a given expected hash. > + * Note that we have two master nodes on a UBIFS image which have different > + * sequence numbers and consequently different CRCs. To be able to match > + * both master nodes we exclude the common node header containing the sequence > + * number and CRC from the hash. > + * > + * Returns 0 if the hashes are equal, a negative error code otherwise. > + */ > +static int mst_node_check_hash(const struct ubifs_info *c, > + const struct ubifs_mst_node *mst, > + const u8 *expected) > +{ > + u8 calc[UBIFS_MAX_HASH_LEN]; > + const void *node = mst; > + > + SHASH_DESC_ON_STACK(shash, c->hash_tfm); > + > + shash->tfm = c->hash_tfm; > + shash->flags = CRYPTO_TFM_REQ_MAY_SLEEP; > + > + crypto_shash_digest(shash, node + sizeof(struct ubifs_ch), > + UBIFS_MST_NODE_SZ - sizeof(struct ubifs_ch), calc); > + > + if (ubifs_check_hash(c, expected, calc)) > + return -EPERM; > + > + return 0; > +} > + > /** > * scan_for_master - search the valid master node. > * @c: UBIFS file-system description object > @@ -114,14 +148,22 @@ static int scan_for_master(struct ubifs_info *c) > if (!ubifs_authenticated(c)) > return 0; > > - err = ubifs_node_verify_hmac(c, c->mst_node, > - sizeof(struct ubifs_mst_node), > - offsetof(struct ubifs_mst_node, hmac)); > - if (err) { > - ubifs_err(c, "Failed to verify master node HMAC"); > - return -EPERM; > + if (ubifs_hmac_zero(c, c->mst_node->hmac)) { > + err = mst_node_check_hash(c, c->mst_node, > + c->sup_node->hash_mst); > + if (err) > + ubifs_err(c, "Failed to verify master node hash"); > + } else { > + err = ubifs_node_verify_hmac(c, c->mst_node, > + sizeof(struct ubifs_mst_node), > + offsetof(struct ubifs_mst_node, hmac)); > + if (err) > + ubifs_err(c, "Failed to verify master node HMAC"); > } > > + if (err) > + return -EPERM; > + > return 0; > > out: > diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c > index 67fac1e8adfb..3dd195dbd852 100644 > --- a/fs/ubifs/sb.c > +++ b/fs/ubifs/sb.c > @@ -590,17 +590,26 @@ static int authenticate_sb_node(struct ubifs_info *c, > return -EINVAL; > } > > - err = ubifs_hmac_wkm(c, hmac_wkm); > - if (err) > - return err; > - > - if (ubifs_check_hmac(c, hmac_wkm, sup->hmac_wkm)) { > - ubifs_err(c, "provided key does not fit"); > - return -ENOKEY; > + /* > + * The super block node can either be authenticated by a HMAC or > + * by a signature in a ubifs_sig_node directly following the > + * super block node to support offline image creation. > + */ > + if (ubifs_hmac_zero(c, sup->hmac)) { > + err = ubifs_sb_verify_signature(c, sup); > + } else { > + err = ubifs_hmac_wkm(c, hmac_wkm); > + if (err) > + return err; > + if (ubifs_check_hmac(c, hmac_wkm, sup->hmac_wkm)) { > + ubifs_err(c, "provided key does not fit"); > + return -ENOKEY; > + } > + err = ubifs_node_verify_hmac(c, sup, sizeof(*sup), > + offsetof(struct ubifs_sb_node, > + hmac)); > } > > - err = ubifs_node_verify_hmac(c, sup, sizeof(*sup), > - offsetof(struct ubifs_sb_node, hmac)); > if (err) > ubifs_err(c, "Failed to authenticate superblock: %d", err); > > @@ -761,18 +770,12 @@ int ubifs_read_superblock(struct ubifs_info *c) > c->old_leb_cnt = c->leb_cnt; > if (c->leb_cnt < c->vi.size && c->leb_cnt < c->max_leb_cnt) { > c->leb_cnt = min_t(int, c->max_leb_cnt, c->vi.size); > - if (c->ro_mount) > - dbg_mnt("Auto resizing (ro) from %d LEBs to %d LEBs", > - c->old_leb_cnt, c->leb_cnt); > - else { > - dbg_mnt("Auto resizing (sb) from %d LEBs to %d LEBs", > - c->old_leb_cnt, c->leb_cnt); > - sup->leb_cnt = cpu_to_le32(c->leb_cnt); > - err = ubifs_write_sb_node(c, sup); > - if (err) > - goto out; > - c->old_leb_cnt = c->leb_cnt; > - } > + sup->leb_cnt = cpu_to_le32(c->leb_cnt); > + > + c->superblock_need_write = 1; > + > + dbg_mnt("Auto resizing from %d LEBs to %d LEBs", > + c->old_leb_cnt, c->leb_cnt); Hmm, I don't fully understand the logic here. What happens to c->old_leb_cnt and the ro_mount case? > } > > c->log_bytes = (long long)c->log_lebs * c->leb_size; > @@ -930,9 +933,7 @@ int ubifs_fixup_free_space(struct ubifs_info *c) > c->space_fixup = 0; > sup->flags &= cpu_to_le32(~UBIFS_FLG_SPACE_FIXUP); > > - err = ubifs_write_sb_node(c, sup); > - if (err) > - return err; > + c->superblock_need_write = 1; > > ubifs_msg(c, "free space fixup complete"); > return err; > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c > index 8dc2818fdd84..19011e3c6ec7 100644 > --- a/fs/ubifs/super.c > +++ b/fs/ubifs/super.c > @@ -582,6 +582,8 @@ static int init_constants_early(struct ubifs_info *c) > c->ranges[UBIFS_AUTH_NODE].min_len = UBIFS_AUTH_NODE_SZ; > c->ranges[UBIFS_AUTH_NODE].max_len = UBIFS_AUTH_NODE_SZ + > UBIFS_MAX_HMAC_LEN; > + c->ranges[UBIFS_SIG_NODE].min_len = UBIFS_SIG_NODE_SZ; > + c->ranges[UBIFS_SIG_NODE].max_len = c->leb_size - UBIFS_SB_NODE_SZ; > > c->ranges[UBIFS_INO_NODE].min_len = UBIFS_INO_NODE_SZ; > c->ranges[UBIFS_INO_NODE].max_len = UBIFS_MAX_INO_NODE_SZ; > @@ -1376,6 +1378,26 @@ static int mount_ubifs(struct ubifs_info *c) > goto out_lpt; > } > > + /* > + * Handle offline signed images: Now that the master node is > + * written and its validation no longer depends on the hash > + * in the superblock, we can update the offline signed > + * superblock with a HMAC version, > + */ > + if (ubifs_authenticated(c) && ubifs_hmac_zero(c, c->sup_node->hmac)) { > + err = ubifs_hmac_wkm(c, c->sup_node->hmac_wkm); > + if (err) > + goto out_lpt; > + c->superblock_need_write = 1; > + } > + > + if (!c->ro_mount && c->superblock_need_write) { > + err = ubifs_write_sb_node(c, c->sup_node); > + if (err) > + goto out_lpt; > + c->superblock_need_write = 0; > + } > + > err = dbg_check_idx_size(c, c->bi.old_idx_sz); > if (err) > goto out_lpt; > @@ -1658,15 +1680,6 @@ static int ubifs_remount_rw(struct ubifs_info *c) > if (err) > goto out; > > - if (c->old_leb_cnt != c->leb_cnt) { > - struct ubifs_sb_node *sup = c->sup_node; > - > - sup->leb_cnt = cpu_to_le32(c->leb_cnt); > - err = ubifs_write_sb_node(c, sup); > - if (err) > - goto out; > - } > - > if (c->need_recovery) { > ubifs_msg(c, "completing deferred recovery"); > err = ubifs_write_rcvrd_mst_node(c); > @@ -1698,6 +1711,16 @@ static int ubifs_remount_rw(struct ubifs_info *c) > goto out; > } > > + if (c->superblock_need_write) { > + struct ubifs_sb_node *sup = c->sup_node; > + > + err = ubifs_write_sb_node(c, sup); > + if (err) > + goto out; > + > + c->superblock_need_write = 0; > + } > + > c->ileb_buf = vmalloc(c->leb_size); > if (!c->ileb_buf) { > err = -ENOMEM; > diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h > index 8b7c1844014f..3a73eb59859f 100644 > --- a/fs/ubifs/ubifs-media.h > +++ b/fs/ubifs/ubifs-media.h > @@ -287,6 +287,8 @@ enum { > #define UBIFS_CS_NODE_SZ sizeof(struct ubifs_cs_node) > #define UBIFS_ORPH_NODE_SZ sizeof(struct ubifs_orph_node) > #define UBIFS_AUTH_NODE_SZ sizeof(struct ubifs_auth_node) > +#define UBIFS_SIG_NODE_SZ sizeof(struct ubifs_sig_node) > + > /* Extended attribute entry nodes are identical to directory entry nodes */ > #define UBIFS_XENT_NODE_SZ UBIFS_DENT_NODE_SZ > /* Only this does not have to be multiple of 8 bytes */ > @@ -373,6 +375,7 @@ enum { > * UBIFS_CS_NODE: commit start node > * UBIFS_ORPH_NODE: orphan node > * UBIFS_AUTH_NODE: authentication node > + * UBIFS_SIG_NODE: signature node > * UBIFS_NODE_TYPES_CNT: count of supported node types > * > * Note, we index arrays by these numbers, so keep them low and contiguous. > @@ -393,6 +396,7 @@ enum { > UBIFS_CS_NODE, > UBIFS_ORPH_NODE, > UBIFS_AUTH_NODE, > + UBIFS_SIG_NODE, > UBIFS_NODE_TYPES_CNT, > }; > > @@ -650,6 +654,8 @@ struct ubifs_pad_node { > * @hmac_wkm: HMAC of a well known message (the string "UBIFS") as a convenience > * to the user to check if the correct key is passed. > * @hash_algo: The hash algo used for this filesystem (one of enum hash_algo) > + * @hash_mst: hash of the master node, only valid for signed images in which > the > + * master node does not contain a hmac > */ > struct ubifs_sb_node { > struct ubifs_ch ch; > @@ -680,7 +686,8 @@ struct ubifs_sb_node { > __u8 hmac[UBIFS_MAX_HMAC_LEN]; > __u8 hmac_wkm[UBIFS_MAX_HMAC_LEN]; > __le16 hash_algo; > - __u8 padding2[3838]; > + __u8 hash_mst[UBIFS_MAX_HASH_LEN]; > + __u8 padding2[3774]; > } __packed; > > /** > @@ -782,6 +789,18 @@ struct ubifs_auth_node { > __u8 hmac[]; > } __packed; > > +/** > + * struct ubifs_sig_node - node for signing other nodes > + * @ch: common header > + * @len: The length of the signature data > + * @sig: The signature data > + */ > +struct ubifs_sig_node { > + struct ubifs_ch ch; > + __le32 len; > + __u8 sig[]; > +} __packed; > + Can we please have a version or type field? Just in case we want to support more than PKCS#7 at some point. This new node is not used at lot, so we can waste a little space... Did you check, what is the typical length of a signature? Maybe adding more padding fields is worth it. > /** > * struct ubifs_branch - key/reference/length branch > * @lnum: LEB number of the target node > diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h > index 1ae12900e01d..56afa87dd3ae 100644 > --- a/fs/ubifs/ubifs.h > +++ b/fs/ubifs/ubifs.h > @@ -1304,6 +1304,7 @@ struct ubifs_info { > unsigned int rw_incompat:1; > unsigned int assert_action:2; > unsigned int authenticated:1; > + unsigned int superblock_need_write:1; > > struct mutex tnc_mutex; > struct ubifs_zbranch zroot; > @@ -1689,6 +1690,9 @@ static inline int ubifs_auth_node_sz(const struct > ubifs_info *c) > else > return 0; > } > +int ubifs_sb_verify_signature(struct ubifs_info *c, > + const struct ubifs_sb_node *sup); > +bool ubifs_hmac_zero(struct ubifs_info *c, const u8 *hmac); > > int ubifs_hmac_wkm(struct ubifs_info *c, u8 *hmac); > > -- > 2.20.1 Thanks, //richard
Hi Richard, Thanks for review. On Sun, May 05, 2019 at 11:13:38PM +0200, Richard Weinberger wrote: > ----- Ursprüngliche Mail ----- > > Von: "Sascha Hauer" <s.hauer@pengutronix.de> > > An: "linux-mtd" <linux-mtd@lists.infradead.org> > > CC: "richard" <richard@nod.at>, kernel@pengutronix.de, "Sascha Hauer" <s.hauer@pengutronix.de> > > Gesendet: Montag, 1. April 2019 16:30:01 > > Betreff: [PATCH] ubifs: support offline signed images > > > HMACs can only be generated on the system the UBIFS image is running on. > > Because at mkfs.ubifs time you don't have the key which might be hardware > specific, right? Yes, right. > > +int ubifs_sb_verify_signature(struct ubifs_info *c, > > + const struct ubifs_sb_node *sup) > > +{ > > + int err; > > + struct ubifs_scan_leb *sleb; > > + struct ubifs_scan_node *snod; > > + const struct ubifs_sig_node *signode; > > + > > + err = ubifs_leb_read(c, UBIFS_SB_LNUM, c->sbuf, 0, c->leb_size, 1); > > This line looks wrong here, leftover from old code? :) Yup, can be removed. > > > + sleb = ubifs_scan(c, UBIFS_SB_LNUM, UBIFS_SB_NODE_SZ, c->sbuf, 0); > > + if (IS_ERR(sleb)) { > > + err = PTR_ERR(sleb); > > + return err; > > + } > > + > > + if (sleb->nodes_cnt == 0) { > > + ubifs_err(c, "Unable to find signature node"); > > + err = -EINVAL; > > + goto out_destroy; > > + } > > + > > + snod = list_entry(sleb->nodes.next, struct ubifs_scan_node, list); > > list_first_entry()? Yes. > > > + if (snod->type != UBIFS_SIG_NODE) { > > + ubifs_err(c, "Signature node is of wrong type"); > > + err = -EINVAL; > > + goto out_destroy; > > + } > > + > > + signode = snod->node; > > + > > + err = verify_pkcs7_signature(sup, sizeof(struct ubifs_sb_node), > > + signode->sig, le32_to_cpu(signode->len), > > signode->len is not verified, please embed a check on the length. Ok. > > > + NULL, VERIFYING_UNSPECIFIED_SIGNATURE, > > + NULL, NULL); > > + > > + if (err) > > + ubifs_err(c, "Failed to verify signature"); > > + else > > + ubifs_msg(c, "Successfully verified super block signature"); > > While this is good news, is it really worth telling the user every time? No, will remove. > > > +out_destroy: > > + ubifs_scan_destroy(sleb); > > + > > + return err; > > +} > > + > > /** > > * ubifs_init_authentication - initialize UBIFS authentication support > > * @c: UBIFS file-system description object > > @@ -499,3 +561,22 @@ int ubifs_hmac_wkm(struct ubifs_info *c, u8 *hmac) > > return err; > > return 0; > > } > > + > > +/* > > + * ubifs_hmac_zero - test if a HMAC is zero > > + * @c: UBIFS file-system description object > > + * @hmac: the HMAC to test > > + * > > + * This function tests if a HMAC is zero and returns true if it is > > + * and false otherwise. > > + */ > > +bool ubifs_hmac_zero(struct ubifs_info *c, const u8 *hmac) > > +{ > > + int i; > > + > > + for (i = 0; i < c->hmac_desc_len; i++) > > + if (hmac[i] != 0) > > + return false; > > + > > + return true; > > +} > > Isn't there an helper available? > Maybe based on memcmp()? Indeed there's memchr_inv() - Find an unmatching character in an area of memory. > > > diff --git a/fs/ubifs/master.c b/fs/ubifs/master.c > > index 5ea51bbd14c7..5655414a76a7 100644 > > --- a/fs/ubifs/master.c > > +++ b/fs/ubifs/master.c > > @@ -60,6 +60,40 @@ int ubifs_compare_master_node(struct ubifs_info *c, void *m1, > > void *m2) > > return 0; > > } > > > > diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c > > index 67fac1e8adfb..3dd195dbd852 100644 > > --- a/fs/ubifs/sb.c > > +++ b/fs/ubifs/sb.c > > @@ -761,18 +770,12 @@ int ubifs_read_superblock(struct ubifs_info *c) > > c->old_leb_cnt = c->leb_cnt; > > if (c->leb_cnt < c->vi.size && c->leb_cnt < c->max_leb_cnt) { > > c->leb_cnt = min_t(int, c->max_leb_cnt, c->vi.size); > > - if (c->ro_mount) > > - dbg_mnt("Auto resizing (ro) from %d LEBs to %d LEBs", > > - c->old_leb_cnt, c->leb_cnt); > > - else { > > - dbg_mnt("Auto resizing (sb) from %d LEBs to %d LEBs", > > - c->old_leb_cnt, c->leb_cnt); > > - sup->leb_cnt = cpu_to_le32(c->leb_cnt); > > - err = ubifs_write_sb_node(c, sup); > > - if (err) > > - goto out; > > - c->old_leb_cnt = c->leb_cnt; > > - } > > + sup->leb_cnt = cpu_to_le32(c->leb_cnt); > > + > > + c->superblock_need_write = 1; > > + > > + dbg_mnt("Auto resizing from %d LEBs to %d LEBs", > > + c->old_leb_cnt, c->leb_cnt); > > Hmm, I don't fully understand the logic here. > What happens to c->old_leb_cnt and the ro_mount case? c->old_leb_cnt is only used by the code to determine if the leb_cnt has changed, i.e. in ubifs_remount_rw() we have: if (c->old_leb_cnt != c->leb_cnt) { ... err = ubifs_write_sb_node(c, sup); } We now have the explicit c->superblock_need_write flag which has the same purpose. c->old_leb_cnt is no longer used, it should have been removed in this patch, but I missed it. Will do in v2. In the ro_mount case nothing changes really. The code will set c->superblock_need_write and based on that flag we will write the updated superblock when we are going readwrite later. The same happened before this patch based on (c->old_leb_cnt != c->leb_cnt), only that I now the master node is written first and the sb_node afterwards. > > + * struct ubifs_sig_node - node for signing other nodes > > + * @ch: common header > > + * @len: The length of the signature data > > + * @sig: The signature data > > + */ > > +struct ubifs_sig_node { > > + struct ubifs_ch ch; > > + __le32 len; > > + __u8 sig[]; > > +} __packed; > > + > > Can we please have a version or type field? > Just in case we want to support more than PKCS#7 at some point. > This new node is not used at lot, so we can waste a little space... Sure, I'll add a type field. > > Did you check, what is the typical length of a signature? > Maybe adding more padding fields is worth it. The signature is 670 bytes in my case. It's the x509 file the Kernel generates for module signing. I don't know how typical that size is. What I know is that there is no upper limit for the size of a x509 file, not sure if that can become a problem some day. Sascha
diff --git a/fs/ubifs/Kconfig b/fs/ubifs/Kconfig index 9da2f135121b..3893668d5dae 100644 --- a/fs/ubifs/Kconfig +++ b/fs/ubifs/Kconfig @@ -78,6 +78,7 @@ config UBIFS_FS_AUTHENTICATION bool "UBIFS authentication support" depends on KEYS select CRYPTO_HMAC + select SYSTEM_DATA_VERIFICATION help Enable authentication support for UBIFS. This feature offers protection against offline changes for both data and metadata of the filesystem. diff --git a/fs/ubifs/auth.c b/fs/ubifs/auth.c index 5bf5fd08879e..cff5820ee54b 100644 --- a/fs/ubifs/auth.c +++ b/fs/ubifs/auth.c @@ -10,10 +10,12 @@ */ #include <linux/crypto.h> +#include <linux/verification.h> #include <crypto/hash.h> #include <crypto/sha.h> #include <crypto/algapi.h> #include <keys/user-type.h> +#include <keys/asymmetric-type.h> #include "ubifs.h" @@ -217,6 +219,66 @@ int __ubifs_node_check_hash(const struct ubifs_info *c, const void *node, return 0; } +/** + * ubifs_sb_verify_signature - verify the signature of a superblock + * @c: UBIFS file-system description object + * @sup: The superblock node + * + * To support offline signed images the superblock can be signed with a + * PKCS#7 signature. The signature is placed directly behind the superblock + * node in an ubifs_sig_node. + * + * Returns 0 when the signature can be successfully verified or a negative + * error code if not. + */ +int ubifs_sb_verify_signature(struct ubifs_info *c, + const struct ubifs_sb_node *sup) +{ + int err; + struct ubifs_scan_leb *sleb; + struct ubifs_scan_node *snod; + const struct ubifs_sig_node *signode; + + err = ubifs_leb_read(c, UBIFS_SB_LNUM, c->sbuf, 0, c->leb_size, 1); + + sleb = ubifs_scan(c, UBIFS_SB_LNUM, UBIFS_SB_NODE_SZ, c->sbuf, 0); + if (IS_ERR(sleb)) { + err = PTR_ERR(sleb); + return err; + } + + if (sleb->nodes_cnt == 0) { + ubifs_err(c, "Unable to find signature node"); + err = -EINVAL; + goto out_destroy; + } + + snod = list_entry(sleb->nodes.next, struct ubifs_scan_node, list); + + if (snod->type != UBIFS_SIG_NODE) { + ubifs_err(c, "Signature node is of wrong type"); + err = -EINVAL; + goto out_destroy; + } + + signode = snod->node; + + err = verify_pkcs7_signature(sup, sizeof(struct ubifs_sb_node), + signode->sig, le32_to_cpu(signode->len), + NULL, VERIFYING_UNSPECIFIED_SIGNATURE, + NULL, NULL); + + if (err) + ubifs_err(c, "Failed to verify signature"); + else + ubifs_msg(c, "Successfully verified super block signature"); + +out_destroy: + ubifs_scan_destroy(sleb); + + return err; +} + /** * ubifs_init_authentication - initialize UBIFS authentication support * @c: UBIFS file-system description object @@ -499,3 +561,22 @@ int ubifs_hmac_wkm(struct ubifs_info *c, u8 *hmac) return err; return 0; } + +/* + * ubifs_hmac_zero - test if a HMAC is zero + * @c: UBIFS file-system description object + * @hmac: the HMAC to test + * + * This function tests if a HMAC is zero and returns true if it is + * and false otherwise. + */ +bool ubifs_hmac_zero(struct ubifs_info *c, const u8 *hmac) +{ + int i; + + for (i = 0; i < c->hmac_desc_len; i++) + if (hmac[i] != 0) + return false; + + return true; +} diff --git a/fs/ubifs/master.c b/fs/ubifs/master.c index 5ea51bbd14c7..5655414a76a7 100644 --- a/fs/ubifs/master.c +++ b/fs/ubifs/master.c @@ -60,6 +60,40 @@ int ubifs_compare_master_node(struct ubifs_info *c, void *m1, void *m2) return 0; } +/* mst_node_check_hash - Check hash of a master node + * @c: UBIFS file-system description object + * @mst: The master node + * @expected: The expected hash of the master node + * + * This checks the hash of a master node against a given expected hash. + * Note that we have two master nodes on a UBIFS image which have different + * sequence numbers and consequently different CRCs. To be able to match + * both master nodes we exclude the common node header containing the sequence + * number and CRC from the hash. + * + * Returns 0 if the hashes are equal, a negative error code otherwise. + */ +static int mst_node_check_hash(const struct ubifs_info *c, + const struct ubifs_mst_node *mst, + const u8 *expected) +{ + u8 calc[UBIFS_MAX_HASH_LEN]; + const void *node = mst; + + SHASH_DESC_ON_STACK(shash, c->hash_tfm); + + shash->tfm = c->hash_tfm; + shash->flags = CRYPTO_TFM_REQ_MAY_SLEEP; + + crypto_shash_digest(shash, node + sizeof(struct ubifs_ch), + UBIFS_MST_NODE_SZ - sizeof(struct ubifs_ch), calc); + + if (ubifs_check_hash(c, expected, calc)) + return -EPERM; + + return 0; +} + /** * scan_for_master - search the valid master node. * @c: UBIFS file-system description object @@ -114,14 +148,22 @@ static int scan_for_master(struct ubifs_info *c) if (!ubifs_authenticated(c)) return 0; - err = ubifs_node_verify_hmac(c, c->mst_node, - sizeof(struct ubifs_mst_node), - offsetof(struct ubifs_mst_node, hmac)); - if (err) { - ubifs_err(c, "Failed to verify master node HMAC"); - return -EPERM; + if (ubifs_hmac_zero(c, c->mst_node->hmac)) { + err = mst_node_check_hash(c, c->mst_node, + c->sup_node->hash_mst); + if (err) + ubifs_err(c, "Failed to verify master node hash"); + } else { + err = ubifs_node_verify_hmac(c, c->mst_node, + sizeof(struct ubifs_mst_node), + offsetof(struct ubifs_mst_node, hmac)); + if (err) + ubifs_err(c, "Failed to verify master node HMAC"); } + if (err) + return -EPERM; + return 0; out: diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c index 67fac1e8adfb..3dd195dbd852 100644 --- a/fs/ubifs/sb.c +++ b/fs/ubifs/sb.c @@ -590,17 +590,26 @@ static int authenticate_sb_node(struct ubifs_info *c, return -EINVAL; } - err = ubifs_hmac_wkm(c, hmac_wkm); - if (err) - return err; - - if (ubifs_check_hmac(c, hmac_wkm, sup->hmac_wkm)) { - ubifs_err(c, "provided key does not fit"); - return -ENOKEY; + /* + * The super block node can either be authenticated by a HMAC or + * by a signature in a ubifs_sig_node directly following the + * super block node to support offline image creation. + */ + if (ubifs_hmac_zero(c, sup->hmac)) { + err = ubifs_sb_verify_signature(c, sup); + } else { + err = ubifs_hmac_wkm(c, hmac_wkm); + if (err) + return err; + if (ubifs_check_hmac(c, hmac_wkm, sup->hmac_wkm)) { + ubifs_err(c, "provided key does not fit"); + return -ENOKEY; + } + err = ubifs_node_verify_hmac(c, sup, sizeof(*sup), + offsetof(struct ubifs_sb_node, + hmac)); } - err = ubifs_node_verify_hmac(c, sup, sizeof(*sup), - offsetof(struct ubifs_sb_node, hmac)); if (err) ubifs_err(c, "Failed to authenticate superblock: %d", err); @@ -761,18 +770,12 @@ int ubifs_read_superblock(struct ubifs_info *c) c->old_leb_cnt = c->leb_cnt; if (c->leb_cnt < c->vi.size && c->leb_cnt < c->max_leb_cnt) { c->leb_cnt = min_t(int, c->max_leb_cnt, c->vi.size); - if (c->ro_mount) - dbg_mnt("Auto resizing (ro) from %d LEBs to %d LEBs", - c->old_leb_cnt, c->leb_cnt); - else { - dbg_mnt("Auto resizing (sb) from %d LEBs to %d LEBs", - c->old_leb_cnt, c->leb_cnt); - sup->leb_cnt = cpu_to_le32(c->leb_cnt); - err = ubifs_write_sb_node(c, sup); - if (err) - goto out; - c->old_leb_cnt = c->leb_cnt; - } + sup->leb_cnt = cpu_to_le32(c->leb_cnt); + + c->superblock_need_write = 1; + + dbg_mnt("Auto resizing from %d LEBs to %d LEBs", + c->old_leb_cnt, c->leb_cnt); } c->log_bytes = (long long)c->log_lebs * c->leb_size; @@ -930,9 +933,7 @@ int ubifs_fixup_free_space(struct ubifs_info *c) c->space_fixup = 0; sup->flags &= cpu_to_le32(~UBIFS_FLG_SPACE_FIXUP); - err = ubifs_write_sb_node(c, sup); - if (err) - return err; + c->superblock_need_write = 1; ubifs_msg(c, "free space fixup complete"); return err; diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 8dc2818fdd84..19011e3c6ec7 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -582,6 +582,8 @@ static int init_constants_early(struct ubifs_info *c) c->ranges[UBIFS_AUTH_NODE].min_len = UBIFS_AUTH_NODE_SZ; c->ranges[UBIFS_AUTH_NODE].max_len = UBIFS_AUTH_NODE_SZ + UBIFS_MAX_HMAC_LEN; + c->ranges[UBIFS_SIG_NODE].min_len = UBIFS_SIG_NODE_SZ; + c->ranges[UBIFS_SIG_NODE].max_len = c->leb_size - UBIFS_SB_NODE_SZ; c->ranges[UBIFS_INO_NODE].min_len = UBIFS_INO_NODE_SZ; c->ranges[UBIFS_INO_NODE].max_len = UBIFS_MAX_INO_NODE_SZ; @@ -1376,6 +1378,26 @@ static int mount_ubifs(struct ubifs_info *c) goto out_lpt; } + /* + * Handle offline signed images: Now that the master node is + * written and its validation no longer depends on the hash + * in the superblock, we can update the offline signed + * superblock with a HMAC version, + */ + if (ubifs_authenticated(c) && ubifs_hmac_zero(c, c->sup_node->hmac)) { + err = ubifs_hmac_wkm(c, c->sup_node->hmac_wkm); + if (err) + goto out_lpt; + c->superblock_need_write = 1; + } + + if (!c->ro_mount && c->superblock_need_write) { + err = ubifs_write_sb_node(c, c->sup_node); + if (err) + goto out_lpt; + c->superblock_need_write = 0; + } + err = dbg_check_idx_size(c, c->bi.old_idx_sz); if (err) goto out_lpt; @@ -1658,15 +1680,6 @@ static int ubifs_remount_rw(struct ubifs_info *c) if (err) goto out; - if (c->old_leb_cnt != c->leb_cnt) { - struct ubifs_sb_node *sup = c->sup_node; - - sup->leb_cnt = cpu_to_le32(c->leb_cnt); - err = ubifs_write_sb_node(c, sup); - if (err) - goto out; - } - if (c->need_recovery) { ubifs_msg(c, "completing deferred recovery"); err = ubifs_write_rcvrd_mst_node(c); @@ -1698,6 +1711,16 @@ static int ubifs_remount_rw(struct ubifs_info *c) goto out; } + if (c->superblock_need_write) { + struct ubifs_sb_node *sup = c->sup_node; + + err = ubifs_write_sb_node(c, sup); + if (err) + goto out; + + c->superblock_need_write = 0; + } + c->ileb_buf = vmalloc(c->leb_size); if (!c->ileb_buf) { err = -ENOMEM; diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h index 8b7c1844014f..3a73eb59859f 100644 --- a/fs/ubifs/ubifs-media.h +++ b/fs/ubifs/ubifs-media.h @@ -287,6 +287,8 @@ enum { #define UBIFS_CS_NODE_SZ sizeof(struct ubifs_cs_node) #define UBIFS_ORPH_NODE_SZ sizeof(struct ubifs_orph_node) #define UBIFS_AUTH_NODE_SZ sizeof(struct ubifs_auth_node) +#define UBIFS_SIG_NODE_SZ sizeof(struct ubifs_sig_node) + /* Extended attribute entry nodes are identical to directory entry nodes */ #define UBIFS_XENT_NODE_SZ UBIFS_DENT_NODE_SZ /* Only this does not have to be multiple of 8 bytes */ @@ -373,6 +375,7 @@ enum { * UBIFS_CS_NODE: commit start node * UBIFS_ORPH_NODE: orphan node * UBIFS_AUTH_NODE: authentication node + * UBIFS_SIG_NODE: signature node * UBIFS_NODE_TYPES_CNT: count of supported node types * * Note, we index arrays by these numbers, so keep them low and contiguous. @@ -393,6 +396,7 @@ enum { UBIFS_CS_NODE, UBIFS_ORPH_NODE, UBIFS_AUTH_NODE, + UBIFS_SIG_NODE, UBIFS_NODE_TYPES_CNT, }; @@ -650,6 +654,8 @@ struct ubifs_pad_node { * @hmac_wkm: HMAC of a well known message (the string "UBIFS") as a convenience * to the user to check if the correct key is passed. * @hash_algo: The hash algo used for this filesystem (one of enum hash_algo) + * @hash_mst: hash of the master node, only valid for signed images in which the + * master node does not contain a hmac */ struct ubifs_sb_node { struct ubifs_ch ch; @@ -680,7 +686,8 @@ struct ubifs_sb_node { __u8 hmac[UBIFS_MAX_HMAC_LEN]; __u8 hmac_wkm[UBIFS_MAX_HMAC_LEN]; __le16 hash_algo; - __u8 padding2[3838]; + __u8 hash_mst[UBIFS_MAX_HASH_LEN]; + __u8 padding2[3774]; } __packed; /** @@ -782,6 +789,18 @@ struct ubifs_auth_node { __u8 hmac[]; } __packed; +/** + * struct ubifs_sig_node - node for signing other nodes + * @ch: common header + * @len: The length of the signature data + * @sig: The signature data + */ +struct ubifs_sig_node { + struct ubifs_ch ch; + __le32 len; + __u8 sig[]; +} __packed; + /** * struct ubifs_branch - key/reference/length branch * @lnum: LEB number of the target node diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 1ae12900e01d..56afa87dd3ae 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1304,6 +1304,7 @@ struct ubifs_info { unsigned int rw_incompat:1; unsigned int assert_action:2; unsigned int authenticated:1; + unsigned int superblock_need_write:1; struct mutex tnc_mutex; struct ubifs_zbranch zroot; @@ -1689,6 +1690,9 @@ static inline int ubifs_auth_node_sz(const struct ubifs_info *c) else return 0; } +int ubifs_sb_verify_signature(struct ubifs_info *c, + const struct ubifs_sb_node *sup); +bool ubifs_hmac_zero(struct ubifs_info *c, const u8 *hmac); int ubifs_hmac_wkm(struct ubifs_info *c, u8 *hmac);
HMACs can only be generated on the system the UBIFS image is running on. To support offline signed images we add a PKCS#7 signature to the UBIFS image which can be created by mkfs.ubifs. Both the master node and the superblock need to be authenticated, during normal runtime both are protected with HMACs. For offline signature support however only a single signature is desired. We add a signature covering the superblock node directly behind it. To protect the master node a hash of the master node is added to the superblock which is used when the master node doesn't contain a HMAC. Transition to a read/write filesystem is also supported. During transition first the master node is rewritten with a HMAC (implicitly, it is written anyway as the FS is marked dirty). Afterwards the superblock is rewritten with a HMAC. Once after the image has been mounted read/write it is HMAC only, the signature is no longer required or even present on the filesystem. In an offline signed image the master node is authenticated by the superblock. In a transition to r/w we have to make sure that the master node is rewritten before the superblock node. In this case the master node gets a HMAC and its authenticity no longer depends on the superblock node. There are some cases in which the current code first writes the superblock node though, so with this patch writing of the superblock node is delayed until the master node is written. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- fs/ubifs/Kconfig | 1 + fs/ubifs/auth.c | 81 ++++++++++++++++++++++++++++++++++++++++++ fs/ubifs/master.c | 54 ++++++++++++++++++++++++---- fs/ubifs/sb.c | 49 ++++++++++++------------- fs/ubifs/super.c | 41 ++++++++++++++++----- fs/ubifs/ubifs-media.h | 21 ++++++++++- fs/ubifs/ubifs.h | 4 +++ 7 files changed, 211 insertions(+), 40 deletions(-)