* [PATCH 0/1] ubifs: support authentication without hmac @ 2020-06-25 15:59 Torben Hohn 2020-06-25 15:59 ` [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given Torben Hohn ` (2 more replies) 0 siblings, 3 replies; 35+ messages in thread From: Torben Hohn @ 2020-06-25 15:59 UTC (permalink / raw) To: richard; +Cc: bigeasy, linux-mtd, tglx, s.hauer This patch adds support for ubifs authentication without HMAC, which obviously only works for a read-only mount. ubiblock and dm-verity are not supported by u-boot, and the kernel on the target is loaded by u-boot out of the RFS. This is a first try to implement this. It boots fine, and the WARN_ON is not triggered. I plan to update the docs also, but i would like to have some positive comments on this before. Torben Hohn (1): ubifs: support authentication, for ro mount, when no key is given fs/ubifs/auth.c | 69 ++++++++++++++++++++++++++++++++++++++++++---- fs/ubifs/gc.c | 2 +- fs/ubifs/journal.c | 12 ++++---- fs/ubifs/lpt.c | 4 +-- fs/ubifs/master.c | 2 +- fs/ubifs/replay.c | 2 +- fs/ubifs/sb.c | 16 +++++++---- fs/ubifs/super.c | 21 ++++++++++---- fs/ubifs/ubifs.h | 48 +++++++++++++++++++++----------- 9 files changed, 133 insertions(+), 43 deletions(-) -- 2.20.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given 2020-06-25 15:59 [PATCH 0/1] ubifs: support authentication without hmac Torben Hohn @ 2020-06-25 15:59 ` Torben Hohn 2020-06-26 4:31 ` Sascha Hauer 2020-06-26 8:09 ` [PATCH 0/1] ubifs: support authentication without hmac Richard Weinberger 2020-06-26 11:29 ` [PATCH v2 0/4] " Torben Hohn 2 siblings, 1 reply; 35+ messages in thread From: Torben Hohn @ 2020-06-25 15:59 UTC (permalink / raw) To: richard; +Cc: bigeasy, linux-mtd, tglx, s.hauer Ubifs authentication requires a hmac key, even when a filesystem is mounted read-only. Split c->authenticated into _rw and _ro. Also implement ubifs_init_authentication_read_only(), which only allocates the structures needed for validating the hashes. That is called, when no auth_key_name is specified, only auth_hash_name. It sets c->authenticated_ro to true. Then implement ubifs_authenticated_read() and ubifs_authenticated_write(). And change all occurences of ubifs_authenticated() to the respective read or write version. ubifs_authenticated_write() verifies, that it is not called when only c->authenticated_ro is active. (The WARN_ON should probably be BUG_ON(), though) Signed-off-by: Torben Hohn <torben.hohn@linutronix.de> --- fs/ubifs/auth.c | 69 ++++++++++++++++++++++++++++++++++++++++++---- fs/ubifs/gc.c | 2 +- fs/ubifs/journal.c | 12 ++++---- fs/ubifs/lpt.c | 4 +-- fs/ubifs/master.c | 2 +- fs/ubifs/replay.c | 2 +- fs/ubifs/sb.c | 16 +++++++---- fs/ubifs/super.c | 21 ++++++++++---- fs/ubifs/ubifs.h | 48 +++++++++++++++++++++----------- 9 files changed, 133 insertions(+), 43 deletions(-) diff --git a/fs/ubifs/auth.c b/fs/ubifs/auth.c index cc5c0abfd536..5429a05f537e 100644 --- a/fs/ubifs/auth.c +++ b/fs/ubifs/auth.c @@ -94,7 +94,7 @@ static struct shash_desc *ubifs_get_desc(const struct ubifs_info *c, struct shash_desc *desc; int err; - if (!ubifs_authenticated(c)) + if (!ubifs_authenticated_read(c)) return NULL; desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(tfm), GFP_KERNEL); @@ -248,6 +248,61 @@ int ubifs_sb_verify_signature(struct ubifs_info *c, return err; } +/** + * ubifs_init_authentication_read_only - init only the read_only parts + * + * @c: UBIFS file-system description object + * + * This function returns 0 for success or a negative error code otherwise. + */ + +int ubifs_init_authentication_read_only(struct ubifs_info *c) +{ + int err; + + if (!c->auth_hash_name) { + ubifs_err(c, "authentication hash name needed with authentication"); + return -EINVAL; + } + + c->auth_hash_algo = match_string(hash_algo_name, HASH_ALGO__LAST, + c->auth_hash_name); + if ((int)c->auth_hash_algo < 0) { + ubifs_err(c, "Unknown hash algo %s specified", + c->auth_hash_name); + return -EINVAL; + } + + c->hash_tfm = crypto_alloc_shash(c->auth_hash_name, 0, 0); + if (IS_ERR(c->hash_tfm)) { + err = PTR_ERR(c->hash_tfm); + ubifs_err(c, "Can not allocate %s: %d", + c->auth_hash_name, err); + goto out; + } + + c->hash_len = crypto_shash_digestsize(c->hash_tfm); + if (c->hash_len > UBIFS_HASH_ARR_SZ) { + ubifs_err(c, "hash %s is bigger than maximum allowed hash size (%d > %d)", + c->auth_hash_name, c->hash_len, UBIFS_HASH_ARR_SZ); + err = -EINVAL; + goto out_free_hash; + } + + c->authenticated_ro = true; + + c->log_hash = ubifs_hash_get_desc(c); + if (IS_ERR(c->log_hash)) + goto out_free_hash; + + err = 0; +out_free_hash: + if (err) + crypto_free_shash(c->hash_tfm); +out: + return err; +} + /** * ubifs_init_authentication - initialize UBIFS authentication support * @c: UBIFS file-system description object @@ -335,7 +390,7 @@ int ubifs_init_authentication(struct ubifs_info *c) if (err) goto out_free_hmac; - c->authenticated = true; + c->authenticated_rw = true; c->log_hash = ubifs_hash_get_desc(c); if (IS_ERR(c->log_hash)) @@ -364,11 +419,15 @@ int ubifs_init_authentication(struct ubifs_info *c) */ void __ubifs_exit_authentication(struct ubifs_info *c) { - if (!ubifs_authenticated(c)) + if (!ubifs_authenticated_read(c)) return; - crypto_free_shash(c->hmac_tfm); crypto_free_shash(c->hash_tfm); + + if (!c->authenticated_rw) + return; + + crypto_free_shash(c->hmac_tfm); kfree(c->log_hash); } @@ -511,7 +570,7 @@ int ubifs_hmac_wkm(struct ubifs_info *c, u8 *hmac) int err; const char well_known_message[] = "UBIFS"; - if (!ubifs_authenticated(c)) + if (!ubifs_authenticated_write(c)) return 0; shash->tfm = c->hmac_tfm; diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index 62cb3db44e6e..8616e8ba97d6 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -410,7 +410,7 @@ static int move_nodes(struct ubifs_info *c, struct ubifs_scan_leb *sleb) moved = 1; } - if (ubifs_authenticated(c) && moved) { + if (ubifs_authenticated_write(c) && moved) { struct ubifs_auth_node *auth; auth = kmalloc(ubifs_auth_node_sz(c), GFP_NOFS); diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index e5ec1afe1c66..da13e5eb93e1 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -80,7 +80,7 @@ static inline void zero_trun_node_unused(struct ubifs_trun_node *trun) static void ubifs_add_auth_dirt(struct ubifs_info *c, int lnum) { - if (ubifs_authenticated(c)) + if (ubifs_authenticated_write(c)) ubifs_add_dirt(c, lnum, ubifs_auth_node_sz(c)); } @@ -278,7 +278,7 @@ static int write_head(struct ubifs_info *c, int jhead, void *buf, int len, dbg_jnl("jhead %s, LEB %d:%d, len %d", dbg_jhead(jhead), *lnum, *offs, len); - if (ubifs_authenticated(c)) { + if (ubifs_authenticated_write(c)) { err = ubifs_hash_nodes(c, buf, len, c->jheads[jhead].log_hash); if (err) return err; @@ -572,7 +572,7 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir, len = aligned_dlen + aligned_ilen + UBIFS_INO_NODE_SZ; /* Make sure to also account for extended attributes */ - if (ubifs_authenticated(c)) + if (ubifs_authenticated_write(c)) len += ALIGN(host_ui->data_len, 8) + ubifs_auth_node_sz(c); else len += host_ui->data_len; @@ -778,7 +778,7 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode, } dlen = UBIFS_DATA_NODE_SZ + out_len; - if (ubifs_authenticated(c)) + if (ubifs_authenticated_write(c)) write_len = ALIGN(dlen, 8) + auth_len; else write_len = dlen; @@ -860,7 +860,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) write_len += UBIFS_INO_NODE_SZ * ui->xattr_cnt; } - if (ubifs_authenticated(c)) + if (ubifs_authenticated_write(c)) write_len += ALIGN(ilen, 8) + ubifs_auth_node_sz(c); else write_len += ilen; @@ -1572,7 +1572,7 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode, /* Must make reservation before allocating sequence numbers */ len = UBIFS_TRUN_NODE_SZ + UBIFS_INO_NODE_SZ; - if (ubifs_authenticated(c)) + if (ubifs_authenticated_write(c)) len += ALIGN(dlen, 8) + ubifs_auth_node_sz(c); else len += dlen; diff --git a/fs/ubifs/lpt.c b/fs/ubifs/lpt.c index e21abf250951..f3a7136518d1 100644 --- a/fs/ubifs/lpt.c +++ b/fs/ubifs/lpt.c @@ -1660,7 +1660,7 @@ int ubifs_lpt_calc_hash(struct ubifs_info *c, u8 *hash) void *buf; int err; - if (!ubifs_authenticated(c)) + if (!ubifs_authenticated_read(c)) return 0; if (!c->nroot) { @@ -1750,7 +1750,7 @@ static int lpt_check_hash(struct ubifs_info *c) int err; u8 hash[UBIFS_HASH_ARR_SZ]; - if (!ubifs_authenticated(c)) + if (!ubifs_authenticated_read(c)) return 0; err = ubifs_lpt_calc_hash(c, hash); diff --git a/fs/ubifs/master.c b/fs/ubifs/master.c index 911d0555b9f2..57304e3f7016 100644 --- a/fs/ubifs/master.c +++ b/fs/ubifs/master.c @@ -129,7 +129,7 @@ static int scan_for_master(struct ubifs_info *c) c->mst_offs = offs; ubifs_scan_destroy(sleb); - if (!ubifs_authenticated(c)) + if (!ubifs_authenticated_read(c)) return 0; if (ubifs_hmac_zero(c, c->mst_node->hmac)) { diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c index b69ffac7e415..6c1b8739359c 100644 --- a/fs/ubifs/replay.c +++ b/fs/ubifs/replay.c @@ -595,7 +595,7 @@ static int authenticate_sleb(struct ubifs_info *c, struct ubifs_scan_leb *sleb, u8 hash[UBIFS_HASH_ARR_SZ]; u8 hmac[UBIFS_HMAC_ARR_SZ]; - if (!ubifs_authenticated(c)) + if (!ubifs_authenticated_write(c)) return sleb->nodes_cnt; list_for_each_entry(snod, &sleb->nodes, list) { diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c index 4b4b65b48c57..52396a92f8af 100644 --- a/fs/ubifs/sb.c +++ b/fs/ubifs/sb.c @@ -176,7 +176,7 @@ static int create_default_filesystem(struct ubifs_info *c) sup_flags |= UBIFS_FLG_BIGLPT; sup_flags |= UBIFS_FLG_DOUBLE_HASH; - if (ubifs_authenticated(c)) { + if (ubifs_authenticated_write(c)) { sup_flags |= UBIFS_FLG_AUTHENTICATION; sup->hash_algo = cpu_to_le16(c->auth_hash_algo); err = ubifs_hmac_wkm(c, sup->hmac_wkm); @@ -542,20 +542,20 @@ static int authenticate_sb_node(struct ubifs_info *c, int hash_algo; int err; - if (c->authenticated && !authenticated) { + if (ubifs_authenticated_read(c) && !authenticated) { ubifs_err(c, "authenticated FS forced, but found FS without authentication"); return -EINVAL; } - if (!c->authenticated && authenticated) { - ubifs_err(c, "authenticated FS found, but no key given"); + if (!ubifs_authenticated_read(c) && authenticated) { + ubifs_err(c, "authenticated FS found, but no key nor hash name given"); return -EINVAL; } ubifs_msg(c, "Mounting in %sauthenticated mode", - c->authenticated ? "" : "un"); + c->authenticated_rw ? "RW " : (c->authenticated_ro ? "RO " :"un")); - if (!c->authenticated) + if (!ubifs_authenticated_read(c)) return 0; if (!IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) @@ -583,6 +583,10 @@ static int authenticate_sb_node(struct ubifs_info *c, if (ubifs_hmac_zero(c, sup->hmac)) { err = ubifs_sb_verify_signature(c, sup); } else { + if (!ubifs_authenticated_write(c)) { + ubifs_err(c, "HMAC authenticated FS found, but no key given"); + return -EINVAL; + } err = ubifs_hmac_wkm(c, hmac_wkm); if (err) return err; diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 7fc2f3f07c16..ec95f1f50e5e 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1291,6 +1291,17 @@ static int mount_ubifs(struct ubifs_info *c) err = -EINVAL; goto out_free; } + } else if (c->auth_hash_name) { + if (IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) { + err = ubifs_init_authentication_read_only(c); + if (err) + goto out_free; + } else { + ubifs_err(c, "auth_hash_name, but UBIFS is built without" + " authentication support"); + err = -EINVAL; + goto out_free; + } } err = ubifs_read_superblock(c); @@ -1383,7 +1394,7 @@ static int mount_ubifs(struct ubifs_info *c) * 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)) { + if (c->authenticated_rw && ubifs_hmac_zero(c, c->sup_node->hmac)) { err = ubifs_hmac_wkm(c, c->sup_node->hmac_wkm); if (err) goto out_lpt; @@ -1430,7 +1441,7 @@ static int mount_ubifs(struct ubifs_info *c) } if (c->need_recovery) { - if (!ubifs_authenticated(c)) { + if (!ubifs_authenticated_write(c)) { err = ubifs_recover_size(c, true); if (err) goto out_orphans; @@ -1440,7 +1451,7 @@ static int mount_ubifs(struct ubifs_info *c) if (err) goto out_orphans; - if (ubifs_authenticated(c)) { + if (ubifs_authenticated_write(c)) { err = ubifs_recover_size(c, false); if (err) goto out_orphans; @@ -1686,7 +1697,7 @@ static int ubifs_remount_rw(struct ubifs_info *c) err = ubifs_write_rcvrd_mst_node(c); if (err) goto out; - if (!ubifs_authenticated(c)) { + if (!ubifs_authenticated_write(c)) { err = ubifs_recover_size(c, true); if (err) goto out; @@ -1771,7 +1782,7 @@ static int ubifs_remount_rw(struct ubifs_info *c) if (err) goto out; - if (ubifs_authenticated(c)) { + if (ubifs_authenticated_write(c)) { err = ubifs_recover_size(c, false); if (err) goto out; diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index bff682309fbe..0789c859a148 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1040,7 +1040,8 @@ struct ubifs_debug_info; * @default_compr: default compression algorithm (%UBIFS_COMPR_LZO, etc) * @rw_incompat: the media is not R/W compatible * @assert_action: action to take when a ubifs_assert() fails - * @authenticated: flag indigating the FS is mounted in authenticated mode + * @authenticated_rw: flag indigating the FS is mounted in authenticated mode + * @authenticated_ro: flag indigating the FS is mounted in read-only authenticated mode (without HMAC) * * @tnc_mutex: protects the Tree Node Cache (TNC), @zroot, @cnext, @enext, and * @calc_idx_sz @@ -1293,7 +1294,8 @@ struct ubifs_info { unsigned int default_compr:2; unsigned int rw_incompat:1; unsigned int assert_action:2; - unsigned int authenticated:1; + unsigned int authenticated_rw:1; + unsigned int authenticated_ro:1; unsigned int superblock_need_write:1; struct mutex tnc_mutex; @@ -1506,21 +1508,34 @@ extern const struct inode_operations ubifs_symlink_inode_operations; extern struct ubifs_compressor *ubifs_compressors[UBIFS_COMPR_TYPES_CNT]; /* auth.c */ -static inline int ubifs_authenticated(const struct ubifs_info *c) +static inline int ubifs_authenticated_read(const struct ubifs_info *c) { - return (IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) && c->authenticated; + return (IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) && (c->authenticated_rw || c->authenticated_ro); +} + +static inline int ubifs_authenticated_write(const struct ubifs_info *c) +{ + /* + * Lets make sure, that ubifs does not try to write + * when authenticate_ro is active. + * + * Because this will certeinly be an error. + */ + WARN_ON(c->authenticated_ro && !c->authenticated_rw); + + return (IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) && c->authenticated_rw; } struct shash_desc *__ubifs_hash_get_desc(const struct ubifs_info *c); static inline struct shash_desc *ubifs_hash_get_desc(const struct ubifs_info *c) { - return ubifs_authenticated(c) ? __ubifs_hash_get_desc(c) : NULL; + return ubifs_authenticated_read(c) ? __ubifs_hash_get_desc(c) : NULL; } static inline int ubifs_shash_init(const struct ubifs_info *c, struct shash_desc *desc) { - if (ubifs_authenticated(c)) + if (ubifs_authenticated_read(c)) return crypto_shash_init(desc); else return 0; @@ -1532,7 +1547,7 @@ static inline int ubifs_shash_update(const struct ubifs_info *c, { int err = 0; - if (ubifs_authenticated(c)) { + if (ubifs_authenticated_read(c)) { err = crypto_shash_update(desc, buf, len); if (err < 0) return err; @@ -1544,7 +1559,7 @@ static inline int ubifs_shash_update(const struct ubifs_info *c, static inline int ubifs_shash_final(const struct ubifs_info *c, struct shash_desc *desc, u8 *out) { - return ubifs_authenticated(c) ? crypto_shash_final(desc, out) : 0; + return ubifs_authenticated_read(c) ? crypto_shash_final(desc, out) : 0; } int __ubifs_node_calc_hash(const struct ubifs_info *c, const void *buf, @@ -1552,7 +1567,7 @@ int __ubifs_node_calc_hash(const struct ubifs_info *c, const void *buf, static inline int ubifs_node_calc_hash(const struct ubifs_info *c, const void *buf, u8 *hash) { - if (ubifs_authenticated(c)) + if (ubifs_authenticated_read(c)) return __ubifs_node_calc_hash(c, buf, hash); else return 0; @@ -1599,17 +1614,18 @@ int __ubifs_node_check_hash(const struct ubifs_info *c, const void *buf, static inline int ubifs_node_check_hash(const struct ubifs_info *c, const void *buf, const u8 *expected) { - if (ubifs_authenticated(c)) + if (ubifs_authenticated_read(c)) return __ubifs_node_check_hash(c, buf, expected); else return 0; } +int ubifs_init_authentication_read_only(struct ubifs_info *c); int ubifs_init_authentication(struct ubifs_info *c); void __ubifs_exit_authentication(struct ubifs_info *c); static inline void ubifs_exit_authentication(struct ubifs_info *c) { - if (ubifs_authenticated(c)) + if (ubifs_authenticated_read(c)) __ubifs_exit_authentication(c); } @@ -1638,7 +1654,7 @@ static inline u8 *ubifs_branch_hash(struct ubifs_info *c, static inline void ubifs_copy_hash(const struct ubifs_info *c, const u8 *from, u8 *to) { - if (ubifs_authenticated(c)) + if (ubifs_authenticated_read(c)) memcpy(to, from, c->hash_len); } @@ -1647,7 +1663,7 @@ int __ubifs_node_insert_hmac(const struct ubifs_info *c, void *buf, static inline int ubifs_node_insert_hmac(const struct ubifs_info *c, void *buf, int len, int ofs_hmac) { - if (ubifs_authenticated(c)) + if (ubifs_authenticated_write(c)) return __ubifs_node_insert_hmac(c, buf, len, ofs_hmac); else return 0; @@ -1658,7 +1674,7 @@ int __ubifs_node_verify_hmac(const struct ubifs_info *c, const void *buf, static inline int ubifs_node_verify_hmac(const struct ubifs_info *c, const void *buf, int len, int ofs_hmac) { - if (ubifs_authenticated(c)) + if (ubifs_authenticated_write(c)) return __ubifs_node_verify_hmac(c, buf, len, ofs_hmac); else return 0; @@ -1674,7 +1690,7 @@ static inline int ubifs_node_verify_hmac(const struct ubifs_info *c, */ static inline int ubifs_auth_node_sz(const struct ubifs_info *c) { - if (ubifs_authenticated(c)) + if (ubifs_authenticated_write(c)) return sizeof(struct ubifs_auth_node) + c->hmac_desc_len; else return 0; @@ -1691,7 +1707,7 @@ static inline int ubifs_shash_copy_state(const struct ubifs_info *c, struct shash_desc *src, struct shash_desc *target) { - if (ubifs_authenticated(c)) + if (ubifs_authenticated_write(c)) return __ubifs_shash_copy_state(c, src, target); else return 0; -- 2.20.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given 2020-06-25 15:59 ` [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given Torben Hohn @ 2020-06-26 4:31 ` Sascha Hauer 2020-06-26 7:27 ` Torben Hohn 0 siblings, 1 reply; 35+ messages in thread From: Sascha Hauer @ 2020-06-26 4:31 UTC (permalink / raw) To: Torben Hohn; +Cc: richard, bigeasy, linux-mtd, tglx Hi Torben, On Thu, Jun 25, 2020 at 05:59:27PM +0200, Torben Hohn wrote: > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c > index 7fc2f3f07c16..ec95f1f50e5e 100644 > --- a/fs/ubifs/super.c > +++ b/fs/ubifs/super.c > @@ -1291,6 +1291,17 @@ static int mount_ubifs(struct ubifs_info *c) > err = -EINVAL; > goto out_free; > } > + } else if (c->auth_hash_name) { > + if (IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) { > + err = ubifs_init_authentication_read_only(c); > + if (err) > + goto out_free; > + } else { > + ubifs_err(c, "auth_hash_name, but UBIFS is built without" > + " authentication support"); > + err = -EINVAL; > + goto out_free; > + } > } In case we don't have a key available for HMAC and can only verify the FS is correctly signed then we have to be sure that we are mounting readonly. This means the above needs an additional check for c->ro_mount. Once we can be sure that UBIFS is in readonly mode when we can't do HMAC then there's no point in adding a ubifs_authenticated_write(), because the places where you call it will never be hit in a readonly mounted filesystem. Regards, Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given 2020-06-26 4:31 ` Sascha Hauer @ 2020-06-26 7:27 ` Torben Hohn 2020-06-26 7:53 ` Richard Weinberger 2020-06-26 8:10 ` Sascha Hauer 0 siblings, 2 replies; 35+ messages in thread From: Torben Hohn @ 2020-06-26 7:27 UTC (permalink / raw) To: Sascha Hauer; +Cc: richard, bigeasy, linux-mtd, tglx On Fri, Jun 26, 2020 at 06:31:20AM +0200, Sascha Hauer wrote: > Hi Torben, > > On Thu, Jun 25, 2020 at 05:59:27PM +0200, Torben Hohn wrote: > > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c > > index 7fc2f3f07c16..ec95f1f50e5e 100644 > > --- a/fs/ubifs/super.c > > +++ b/fs/ubifs/super.c > > @@ -1291,6 +1291,17 @@ static int mount_ubifs(struct ubifs_info *c) > > err = -EINVAL; > > goto out_free; > > } > > + } else if (c->auth_hash_name) { > > + if (IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) { > > + err = ubifs_init_authentication_read_only(c); > > + if (err) > > + goto out_free; > > + } else { > > + ubifs_err(c, "auth_hash_name, but UBIFS is built without" > > + " authentication support"); > > + err = -EINVAL; > > + goto out_free; > > + } > > } > > In case we don't have a key available for HMAC and can only verify the > FS is correctly signed then we have to be sure that we are mounting > readonly. This means the above needs an additional check for > c->ro_mount. Indeed, i had that check in authenticate_sb_node() in an earlier version, and forgot to add it here. Will do. > > Once we can be sure that UBIFS is in readonly mode when we can't do HMAC > then there's no point in adding a ubifs_authenticated_write(), because > the places where you call it will never be hit in a readonly mounted > filesystem. The point is making sure, that it really is never hit in a readonly filesystem. Now, and in the future. If we miss one point, we might trigger the hmac code with an empty hmac. Although it might just crash. Maybe we could generate a random key, to lessen the impact of such an error. But i doubt that i have enough entropy to make that more than a fig leaf. Another topic: Richard raised the point on irc, that in a dirty filesystem, the journal would be replayed even in ro mode. i dont see that happening. -------------------------------------------------------------------------- static int mount_ubifs(struct ubifs_info *c) { [...] if (!c->ro_mount) { [...] } else if (c->need_recovery) { err = ubifs_recover_size(c, false); if (err) goto out_orphans; } else { /* * Even if we mount read-only, we have to set space in GC LEB * to proper value because this affects UBIFS free space * reporting. We do not want to have a situation when * re-mounting from R/O to R/W changes amount of free space. */ err = take_gc_lnum(c); if (err) goto out_orphans; } [...] } -------------------------------------------------------------------------- ubifs_recover_size() with in_place = false, does not seem to write. > > Regards, > Sascha > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- Torben Hohn Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99 Hinweise zum Datenschutz finden Sie hier (Informations on data privacy can be found here): https://linutronix.de/kontakt/Datenschutz.php Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen | Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given 2020-06-26 7:27 ` Torben Hohn @ 2020-06-26 7:53 ` Richard Weinberger 2020-06-26 8:10 ` Sascha Hauer 1 sibling, 0 replies; 35+ messages in thread From: Richard Weinberger @ 2020-06-26 7:53 UTC (permalink / raw) To: Torben Hohn; +Cc: linux-mtd, Sascha Hauer, tglx, bigeasy ----- Ursprüngliche Mail ----- > Another topic: > > Richard raised the point on irc, that in a dirty filesystem, the journal > would be replayed even in ro mode. > > i dont see that happening. I think there is a confusion. Replay is not the same as recovery. Of course it does not recover, this needs writing. But it has to replay the journal, otherwise the index tree can point to already garbage collected LEBs. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given 2020-06-26 7:27 ` Torben Hohn 2020-06-26 7:53 ` Richard Weinberger @ 2020-06-26 8:10 ` Sascha Hauer 2020-06-26 9:39 ` Torben Hohn 1 sibling, 1 reply; 35+ messages in thread From: Sascha Hauer @ 2020-06-26 8:10 UTC (permalink / raw) To: Torben Hohn; +Cc: richard, bigeasy, linux-mtd, tglx On Fri, Jun 26, 2020 at 09:27:14AM +0200, Torben Hohn wrote: > On Fri, Jun 26, 2020 at 06:31:20AM +0200, Sascha Hauer wrote: > > Hi Torben, > > > > On Thu, Jun 25, 2020 at 05:59:27PM +0200, Torben Hohn wrote: > > > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c > > > index 7fc2f3f07c16..ec95f1f50e5e 100644 > > > --- a/fs/ubifs/super.c > > > +++ b/fs/ubifs/super.c > > > @@ -1291,6 +1291,17 @@ static int mount_ubifs(struct ubifs_info *c) > > > err = -EINVAL; > > > goto out_free; > > > } > > > + } else if (c->auth_hash_name) { > > > + if (IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) { > > > + err = ubifs_init_authentication_read_only(c); > > > + if (err) > > > + goto out_free; > > > + } else { > > > + ubifs_err(c, "auth_hash_name, but UBIFS is built without" > > > + " authentication support"); > > > + err = -EINVAL; > > > + goto out_free; > > > + } > > > } > > > > In case we don't have a key available for HMAC and can only verify the > > FS is correctly signed then we have to be sure that we are mounting > > readonly. This means the above needs an additional check for > > c->ro_mount. > > Indeed, i had that check in authenticate_sb_node() in an earlier > version, and forgot to add it here. > > Will do. > > > > > Once we can be sure that UBIFS is in readonly mode when we can't do HMAC > > then there's no point in adding a ubifs_authenticated_write(), because > > the places where you call it will never be hit in a readonly mounted > > filesystem. > > The point is making sure, that it really is never hit in a readonly > filesystem. Now, and in the future. If we miss one point, we might > trigger the hmac code with an empty hmac. Although it might just crash. If that's your point then you can add a ubifs_assert(c, c->ro_mount) at those places. This has the advantage that it triggers not only in authenticated mode, but also in unauthenticated mode. Please add this assertion explicitly and not indirectly in ubifs_authenticated_write(). This function has a strange semantics, the name suggests that it returns the status of authenticated write. It's quite unexpected to me that it triggers a warning when called with only readonly authentication available. Regards, Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given 2020-06-26 8:10 ` Sascha Hauer @ 2020-06-26 9:39 ` Torben Hohn 0 siblings, 0 replies; 35+ messages in thread From: Torben Hohn @ 2020-06-26 9:39 UTC (permalink / raw) To: Sascha Hauer; +Cc: richard, bigeasy, linux-mtd, tglx On Fri, Jun 26, 2020 at 10:10:28AM +0200, Sascha Hauer wrote: > On Fri, Jun 26, 2020 at 09:27:14AM +0200, Torben Hohn wrote: > > On Fri, Jun 26, 2020 at 06:31:20AM +0200, Sascha Hauer wrote: > > > Hi Torben, > > > > > > On Thu, Jun 25, 2020 at 05:59:27PM +0200, Torben Hohn wrote: > > > > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c > > > > index 7fc2f3f07c16..ec95f1f50e5e 100644 > > > > --- a/fs/ubifs/super.c > > > > +++ b/fs/ubifs/super.c > > > > @@ -1291,6 +1291,17 @@ static int mount_ubifs(struct ubifs_info *c) > > > > err = -EINVAL; > > > > goto out_free; > > > > } > > > > + } else if (c->auth_hash_name) { > > > > + if (IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) { > > > > + err = ubifs_init_authentication_read_only(c); > > > > + if (err) > > > > + goto out_free; > > > > + } else { > > > > + ubifs_err(c, "auth_hash_name, but UBIFS is built without" > > > > + " authentication support"); > > > > + err = -EINVAL; > > > > + goto out_free; > > > > + } > > > > } > > > > > > In case we don't have a key available for HMAC and can only verify the > > > FS is correctly signed then we have to be sure that we are mounting > > > readonly. This means the above needs an additional check for > > > c->ro_mount. > > > > Indeed, i had that check in authenticate_sb_node() in an earlier > > version, and forgot to add it here. > > > > Will do. > > > > > > > > Once we can be sure that UBIFS is in readonly mode when we can't do HMAC > > > then there's no point in adding a ubifs_authenticated_write(), because > > > the places where you call it will never be hit in a readonly mounted > > > filesystem. > > > > The point is making sure, that it really is never hit in a readonly > > filesystem. Now, and in the future. If we miss one point, we might > > trigger the hmac code with an empty hmac. Although it might just crash. > > If that's your point then you can add a ubifs_assert(c, c->ro_mount) at > those places. This has the advantage that it triggers not only in > authenticated mode, but also in unauthenticated mode. Please add this > assertion explicitly and not indirectly in ubifs_authenticated_write(). > This function has a strange semantics, the name suggests that it returns > the status of authenticated write. It's quite unexpected to me that it > triggers a warning when called with only readonly authentication > available. Ok. will do. > > Regards, > Sascha > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- Torben Hohn Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99 Hinweise zum Datenschutz finden Sie hier (Informations on data privacy can be found here): https://linutronix.de/kontakt/Datenschutz.php Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen | Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 0/1] ubifs: support authentication without hmac 2020-06-25 15:59 [PATCH 0/1] ubifs: support authentication without hmac Torben Hohn 2020-06-25 15:59 ` [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given Torben Hohn @ 2020-06-26 8:09 ` Richard Weinberger 2020-06-29 6:46 ` Alexander Dahl 2020-06-26 11:29 ` [PATCH v2 0/4] " Torben Hohn 2 siblings, 1 reply; 35+ messages in thread From: Richard Weinberger @ 2020-06-26 8:09 UTC (permalink / raw) To: Torben Hohn; +Cc: bigeasy, linux-mtd, tglx, Sascha Hauer Torben, ----- Ursprüngliche Mail ----- > Von: "Torben Hohn" <torben.hohn@linutronix.de> > An: "richard" <richard@nod.at> > CC: "bigeasy" <bigeasy@linutronix.de>, "tglx" <tglx@linutronix.de>, "linux-mtd" <linux-mtd@lists.infradead.org>, "Sascha > Hauer" <s.hauer@pengutronix.de> > Gesendet: Donnerstag, 25. Juni 2020 17:59:26 > Betreff: [PATCH 0/1] ubifs: support authentication without hmac > This patch adds support for ubifs authentication without HMAC, > which obviously only works for a read-only mount. > > ubiblock and dm-verity are not supported by u-boot, and > the kernel on the target is loaded by u-boot out of the RFS. As I said on IRC yesterday. There is a bug with UBIFS versioning. u-boot is not supposed to read from authenticated UBIFS since it has no authentication support at all an might trip over changed data structures. We forgot to raise the UBIFS version to w5r1 for authenticated images and only introduced a new feature flag. This causes old UBIFS implementations like u-boot's to not enforce the super block feature flag field. Before w4 feature flags didn't get enfocred. :-( Patches for mkfs.ubifs and kernel are on their way, I just need to carefully test them with many different old images, u-boot and kernel combinations. I think it is high noon that u-boot updates their UBIFS, then a non-authenticated should be trivial. Of course you need to verify all files you read from it manually then. > This is a first try to implement this. > It boots fine, and the WARN_ON is not triggered. > > I plan to update the docs also, but i would like to have > some positive comments on this before. I think this is a useful feature, please give me a few day to think about all implications. That said, I'm not really a fan of reading files from UBIFS in u-boot. We have UBI static volumes for that... Maybe you can give this a try and void some complexity in the bootloader. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 0/1] ubifs: support authentication without hmac 2020-06-26 8:09 ` [PATCH 0/1] ubifs: support authentication without hmac Richard Weinberger @ 2020-06-29 6:46 ` Alexander Dahl 2020-06-29 7:04 ` Richard Weinberger 0 siblings, 1 reply; 35+ messages in thread From: Alexander Dahl @ 2020-06-29 6:46 UTC (permalink / raw) To: linux-mtd, Sascha Hauer; +Cc: Richard Weinberger, Torben Hohn Hello Richard, Am Freitag, 26. Juni 2020, 10:09:14 CEST schrieb Richard Weinberger: > That said, I'm not really a fan of reading files from UBIFS in u-boot. > We have UBI static volumes for that... > Maybe you can give this a try and void some complexity in the bootloader. Surprised to hear that. I saw some boards lately doing exactly that. Things like kernel image are copied in Linux to e.g. /boot and U-Boot just picks it up from there. I tried to find out more about using UBI static volumes in general and for that usecase, but neither http://linux-mtd.infradead.org/doc/ubi.html nor http://linux-mtd.infradead.org/faq/ubi.html point me in a direction where to find real world examples to look at. Are there any boards in the wild using this? Maybe with defconfigs in U-Boot already? Greets Alex ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 0/1] ubifs: support authentication without hmac 2020-06-29 6:46 ` Alexander Dahl @ 2020-06-29 7:04 ` Richard Weinberger 2020-06-29 7:48 ` Wolfgang Denk 2020-06-30 13:36 ` Richard Weinberger 0 siblings, 2 replies; 35+ messages in thread From: Richard Weinberger @ 2020-06-29 7:04 UTC (permalink / raw) To: Alexander Dahl; +Cc: Sascha Hauer, linux-mtd, Torben Hohn Alex, ----- Ursprüngliche Mail ----- > Von: "Alexander Dahl" <ada@thorsis.com> > An: "linux-mtd" <linux-mtd@lists.infradead.org>, "Sascha Hauer" <s.hauer@pengutronix.de> > CC: "richard" <richard@nod.at>, "Torben Hohn" <torben.hohn@linutronix.de> > Gesendet: Montag, 29. Juni 2020 08:46:10 > Betreff: Re: [PATCH 0/1] ubifs: support authentication without hmac > Hello Richard, > > Am Freitag, 26. Juni 2020, 10:09:14 CEST schrieb Richard Weinberger: >> That said, I'm not really a fan of reading files from UBIFS in u-boot. >> We have UBI static volumes for that... >> Maybe you can give this a try and void some complexity in the bootloader. > > Surprised to hear that. I saw some boards lately doing exactly that. Things > like kernel image are copied in Linux to e.g. /boot and U-Boot just picks it > up from there. Well, not everyone is using U-Boot. ;-) > I tried to find out more about using UBI static volumes in general and for > that usecase, but neither http://linux-mtd.infradead.org/doc/ubi.html nor > http://linux-mtd.infradead.org/faq/ubi.html point me in a direction where to > find real world examples to look at. Are there any boards in the wild using > this? Maybe with defconfigs in U-Boot already? U-Boot choose the UBIFS way AFAICT. But if you have your own minimal loader you can read from an UBI static volume with a few lines of C (~200 LoC). This is what I see/use most of the time. Using static volumes in U-Boot is also possible, IIRC just use "ubi read". From http://www.linux-mtd.infradead.org/doc/ubi.html#L_overview: """ There are 2 types of UBI volumes: dynamic volumes and static volumes. Static volumes are read-only and their contents are protected by CRC-32 checksums, while dynamic volumes are read-write and the upper layers (e.g., a file-system) are responsible for ensuring data integrity. Static volumes are typically used for the kernel, initramfs, and dtb. Larger static volumes may incur a significant penalty when opening, as the CRC-32 needs to be calculated at this time. If you are looking to use static volumes for anything besides the kernel, initramfs, or dtb you are likely doing something wrong and would be better off using a dynamic volume instead. """ Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 0/1] ubifs: support authentication without hmac 2020-06-29 7:04 ` Richard Weinberger @ 2020-06-29 7:48 ` Wolfgang Denk 2020-06-29 7:51 ` Richard Weinberger 2020-06-30 13:36 ` Richard Weinberger 1 sibling, 1 reply; 35+ messages in thread From: Wolfgang Denk @ 2020-06-29 7:48 UTC (permalink / raw) To: Richard Weinberger; +Cc: Alexander Dahl, Sascha Hauer, linux-mtd, Torben Hohn Dear Richard, In message <1067478399.71066.1593414287623.JavaMail.zimbra@nod.at> you wrote: > > > I tried to find out more about using UBI static volumes in general and for > > that usecase, but neither http://linux-mtd.infradead.org/doc/ubi.html nor > > http://linux-mtd.infradead.org/faq/ubi.html point me in a direction where to > > find real world examples to look at. Are there any boards in the wild using > > this? Maybe with defconfigs in U-Boot already? > > U-Boot choose the UBIFS way AFAICT. No, U-Boot does not implement any preferences here, i. e. both methods can be used out of the box. It is just the preference of the user who decides which way to go - using the file system seems more flexible to some unsers, while others care about the additional boot time needed to mount the file system or the additional memory footprint needed for the file system code. Your choice. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de "Have you lived in this village all your life?" "No, not yet." ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 0/1] ubifs: support authentication without hmac 2020-06-29 7:48 ` Wolfgang Denk @ 2020-06-29 7:51 ` Richard Weinberger 2020-06-30 5:50 ` Wolfgang Denk 0 siblings, 1 reply; 35+ messages in thread From: Richard Weinberger @ 2020-06-29 7:51 UTC (permalink / raw) To: Wolfgang Denk; +Cc: Alexander Dahl, Sascha Hauer, linux-mtd, Torben Hohn Wolfgang, ----- Ursprüngliche Mail ----- > Von: "Wolfgang Denk" <wd@denx.de> > An: "richard" <richard@nod.at> > CC: "Alexander Dahl" <ada@thorsis.com>, "Sascha Hauer" <s.hauer@pengutronix.de>, "linux-mtd" > <linux-mtd@lists.infradead.org>, "Torben Hohn" <torben.hohn@linutronix.de> > Gesendet: Montag, 29. Juni 2020 09:48:14 > Betreff: Re: [PATCH 0/1] ubifs: support authentication without hmac > Dear Richard, > > In message <1067478399.71066.1593414287623.JavaMail.zimbra@nod.at> you wrote: >> >> > I tried to find out more about using UBI static volumes in general and for >> > that usecase, but neither http://linux-mtd.infradead.org/doc/ubi.html nor >> > http://linux-mtd.infradead.org/faq/ubi.html point me in a direction where to >> > find real world examples to look at. Are there any boards in the wild using >> > this? Maybe with defconfigs in U-Boot already? >> >> U-Boot choose the UBIFS way AFAICT. > > No, U-Boot does not implement any preferences here, i. e. both > methods can be used out of the box. It is just the preference of > the user who decides which way to go - using the file system seems > more flexible to some unsers, while others care about the additional > boot time needed to mount the file system or the additional memory > footprint needed for the file system code. > > Your choice. Good to know! Thanks for pointing this out. I think for most users reading from a filesystem feels more natural since on other storage systems this is the only way to go. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 0/1] ubifs: support authentication without hmac 2020-06-29 7:51 ` Richard Weinberger @ 2020-06-30 5:50 ` Wolfgang Denk 0 siblings, 0 replies; 35+ messages in thread From: Wolfgang Denk @ 2020-06-30 5:50 UTC (permalink / raw) To: Richard Weinberger; +Cc: Alexander Dahl, Sascha Hauer, linux-mtd, Torben Hohn Dear Richard, In message <667429184.71186.1593417085045.JavaMail.zimbra@nod.at> you wrote: > > > No, U-Boot does not implement any preferences here, i. e. both > > methods can be used out of the box. It is just the preference of > > the user who decides which way to go - using the file system seems > > more flexible to some unsers, while others care about the additional > > boot time needed to mount the file system or the additional memory > > footprint needed for the file system code. > > > > Your choice. > > Good to know! Thanks for pointing this out. > > I think for most users reading from a filesystem feels more natural > since on other storage systems this is the only way to go. It's also easier to use a file system for example during development, for example when you often switch between several different kernel images and such. But as mentioned it comes with the penalty of additional overhead, both in memory footprint and boot time, so for the production configuration it makes sense to use a static UBI volume instead. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de "The question of whether a computer can think is no more interesting than the question of whether a submarine can swim" - Edsgar W. Dijkstra ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 0/1] ubifs: support authentication without hmac 2020-06-29 7:04 ` Richard Weinberger 2020-06-29 7:48 ` Wolfgang Denk @ 2020-06-30 13:36 ` Richard Weinberger 2020-06-30 14:36 ` Alexander Dahl 1 sibling, 1 reply; 35+ messages in thread From: Richard Weinberger @ 2020-06-30 13:36 UTC (permalink / raw) To: Richard Weinberger; +Cc: Alexander Dahl, Sascha Hauer, linux-mtd, Torben Hohn Alex, On Mon, Jun 29, 2020 at 9:11 AM Richard Weinberger <richard@nod.at> wrote: > U-Boot choose the UBIFS way AFAICT. > > But if you have your own minimal loader you can read from an UBI static volume with > a few lines of C (~200 LoC). This is what I see/use most of the time. > Using static volumes in U-Boot is also possible, IIRC just use "ubi read". > > From http://www.linux-mtd.infradead.org/doc/ubi.html#L_overview: > """ > There are 2 types of UBI volumes: dynamic volumes and static volumes. Static volumes are read-only and their contents are protected by CRC-32 checksums, while dynamic volumes are read-write and the upper layers (e.g., a file-system) are responsible for ensuring data integrity. > > Static volumes are typically used for the kernel, initramfs, and dtb. Larger static volumes may incur a significant penalty when opening, as the CRC-32 needs to be calculated at this time. If you are looking to use static volumes for anything besides the kernel, initramfs, or dtb you are likely doing something wrong and would be better off using a dynamic volume instead. > """ Did this and Wolfgang's input help? -- Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 0/1] ubifs: support authentication without hmac 2020-06-30 13:36 ` Richard Weinberger @ 2020-06-30 14:36 ` Alexander Dahl 0 siblings, 0 replies; 35+ messages in thread From: Alexander Dahl @ 2020-06-30 14:36 UTC (permalink / raw) To: Richard Weinberger Cc: Richard Weinberger, Sascha Hauer, linux-mtd, Torben Hohn Hello Richard, thanks for asking back. Am Dienstag, 30. Juni 2020, 15:36:28 CEST schrieb Richard Weinberger: > On Mon, Jun 29, 2020 at 9:11 AM Richard Weinberger <richard@nod.at> wrote: > > U-Boot choose the UBIFS way AFAICT. > > > > But if you have your own minimal loader you can read from an UBI static > > volume with a few lines of C (~200 LoC). This is what I see/use most of > > the time. Using static volumes in U-Boot is also possible, IIRC just use > > "ubi read". > > > > From http://www.linux-mtd.infradead.org/doc/ubi.html#L_overview: > > """ > > There are 2 types of UBI volumes: dynamic volumes and static volumes. > > Static volumes are read-only and their contents are protected by CRC-32 > > checksums, while dynamic volumes are read-write and the upper layers > > (e.g., a file-system) are responsible for ensuring data integrity. > > > > Static volumes are typically used for the kernel, initramfs, and dtb. > > Larger static volumes may incur a significant penalty when opening, as > > the CRC-32 needs to be calculated at this time. If you are looking to use > > static volumes for anything besides the kernel, initramfs, or dtb you are > > likely doing something wrong and would be better off using a dynamic > > volume instead. """ > > Did this and Wolfgang's input help? Yes, it did. I see the benefits of static UBI volumes, and I wish we had known this for our projects like 10 years ago. However I also appreciated Wolfgang's input, it's basically what we do with more recent projects. The possibility to easily upgrade kernel image and/or dts speeds up development a lot and causes less headaches on system upgrade. Greets Alex ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 0/4] ubifs: support authentication without hmac 2020-06-25 15:59 [PATCH 0/1] ubifs: support authentication without hmac Torben Hohn 2020-06-25 15:59 ` [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given Torben Hohn 2020-06-26 8:09 ` [PATCH 0/1] ubifs: support authentication without hmac Richard Weinberger @ 2020-06-26 11:29 ` Torben Hohn 2020-06-26 11:29 ` [PATCH v2 1/4] ubifs: move #include "debug.h" above auth.c Torben Hohn ` (4 more replies) 2 siblings, 5 replies; 35+ messages in thread From: Torben Hohn @ 2020-06-26 11:29 UTC (permalink / raw) To: richard; +Cc: bigeasy, linux-mtd, tglx, s.hauer This PQ adds support for ubifs authentication without HMAC, which obviously only works for a read-only mount. ubiblock and dm-verity are not supported by u-boot, and the kernel on the target is loaded by u-boot out of the RFS. This is a first try to implement this. It boots fine, and the WARN_ON is not triggered. I plan to update the docs also, but i would like to have some positive comments on this before. Changes since v1: - apply comments from Sascha an revert the ubifs_authicated_(read|write) stuff. Use ubifs_assert(c, !c->ro_mount) instead. - Prevent remount rw, when hmac-less authentication is used - add missing check, for ro mode, when no auth_key_name is specified. Torben Hohn (4): ubifs: move #include "debug.h" above auth.c ubifs: support authentication, for ro mount, when no key is given ubifs: sprinkle ubifs_assert(c, !c->ro_mount) in hmac auth ubifs: prevent remounting rw when no hmac key was given fs/ubifs/auth.c | 62 +++++++++++++++++++++++++++++++++++++++++++++- fs/ubifs/gc.c | 1 + fs/ubifs/journal.c | 8 ++++++ fs/ubifs/replay.c | 1 + fs/ubifs/sb.c | 5 ++++ fs/ubifs/super.c | 28 ++++++++++++++++++++- fs/ubifs/ubifs.h | 12 ++++++++- 7 files changed, 114 insertions(+), 3 deletions(-) -- 2.20.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 1/4] ubifs: move #include "debug.h" above auth.c 2020-06-26 11:29 ` [PATCH v2 0/4] " Torben Hohn @ 2020-06-26 11:29 ` Torben Hohn 2020-06-26 11:29 ` [PATCH v2 2/4] ubifs: support authentication, for ro mount, when no key is given Torben Hohn ` (3 subsequent siblings) 4 siblings, 0 replies; 35+ messages in thread From: Torben Hohn @ 2020-06-26 11:29 UTC (permalink / raw) To: richard; +Cc: bigeasy, linux-mtd, tglx, s.hauer prepare to use ubifs_assert() in some inline functions from the auth.c block. move #include "debug.h" above the auth.c block. Signed-off-by: Torben Hohn <torben.hohn@linutronix.de> --- fs/ubifs/ubifs.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index bff682309fbe..95ed45022e51 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1505,6 +1505,8 @@ extern const struct inode_operations ubifs_dir_inode_operations; extern const struct inode_operations ubifs_symlink_inode_operations; extern struct ubifs_compressor *ubifs_compressors[UBIFS_COMPR_TYPES_CNT]; +#include "debug.h" + /* auth.c */ static inline int ubifs_authenticated(const struct ubifs_info *c) { @@ -2063,7 +2065,6 @@ void ubifs_compress(const struct ubifs_info *c, const void *in_buf, int in_len, int ubifs_decompress(const struct ubifs_info *c, const void *buf, int len, void *out, int *out_len, int compr_type); -#include "debug.h" #include "misc.h" #include "key.h" -- 2.20.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 2/4] ubifs: support authentication, for ro mount, when no key is given 2020-06-26 11:29 ` [PATCH v2 0/4] " Torben Hohn 2020-06-26 11:29 ` [PATCH v2 1/4] ubifs: move #include "debug.h" above auth.c Torben Hohn @ 2020-06-26 11:29 ` Torben Hohn 2020-06-26 11:29 ` [PATCH v2 3/4] ubifs: sprinkle ubifs_assert(c, !c->ro_mount) in hmac auth Torben Hohn ` (2 subsequent siblings) 4 siblings, 0 replies; 35+ messages in thread From: Torben Hohn @ 2020-06-26 11:29 UTC (permalink / raw) To: richard; +Cc: bigeasy, linux-mtd, tglx, s.hauer Ubifs authentication requires a hmac key, even when a filesystem is mounted read-only. Implement ubifs_init_authentication_read_only(), which only allocates the structures needed for validating the hashes. Call ubifs_init_authentication_read_only() when no auth_key_name is specified, and the filesystem is to be mounted read only. Fixup __ubifs_exit_authentication() to free c->hmac_tfm only when !c->ro_mount. Signed-off-by: Torben Hohn <torben.hohn@linutronix.de> --- fs/ubifs/auth.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++- fs/ubifs/sb.c | 4 ++++ fs/ubifs/super.c | 19 ++++++++++++++- fs/ubifs/ubifs.h | 1 + 4 files changed, 83 insertions(+), 2 deletions(-) diff --git a/fs/ubifs/auth.c b/fs/ubifs/auth.c index cc5c0abfd536..52ce7a2218a5 100644 --- a/fs/ubifs/auth.c +++ b/fs/ubifs/auth.c @@ -248,6 +248,61 @@ int ubifs_sb_verify_signature(struct ubifs_info *c, return err; } +/** + * ubifs_init_authentication_read_only - init only the read_only parts + * + * @c: UBIFS file-system description object + * + * This function returns 0 for success or a negative error code otherwise. + */ + +int ubifs_init_authentication_read_only(struct ubifs_info *c) +{ + int err; + + if (!c->auth_hash_name) { + ubifs_err(c, "authentication hash name needed with authentication"); + return -EINVAL; + } + + c->auth_hash_algo = match_string(hash_algo_name, HASH_ALGO__LAST, + c->auth_hash_name); + if ((int)c->auth_hash_algo < 0) { + ubifs_err(c, "Unknown hash algo %s specified", + c->auth_hash_name); + return -EINVAL; + } + + c->hash_tfm = crypto_alloc_shash(c->auth_hash_name, 0, 0); + if (IS_ERR(c->hash_tfm)) { + err = PTR_ERR(c->hash_tfm); + ubifs_err(c, "Can not allocate %s: %d", + c->auth_hash_name, err); + goto out; + } + + c->hash_len = crypto_shash_digestsize(c->hash_tfm); + if (c->hash_len > UBIFS_HASH_ARR_SZ) { + ubifs_err(c, "hash %s is bigger than maximum allowed hash size (%d > %d)", + c->auth_hash_name, c->hash_len, UBIFS_HASH_ARR_SZ); + err = -EINVAL; + goto out_free_hash; + } + + c->authenticated = true; + + c->log_hash = ubifs_hash_get_desc(c); + if (IS_ERR(c->log_hash)) + goto out_free_hash; + + err = 0; +out_free_hash: + if (err) + crypto_free_shash(c->hash_tfm); +out: + return err; +} + /** * ubifs_init_authentication - initialize UBIFS authentication support * @c: UBIFS file-system description object @@ -367,9 +422,13 @@ void __ubifs_exit_authentication(struct ubifs_info *c) if (!ubifs_authenticated(c)) return; - crypto_free_shash(c->hmac_tfm); crypto_free_shash(c->hash_tfm); kfree(c->log_hash); + + if (c->ro_mount) + return; + + crypto_free_shash(c->hmac_tfm); } /** diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c index 4b4b65b48c57..d898ea5edd7c 100644 --- a/fs/ubifs/sb.c +++ b/fs/ubifs/sb.c @@ -583,6 +583,10 @@ static int authenticate_sb_node(struct ubifs_info *c, if (ubifs_hmac_zero(c, sup->hmac)) { err = ubifs_sb_verify_signature(c, sup); } else { + if (!c->hmac_tfm) { + ubifs_err(c, "HMAC authenticated FS found, but no key given"); + return -EINVAL; + } err = ubifs_hmac_wkm(c, hmac_wkm); if (err) return err; diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 7fc2f3f07c16..13175da14464 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1291,6 +1291,23 @@ static int mount_ubifs(struct ubifs_info *c) err = -EINVAL; goto out_free; } + } else if (c->auth_hash_name) { + if (!c->ro_mount) { + ubifs_err(c, "auth_hash_name without auth_key_name, but no ro mount"); + err = -EINVAL; + goto out_free; + } + + if (IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) { + err = ubifs_init_authentication_read_only(c); + if (err) + goto out_free; + } else { + ubifs_err(c, "auth_hash_name, but UBIFS is built without" + " authentication support"); + err = -EINVAL; + goto out_free; + } } err = ubifs_read_superblock(c); @@ -1383,7 +1400,7 @@ static int mount_ubifs(struct ubifs_info *c) * 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)) { + if (!c->ro_mount && c->authenticated && ubifs_hmac_zero(c, c->sup_node->hmac)) { err = ubifs_hmac_wkm(c, c->sup_node->hmac_wkm); if (err) goto out_lpt; diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 95ed45022e51..80e2800927ec 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1607,6 +1607,7 @@ static inline int ubifs_node_check_hash(const struct ubifs_info *c, return 0; } +int ubifs_init_authentication_read_only(struct ubifs_info *c); int ubifs_init_authentication(struct ubifs_info *c); void __ubifs_exit_authentication(struct ubifs_info *c); static inline void ubifs_exit_authentication(struct ubifs_info *c) -- 2.20.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 3/4] ubifs: sprinkle ubifs_assert(c, !c->ro_mount) in hmac auth 2020-06-26 11:29 ` [PATCH v2 0/4] " Torben Hohn 2020-06-26 11:29 ` [PATCH v2 1/4] ubifs: move #include "debug.h" above auth.c Torben Hohn 2020-06-26 11:29 ` [PATCH v2 2/4] ubifs: support authentication, for ro mount, when no key is given Torben Hohn @ 2020-06-26 11:29 ` Torben Hohn 2020-06-26 11:29 ` [PATCH v2 4/4] ubifs: prevent remounting rw when no hmac key was given Torben Hohn 2020-06-26 14:16 ` [PATCH v2 0/4] ubifs: support authentication without hmac Richard Weinberger 4 siblings, 0 replies; 35+ messages in thread From: Torben Hohn @ 2020-06-26 11:29 UTC (permalink / raw) To: richard; +Cc: bigeasy, linux-mtd, tglx, s.hauer make sure that the hmac ubifs authentication code, which uses the hmac is only called, for rw mounts. This allows to ensure that we are not in the new read only authentication mode, when the hmac is accessed. Signed-off-by: Torben Hohn <torben.hohn@linutronix.de> --- fs/ubifs/auth.c | 1 + fs/ubifs/gc.c | 1 + fs/ubifs/journal.c | 8 ++++++++ fs/ubifs/replay.c | 1 + fs/ubifs/sb.c | 1 + fs/ubifs/super.c | 5 +++++ fs/ubifs/ubifs.h | 8 ++++++++ 7 files changed, 25 insertions(+) diff --git a/fs/ubifs/auth.c b/fs/ubifs/auth.c index 52ce7a2218a5..15a1a18a42ed 100644 --- a/fs/ubifs/auth.c +++ b/fs/ubifs/auth.c @@ -570,6 +570,7 @@ int ubifs_hmac_wkm(struct ubifs_info *c, u8 *hmac) int err; const char well_known_message[] = "UBIFS"; + ubifs_assert(c, !c->ro_mount); if (!ubifs_authenticated(c)) return 0; diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c index 62cb3db44e6e..8009ceb362d2 100644 --- a/fs/ubifs/gc.c +++ b/fs/ubifs/gc.c @@ -410,6 +410,7 @@ static int move_nodes(struct ubifs_info *c, struct ubifs_scan_leb *sleb) moved = 1; } + ubifs_assert(c, !c->ro_mount); if (ubifs_authenticated(c) && moved) { struct ubifs_auth_node *auth; diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index e5ec1afe1c66..5c23846f2d40 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -80,6 +80,7 @@ static inline void zero_trun_node_unused(struct ubifs_trun_node *trun) static void ubifs_add_auth_dirt(struct ubifs_info *c, int lnum) { + ubifs_assert(c, !c->ro_mount); if (ubifs_authenticated(c)) ubifs_add_dirt(c, lnum, ubifs_auth_node_sz(c)); } @@ -278,7 +279,9 @@ static int write_head(struct ubifs_info *c, int jhead, void *buf, int len, dbg_jnl("jhead %s, LEB %d:%d, len %d", dbg_jhead(jhead), *lnum, *offs, len); + ubifs_assert(c, !c->ro_mount); if (ubifs_authenticated(c)) { + err = ubifs_hash_nodes(c, buf, len, c->jheads[jhead].log_hash); if (err) return err; @@ -572,6 +575,7 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir, len = aligned_dlen + aligned_ilen + UBIFS_INO_NODE_SZ; /* Make sure to also account for extended attributes */ + ubifs_assert(c, !c->ro_mount); if (ubifs_authenticated(c)) len += ALIGN(host_ui->data_len, 8) + ubifs_auth_node_sz(c); else @@ -778,6 +782,8 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode, } dlen = UBIFS_DATA_NODE_SZ + out_len; + + ubifs_assert(c, !c->ro_mount); if (ubifs_authenticated(c)) write_len = ALIGN(dlen, 8) + auth_len; else @@ -860,6 +866,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) write_len += UBIFS_INO_NODE_SZ * ui->xattr_cnt; } + ubifs_assert(c, !c->ro_mount); if (ubifs_authenticated(c)) write_len += ALIGN(ilen, 8) + ubifs_auth_node_sz(c); else @@ -1572,6 +1579,7 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode, /* Must make reservation before allocating sequence numbers */ len = UBIFS_TRUN_NODE_SZ + UBIFS_INO_NODE_SZ; + ubifs_assert(c, !c->ro_mount); if (ubifs_authenticated(c)) len += ALIGN(dlen, 8) + ubifs_auth_node_sz(c); else diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c index b69ffac7e415..a3a37c196f56 100644 --- a/fs/ubifs/replay.c +++ b/fs/ubifs/replay.c @@ -595,6 +595,7 @@ static int authenticate_sleb(struct ubifs_info *c, struct ubifs_scan_leb *sleb, u8 hash[UBIFS_HASH_ARR_SZ]; u8 hmac[UBIFS_HMAC_ARR_SZ]; + ubifs_assert(c, !c->ro_mount); if (!ubifs_authenticated(c)) return sleb->nodes_cnt; diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c index d898ea5edd7c..c2104235fb56 100644 --- a/fs/ubifs/sb.c +++ b/fs/ubifs/sb.c @@ -176,6 +176,7 @@ static int create_default_filesystem(struct ubifs_info *c) sup_flags |= UBIFS_FLG_BIGLPT; sup_flags |= UBIFS_FLG_DOUBLE_HASH; + ubifs_assert(c, !c->ro_mount); if (ubifs_authenticated(c)) { sup_flags |= UBIFS_FLG_AUTHENTICATION; sup->hash_algo = cpu_to_le16(c->auth_hash_algo); diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 13175da14464..b41ca9ee5763 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1447,6 +1447,7 @@ static int mount_ubifs(struct ubifs_info *c) } if (c->need_recovery) { + ubifs_assert(c, !c->ro_mount); if (!ubifs_authenticated(c)) { err = ubifs_recover_size(c, true); if (err) @@ -1457,6 +1458,7 @@ static int mount_ubifs(struct ubifs_info *c) if (err) goto out_orphans; + ubifs_assert(c, !c->ro_mount); if (ubifs_authenticated(c)) { err = ubifs_recover_size(c, false); if (err) @@ -1703,6 +1705,8 @@ static int ubifs_remount_rw(struct ubifs_info *c) err = ubifs_write_rcvrd_mst_node(c); if (err) goto out; + + ubifs_assert(c, !c->ro_mount); if (!ubifs_authenticated(c)) { err = ubifs_recover_size(c, true); if (err) @@ -1788,6 +1792,7 @@ static int ubifs_remount_rw(struct ubifs_info *c) if (err) goto out; + ubifs_assert(c, !c->ro_mount); if (ubifs_authenticated(c)) { err = ubifs_recover_size(c, false); if (err) diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 80e2800927ec..79880cf0e1a0 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1650,6 +1650,8 @@ int __ubifs_node_insert_hmac(const struct ubifs_info *c, void *buf, static inline int ubifs_node_insert_hmac(const struct ubifs_info *c, void *buf, int len, int ofs_hmac) { + ubifs_assert(c, !c->ro_mount); + if (ubifs_authenticated(c)) return __ubifs_node_insert_hmac(c, buf, len, ofs_hmac); else @@ -1661,6 +1663,8 @@ int __ubifs_node_verify_hmac(const struct ubifs_info *c, const void *buf, static inline int ubifs_node_verify_hmac(const struct ubifs_info *c, const void *buf, int len, int ofs_hmac) { + ubifs_assert(c, !c->ro_mount); + if (ubifs_authenticated(c)) return __ubifs_node_verify_hmac(c, buf, len, ofs_hmac); else @@ -1677,6 +1681,8 @@ static inline int ubifs_node_verify_hmac(const struct ubifs_info *c, */ static inline int ubifs_auth_node_sz(const struct ubifs_info *c) { + ubifs_assert(c, !c->ro_mount); + if (ubifs_authenticated(c)) return sizeof(struct ubifs_auth_node) + c->hmac_desc_len; else @@ -1694,6 +1700,8 @@ static inline int ubifs_shash_copy_state(const struct ubifs_info *c, struct shash_desc *src, struct shash_desc *target) { + ubifs_assert(c, !c->ro_mount); + if (ubifs_authenticated(c)) return __ubifs_shash_copy_state(c, src, target); else -- 2.20.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 4/4] ubifs: prevent remounting rw when no hmac key was given 2020-06-26 11:29 ` [PATCH v2 0/4] " Torben Hohn ` (2 preceding siblings ...) 2020-06-26 11:29 ` [PATCH v2 3/4] ubifs: sprinkle ubifs_assert(c, !c->ro_mount) in hmac auth Torben Hohn @ 2020-06-26 11:29 ` Torben Hohn 2020-06-26 12:27 ` Richard Weinberger 2020-06-26 14:16 ` [PATCH v2 0/4] ubifs: support authentication without hmac Richard Weinberger 4 siblings, 1 reply; 35+ messages in thread From: Torben Hohn @ 2020-06-26 11:29 UTC (permalink / raw) To: richard; +Cc: bigeasy, linux-mtd, tglx, s.hauer After adding readonly hmac-less authentication support, prevent remounting the filesystem in rw mode, when the hmac is not available. Signed-off-by: Torben Hohn <torben.hohn@linutronix.de> --- fs/ubifs/super.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index b41ca9ee5763..62bdef8f1ddf 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1996,6 +1996,10 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data) ubifs_msg(c, "cannot re-mount R/W - UBI volume is R/O"); return -EROFS; } + if (ubifs_authenticated(c) && !c->hash_tfm) { + ubifs_msg(c, "cannot re-mount R/W due to missing hmac key, for authentication"); + return -EROFS; + } err = ubifs_remount_rw(c); if (err) return err; -- 2.20.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v2 4/4] ubifs: prevent remounting rw when no hmac key was given 2020-06-26 11:29 ` [PATCH v2 4/4] ubifs: prevent remounting rw when no hmac key was given Torben Hohn @ 2020-06-26 12:27 ` Richard Weinberger 2020-06-29 8:53 ` Torben Hohn 0 siblings, 1 reply; 35+ messages in thread From: Richard Weinberger @ 2020-06-26 12:27 UTC (permalink / raw) To: Torben Hohn; +Cc: bigeasy, linux-mtd, tglx, Sascha Hauer ----- Ursprüngliche Mail ----- > Von: "Torben Hohn" <torben.hohn@linutronix.de> > An: "richard" <richard@nod.at> > CC: "bigeasy" <bigeasy@linutronix.de>, "tglx" <tglx@linutronix.de>, "linux-mtd" <linux-mtd@lists.infradead.org>, "Sascha > Hauer" <s.hauer@pengutronix.de> > Gesendet: Freitag, 26. Juni 2020 13:29:07 > Betreff: [PATCH v2 4/4] ubifs: prevent remounting rw when no hmac key was given > After adding readonly hmac-less authentication support, > prevent remounting the filesystem in rw mode, when > the hmac is not available. > > Signed-off-by: Torben Hohn <torben.hohn@linutronix.de> > --- > fs/ubifs/super.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c > index b41ca9ee5763..62bdef8f1ddf 100644 > --- a/fs/ubifs/super.c > +++ b/fs/ubifs/super.c > @@ -1996,6 +1996,10 @@ static int ubifs_remount_fs(struct super_block *sb, int > *flags, char *data) > ubifs_msg(c, "cannot re-mount R/W - UBI volume is R/O"); > return -EROFS; > } > + if (ubifs_authenticated(c) && !c->hash_tfm) { > + ubifs_msg(c, "cannot re-mount R/W due to missing hmac key, for > authentication"); > + return -EROFS; > + } But the case that one remounts rw and provides a HAMC is handled? Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 4/4] ubifs: prevent remounting rw when no hmac key was given 2020-06-26 12:27 ` Richard Weinberger @ 2020-06-29 8:53 ` Torben Hohn 2020-06-29 10:52 ` Richard Weinberger 0 siblings, 1 reply; 35+ messages in thread From: Torben Hohn @ 2020-06-29 8:53 UTC (permalink / raw) To: Richard Weinberger; +Cc: bigeasy, linux-mtd, tglx, Sascha Hauer On Fri, Jun 26, 2020 at 02:27:21PM +0200, Richard Weinberger wrote: > ----- Ursprüngliche Mail ----- > > Von: "Torben Hohn" <torben.hohn@linutronix.de> > > An: "richard" <richard@nod.at> > > CC: "bigeasy" <bigeasy@linutronix.de>, "tglx" <tglx@linutronix.de>, "linux-mtd" <linux-mtd@lists.infradead.org>, "Sascha > > Hauer" <s.hauer@pengutronix.de> > > Gesendet: Freitag, 26. Juni 2020 13:29:07 > > Betreff: [PATCH v2 4/4] ubifs: prevent remounting rw when no hmac key was given > > > After adding readonly hmac-less authentication support, > > prevent remounting the filesystem in rw mode, when > > the hmac is not available. > > > > Signed-off-by: Torben Hohn <torben.hohn@linutronix.de> > > --- > > fs/ubifs/super.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c > > index b41ca9ee5763..62bdef8f1ddf 100644 > > --- a/fs/ubifs/super.c > > +++ b/fs/ubifs/super.c > > @@ -1996,6 +1996,10 @@ static int ubifs_remount_fs(struct super_block *sb, int > > *flags, char *data) > > ubifs_msg(c, "cannot re-mount R/W - UBI volume is R/O"); > > return -EROFS; > > } > > + if (ubifs_authenticated(c) && !c->hash_tfm) { > > + ubifs_msg(c, "cannot re-mount R/W due to missing hmac key, for > > authentication"); > > + return -EROFS; > > + } > > But the case that one remounts rw and provides a HAMC is handled? No. And i am not sure, whether thats a thing, we want to support. I would suggest, we clarify that in the message. One would need to check, whether c->auth_key_name is set now, and then instantiate c->hash_tfm. ubifs_init_authentication is not called upon remount. so a remount with a changed auth_key_name is not supported either. > > Thanks, > //richard -- Torben Hohn Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99 Hinweise zum Datenschutz finden Sie hier (Informations on data privacy can be found here): https://linutronix.de/kontakt/Datenschutz.php Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen | Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 4/4] ubifs: prevent remounting rw when no hmac key was given 2020-06-29 8:53 ` Torben Hohn @ 2020-06-29 10:52 ` Richard Weinberger 0 siblings, 0 replies; 35+ messages in thread From: Richard Weinberger @ 2020-06-29 10:52 UTC (permalink / raw) To: Torben Hohn; +Cc: bigeasy, linux-mtd, tglx, Sascha Hauer Torben, ----- Ursprüngliche Mail ----- >> >> But the case that one remounts rw and provides a HAMC is handled? > > No. And i am not sure, whether thats a thing, we want to support. > I would suggest, we clarify that in the message. Yeah. > > One would need to check, whether c->auth_key_name is set now, > and then instantiate c->hash_tfm. > > ubifs_init_authentication is not called upon remount. > so a remount with a changed auth_key_name is not supported either. Because it is technically not possible. We'd need both keys then. But I agree we should reject it. Remounting is tricky... Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-06-26 11:29 ` [PATCH v2 0/4] " Torben Hohn ` (3 preceding siblings ...) 2020-06-26 11:29 ` [PATCH v2 4/4] ubifs: prevent remounting rw when no hmac key was given Torben Hohn @ 2020-06-26 14:16 ` Richard Weinberger 2020-06-26 14:36 ` Richard Weinberger 2020-06-29 9:07 ` Torben Hohn 4 siblings, 2 replies; 35+ messages in thread From: Richard Weinberger @ 2020-06-26 14:16 UTC (permalink / raw) To: Torben Hohn; +Cc: bigeasy, linux-mtd, tglx, david, Sascha Hauer Torben, ----- Ursprüngliche Mail ----- > Von: "Torben Hohn" <torben.hohn@linutronix.de> > An: "richard" <richard@nod.at> > CC: "bigeasy" <bigeasy@linutronix.de>, "tglx" <tglx@linutronix.de>, "linux-mtd" <linux-mtd@lists.infradead.org>, "Sascha > Hauer" <s.hauer@pengutronix.de> > Gesendet: Freitag, 26. Juni 2020 13:29:03 > Betreff: [PATCH v2 0/4] ubifs: support authentication without hmac > This PQ adds support for ubifs authentication without HMAC, > which obviously only works for a read-only mount. > > ubiblock and dm-verity are not supported by u-boot, and > the kernel on the target is loaded by u-boot out of the RFS. > > This is a first try to implement this. > It boots fine, and the WARN_ON is not triggered. > > I plan to update the docs also, but i would like to have > some positive comments on this before. > > Changes since v1: > > - apply comments from Sascha an revert the > ubifs_authicated_(read|write) stuff. > Use ubifs_assert(c, !c->ro_mount) instead. > - Prevent remount rw, when hmac-less authentication is used > - add missing check, for ro mode, when no auth_key_name is specified. I didn't dig deep into the code so far, I'm still checking the concept. Your approach works only on pristine offline signed images from mkfs.ubifs. So, if somebody does this, it won't work: $ keyctl padd logon ubifs:authfs @s < secret.key $ mount -t ubifs /dev/ubi0_0 /mnt/ -o auth_hash_name=sha256,auth_key=ubifs:authfs ... change the fs ... $ umount /mnt $ mount -t ubifs /dev/ubi0_0 /mnt/ -o auth_hash_name=sha256,ro The ro mount will fail because UBIFS is no longer able to verify the super block using the system key ring. It was overwritten by they ubifs:authfs key. A possible solution is keeping a copy of the offline sign key forever in the fs. But I'm not sure whether this is wise. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-06-26 14:16 ` [PATCH v2 0/4] ubifs: support authentication without hmac Richard Weinberger @ 2020-06-26 14:36 ` Richard Weinberger 2020-06-29 9:13 ` Torben Hohn 2020-06-29 9:07 ` Torben Hohn 1 sibling, 1 reply; 35+ messages in thread From: Richard Weinberger @ 2020-06-26 14:36 UTC (permalink / raw) To: Torben Hohn; +Cc: bigeasy, linux-mtd, tglx, david, Sascha Hauer ----- Ursprüngliche Mail ----- > I didn't dig deep into the code so far, I'm still checking the concept. > > Your approach works only on pristine offline signed images from mkfs.ubifs. > So, if somebody does this, it won't work: > > $ keyctl padd logon ubifs:authfs @s < secret.key > $ mount -t ubifs /dev/ubi0_0 /mnt/ -o > auth_hash_name=sha256,auth_key=ubifs:authfs > > ... change the fs ... > > $ umount /mnt > $ mount -t ubifs /dev/ubi0_0 /mnt/ -o auth_hash_name=sha256,ro > > The ro mount will fail because UBIFS is no longer able to verify the super block > using the system key ring. It was overwritten by they ubifs:authfs key. > > A possible solution is keeping a copy of the offline sign key forever in the fs. > But I'm not sure whether this is wise. Or we change the feature from "ro mount without hmac" to "keep offline sign key and imply ro mount". IOW adding a new mount option such as "auth_keep_offlinekey". If mounted with this option UBIFS will not look for a hmac and enforce read-only mode. Hmm? Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-06-26 14:36 ` Richard Weinberger @ 2020-06-29 9:13 ` Torben Hohn 0 siblings, 0 replies; 35+ messages in thread From: Torben Hohn @ 2020-06-29 9:13 UTC (permalink / raw) To: Richard Weinberger; +Cc: bigeasy, linux-mtd, tglx, david, Sascha Hauer On Fri, Jun 26, 2020 at 04:36:26PM +0200, Richard Weinberger wrote: > ----- Ursprüngliche Mail ----- > > I didn't dig deep into the code so far, I'm still checking the concept. > > > > Your approach works only on pristine offline signed images from mkfs.ubifs. > > So, if somebody does this, it won't work: > > > > $ keyctl padd logon ubifs:authfs @s < secret.key > > $ mount -t ubifs /dev/ubi0_0 /mnt/ -o > > auth_hash_name=sha256,auth_key=ubifs:authfs > > > > ... change the fs ... > > > > $ umount /mnt > > $ mount -t ubifs /dev/ubi0_0 /mnt/ -o auth_hash_name=sha256,ro > > > > The ro mount will fail because UBIFS is no longer able to verify the super block > > using the system key ring. It was overwritten by they ubifs:authfs key. > > > > A possible solution is keeping a copy of the offline sign key forever in the fs. > > But I'm not sure whether this is wise. > > Or we change the feature from "ro mount without hmac" to "keep offline sign key and imply ro mount". > IOW adding a new mount option such as "auth_keep_offlinekey". If mounted with this option > UBIFS will not look for a hmac and enforce read-only mode. Thats just another name for the same feature. But it indeed seems to make the implications clearer. And it porbably also makes the code easier to read. > > Hmm? > > Thanks, > //richard -- Torben Hohn Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99 Hinweise zum Datenschutz finden Sie hier (Informations on data privacy can be found here): https://linutronix.de/kontakt/Datenschutz.php Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen | Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-06-26 14:16 ` [PATCH v2 0/4] ubifs: support authentication without hmac Richard Weinberger 2020-06-26 14:36 ` Richard Weinberger @ 2020-06-29 9:07 ` Torben Hohn 2020-06-29 10:46 ` Richard Weinberger 1 sibling, 1 reply; 35+ messages in thread From: Torben Hohn @ 2020-06-29 9:07 UTC (permalink / raw) To: Richard Weinberger; +Cc: bigeasy, linux-mtd, tglx, david, Sascha Hauer On Fri, Jun 26, 2020 at 04:16:51PM +0200, Richard Weinberger wrote: > Torben, > > ----- Ursprüngliche Mail ----- > > Von: "Torben Hohn" <torben.hohn@linutronix.de> > > An: "richard" <richard@nod.at> > > CC: "bigeasy" <bigeasy@linutronix.de>, "tglx" <tglx@linutronix.de>, "linux-mtd" <linux-mtd@lists.infradead.org>, "Sascha > > Hauer" <s.hauer@pengutronix.de> > > Gesendet: Freitag, 26. Juni 2020 13:29:03 > > Betreff: [PATCH v2 0/4] ubifs: support authentication without hmac > > > This PQ adds support for ubifs authentication without HMAC, > > which obviously only works for a read-only mount. > > > > ubiblock and dm-verity are not supported by u-boot, and > > the kernel on the target is loaded by u-boot out of the RFS. > > > > This is a first try to implement this. > > It boots fine, and the WARN_ON is not triggered. > > > > I plan to update the docs also, but i would like to have > > some positive comments on this before. > > > > Changes since v1: > > > > - apply comments from Sascha an revert the > > ubifs_authicated_(read|write) stuff. > > Use ubifs_assert(c, !c->ro_mount) instead. > > - Prevent remount rw, when hmac-less authentication is used > > - add missing check, for ro mode, when no auth_key_name is specified. > > I didn't dig deep into the code so far, I'm still checking the concept. > > Your approach works only on pristine offline signed images from mkfs.ubifs. > So, if somebody does this, it won't work: > > $ keyctl padd logon ubifs:authfs @s < secret.key > $ mount -t ubifs /dev/ubi0_0 /mnt/ -o auth_hash_name=sha256,auth_key=ubifs:authfs > > ... change the fs ... > > $ umount /mnt > $ mount -t ubifs /dev/ubi0_0 /mnt/ -o auth_hash_name=sha256,ro > > The ro mount will fail because UBIFS is no longer able to verify the super block > using the system key ring. It was overwritten by they ubifs:authfs key. Yes. But that is the intended behaviour. If the filesystem has been changed, it must not be mounted again. I would rather like to make it impossible to mount the filesystem in rw mode, because this is an attack scenario. It would refuse to mount upon reboot. Making it possible to remount root rw, with a fresh key is nice for development, but its not desired in production. > > A possible solution is keeping a copy of the offline sign key forever in the fs. > But I'm not sure whether this is wise. Heh ? you mean the private key. NO When its not possible to store a secret on the h/w then the only option we have is asymmetric sigatures, with the private key never being on the device. This obviously means, that the device is not able to write to RFS. > > Thanks, > //richard -- Torben Hohn Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99 Hinweise zum Datenschutz finden Sie hier (Informations on data privacy can be found here): https://linutronix.de/kontakt/Datenschutz.php Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen | Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-06-29 9:07 ` Torben Hohn @ 2020-06-29 10:46 ` Richard Weinberger 2020-07-02 14:40 ` Thomas Gleixner 0 siblings, 1 reply; 35+ messages in thread From: Richard Weinberger @ 2020-06-29 10:46 UTC (permalink / raw) To: Torben Hohn; +Cc: bigeasy, linux-mtd, tglx, david, Sascha Hauer Torben, ----- Ursprüngliche Mail ----- >> The ro mount will fail because UBIFS is no longer able to verify the super block >> using the system key ring. It was overwritten by they ubifs:authfs key. > > Yes. But that is the intended behaviour. > If the filesystem has been changed, it must not be mounted again. In your use case. > I would rather like to make it impossible to mount the filesystem in rw > mode, because this is an attack scenario. It would refuse to mount upon > reboot. Making it possible to remount root rw, with a fresh key is > nice for development, but its not desired in production. > > >> >> A possible solution is keeping a copy of the offline sign key forever in the fs. >> But I'm not sure whether this is wise. > > Heh ? you mean the private key. NO No, I used bad wording. :-) The superblock is signed by the offline key. As soon you switch to the new key the super block is rewritten and can no longer verified this key. Instead of rewriting the idea was keeping a copy. Anyway, like said in the other mail, I think if we change the feature to "keep offline sign key and imply ro mount" things will be more smooth with less corner cases. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-06-29 10:46 ` Richard Weinberger @ 2020-07-02 14:40 ` Thomas Gleixner 2020-07-02 15:00 ` Richard Weinberger 0 siblings, 1 reply; 35+ messages in thread From: Thomas Gleixner @ 2020-07-02 14:40 UTC (permalink / raw) To: Richard Weinberger, Torben Hohn; +Cc: bigeasy, linux-mtd, Sascha Hauer, david Richard Weinberger <richard@nod.at> writes: > The superblock is signed by the offline key. As soon you switch to the new key > the super block is rewritten and can no longer verified this key. > Instead of rewriting the idea was keeping a copy. > > Anyway, like said in the other mail, I think if we change the feature to > "keep offline sign key and imply ro mount" things will be more smooth with less corner > cases. I don't think so. The desired mode is to prevent RW mounts for a factory signed image which implies the prevention of rewriting the superblock. This is not a conveniance feature, it's a strict security feature. Once the thing has been changed from the factory generated state it's invalid no matter what. If a developer shoots himself in the foot with that, no big deal. He got what he asked for :) Thanks, tglx ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-07-02 14:40 ` Thomas Gleixner @ 2020-07-02 15:00 ` Richard Weinberger 2020-07-02 18:48 ` Thomas Gleixner 0 siblings, 1 reply; 35+ messages in thread From: Richard Weinberger @ 2020-07-02 15:00 UTC (permalink / raw) To: tglx; +Cc: bigeasy, linux-mtd, Sascha Hauer, david, Torben Hohn ----- Ursprüngliche Mail ----- > Von: "tglx" <tglx@linutronix.de> > An: "richard" <richard@nod.at>, "Torben Hohn" <torben.hohn@linutronix.de> > CC: "bigeasy" <bigeasy@linutronix.de>, "linux-mtd" <linux-mtd@lists.infradead.org>, "Sascha Hauer" > <s.hauer@pengutronix.de>, "david" <david@sigma-star.at> > Gesendet: Donnerstag, 2. Juli 2020 16:40:24 > Betreff: Re: [PATCH v2 0/4] ubifs: support authentication without hmac > Richard Weinberger <richard@nod.at> writes: >> The superblock is signed by the offline key. As soon you switch to the new key >> the super block is rewritten and can no longer verified this key. >> Instead of rewriting the idea was keeping a copy. >> >> Anyway, like said in the other mail, I think if we change the feature to >> "keep offline sign key and imply ro mount" things will be more smooth with less >> corner >> cases. > > I don't think so. The desired mode is to prevent RW mounts for a factory > signed image which implies the prevention of rewriting the superblock. This is exactly what I'm asking for. Keep the factory signed super block and imply read-only mode. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-07-02 15:00 ` Richard Weinberger @ 2020-07-02 18:48 ` Thomas Gleixner 2020-07-02 19:03 ` Richard Weinberger 0 siblings, 1 reply; 35+ messages in thread From: Thomas Gleixner @ 2020-07-02 18:48 UTC (permalink / raw) To: Richard Weinberger; +Cc: bigeasy, linux-mtd, Sascha Hauer, david, Torben Hohn Richard Weinberger <richard@nod.at> writes: > ----- Ursprüngliche Mail ----- >> Von: "tglx" <tglx@linutronix.de> >> An: "richard" <richard@nod.at>, "Torben Hohn" <torben.hohn@linutronix.de> >> CC: "bigeasy" <bigeasy@linutronix.de>, "linux-mtd" <linux-mtd@lists.infradead.org>, "Sascha Hauer" >> <s.hauer@pengutronix.de>, "david" <david@sigma-star.at> >> Gesendet: Donnerstag, 2. Juli 2020 16:40:24 >> Betreff: Re: [PATCH v2 0/4] ubifs: support authentication without hmac > >> Richard Weinberger <richard@nod.at> writes: >>> The superblock is signed by the offline key. As soon you switch to the new key >>> the super block is rewritten and can no longer verified this key. >>> Instead of rewriting the idea was keeping a copy. >>> >>> Anyway, like said in the other mail, I think if we change the feature to >>> "keep offline sign key and imply ro mount" things will be more smooth with less >>> corner >>> cases. >> >> I don't think so. The desired mode is to prevent RW mounts for a factory >> signed image which implies the prevention of rewriting the superblock. > > This is exactly what I'm asking for. > Keep the factory signed super block and imply read-only mode. And that's what Torben implemented unless I'm missing something. Thanks, tglx ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-07-02 18:48 ` Thomas Gleixner @ 2020-07-02 19:03 ` Richard Weinberger 2020-07-03 8:16 ` bigeasy 2020-07-03 9:12 ` Thomas Gleixner 0 siblings, 2 replies; 35+ messages in thread From: Richard Weinberger @ 2020-07-02 19:03 UTC (permalink / raw) To: tglx; +Cc: bigeasy, linux-mtd, Sascha Hauer, david, Torben Hohn ----- Ursprüngliche Mail ----- >>>> Anyway, like said in the other mail, I think if we change the feature to >>>> "keep offline sign key and imply ro mount" things will be more smooth with less >>>> corner >>>> cases. >>> >>> I don't think so. The desired mode is to prevent RW mounts for a factory >>> signed image which implies the prevention of rewriting the superblock. >> >> This is exactly what I'm asking for. >> Keep the factory signed super block and imply read-only mode. > > And that's what Torben implemented unless I'm missing something. Torben implemented it the other way around, he allows mounting without the HMAC if UBIFS mount is read-only. This covers also the proposed use-case but as I stated it has issues with remounting and makes the implementation more complicated than it should be. That's why I proposed adding a new mount option like "keep_offline_signature" or what name fits better. That gives us the following pros: 1. Makes the implementation super simple. If keep_offline_signature is set and rw mount requested, reject. RW remount can rejected very easily, store keep_offline_signature in the ubifs context. 2. If the super block got already re-written, reject. You can check sub->hmac[] for being non-zero. That way we can give the user a decent error message in case they do stupid things. 3. Userspace can verify whether the UBIFS fs is pristine by checking for the keep_offline_signature mount flag in /proc/self/mountinfo. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-07-02 19:03 ` Richard Weinberger @ 2020-07-03 8:16 ` bigeasy 2020-07-03 8:20 ` Richard Weinberger 2020-07-03 9:12 ` Thomas Gleixner 1 sibling, 1 reply; 35+ messages in thread From: bigeasy @ 2020-07-03 8:16 UTC (permalink / raw) To: Richard Weinberger; +Cc: tglx, linux-mtd, Sascha Hauer, david, Torben Hohn On 2020-07-02 21:03:54 [+0200], Richard Weinberger wrote: > ----- Ursprüngliche Mail ----- > >>>> Anyway, like said in the other mail, I think if we change the feature to > >>>> "keep offline sign key and imply ro mount" things will be more smooth with less > >>>> corner > >>>> cases. > >>> > >>> I don't think so. The desired mode is to prevent RW mounts for a factory > >>> signed image which implies the prevention of rewriting the superblock. > >> > >> This is exactly what I'm asking for. > >> Keep the factory signed super block and imply read-only mode. > > > > And that's what Torben implemented unless I'm missing something. > > Torben implemented it the other way around, he allows mounting without > the HMAC if UBIFS mount is read-only. > This covers also the proposed use-case but as I stated it has issues with > remounting and makes the implementation more complicated than it should be. > > That's why I proposed adding a new mount option like "keep_offline_signature" or > what name fits better. That gives us the following pros: so you want an extra option instead of setting SB_RDONLY on RO mounts without the key and not allowing RW mounts in this case? > 1. Makes the implementation super simple. > If keep_offline_signature is set and rw mount requested, reject. > RW remount can rejected very easily, store keep_offline_signature in the ubifs context. > > 2. If the super block got already re-written, reject. > You can check sub->hmac[] for being non-zero. > That way we can give the user a decent error message in case they do stupid things. re-written as in a prior RW mount with the key? > 3. Userspace can verify whether the UBIFS fs is pristine by checking > for the keep_offline_signature mount flag in /proc/self/mountinfo. Could this information be dubious if the UBIFS was mounted RW once (with the key around) and then mounted RO,keep_offline_signature ? So you would have to only allow keep_offline_signature if your point (2) is true? > Thanks, > //richard Sebastian ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-07-03 8:16 ` bigeasy @ 2020-07-03 8:20 ` Richard Weinberger 0 siblings, 0 replies; 35+ messages in thread From: Richard Weinberger @ 2020-07-03 8:20 UTC (permalink / raw) To: bigeasy; +Cc: tglx, linux-mtd, Sascha Hauer, david, Torben Hohn Sebastian, ----- Ursprüngliche Mail ----- >> > And that's what Torben implemented unless I'm missing something. >> >> Torben implemented it the other way around, he allows mounting without >> the HMAC if UBIFS mount is read-only. >> This covers also the proposed use-case but as I stated it has issues with >> remounting and makes the implementation more complicated than it should be. >> >> That's why I proposed adding a new mount option like "keep_offline_signature" or >> what name fits better. That gives us the following pros: > > so you want an extra option instead of setting SB_RDONLY on RO mounts > without the key and not allowing RW mounts in this case? Yes. >> 1. Makes the implementation super simple. >> If keep_offline_signature is set and rw mount requested, reject. >> RW remount can rejected very easily, store keep_offline_signature in the ubifs >> context. >> >> 2. If the super block got already re-written, reject. >> You can check sub->hmac[] for being non-zero. >> That way we can give the user a decent error message in case they do stupid >> things. > > re-written as in a prior RW mount with the key? Yes. >> 3. Userspace can verify whether the UBIFS fs is pristine by checking >> for the keep_offline_signature mount flag in /proc/self/mountinfo. > > Could this information be dubious if the UBIFS was mounted RW once (with > the key around) and then mounted RO,keep_offline_signature ? So you > would have to only allow keep_offline_signature if your point (2) is > true? No. Because as soon you mount once RW the super block is re-written with the provided HMAC. You can detect this and refuse the mount option. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/4] ubifs: support authentication without hmac 2020-07-02 19:03 ` Richard Weinberger 2020-07-03 8:16 ` bigeasy @ 2020-07-03 9:12 ` Thomas Gleixner 1 sibling, 0 replies; 35+ messages in thread From: Thomas Gleixner @ 2020-07-03 9:12 UTC (permalink / raw) To: Richard Weinberger; +Cc: bigeasy, linux-mtd, Sascha Hauer, david, Torben Hohn Richard Weinberger <richard@nod.at> writes: >> And that's what Torben implemented unless I'm missing something. > > Torben implemented it the other way around, he allows mounting without > the HMAC if UBIFS mount is read-only. > This covers also the proposed use-case but as I stated it has issues with > remounting and makes the implementation more complicated than it should be. > > That's why I proposed adding a new mount option like "keep_offline_signature" or > what name fits better. That gives us the following pros: > > 1. Makes the implementation super simple. > If keep_offline_signature is set and rw mount requested, reject. > RW remount can rejected very easily, store keep_offline_signature in the ubifs context. > > 2. If the super block got already re-written, reject. > You can check sub->hmac[] for being non-zero. > That way we can give the user a decent error message in case they do stupid things. > > 3. Userspace can verify whether the UBIFS fs is pristine by checking > for the keep_offline_signature mount flag in /proc/self/mountinfo. Works for me. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2020-07-03 9:14 UTC | newest] Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-25 15:59 [PATCH 0/1] ubifs: support authentication without hmac Torben Hohn 2020-06-25 15:59 ` [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given Torben Hohn 2020-06-26 4:31 ` Sascha Hauer 2020-06-26 7:27 ` Torben Hohn 2020-06-26 7:53 ` Richard Weinberger 2020-06-26 8:10 ` Sascha Hauer 2020-06-26 9:39 ` Torben Hohn 2020-06-26 8:09 ` [PATCH 0/1] ubifs: support authentication without hmac Richard Weinberger 2020-06-29 6:46 ` Alexander Dahl 2020-06-29 7:04 ` Richard Weinberger 2020-06-29 7:48 ` Wolfgang Denk 2020-06-29 7:51 ` Richard Weinberger 2020-06-30 5:50 ` Wolfgang Denk 2020-06-30 13:36 ` Richard Weinberger 2020-06-30 14:36 ` Alexander Dahl 2020-06-26 11:29 ` [PATCH v2 0/4] " Torben Hohn 2020-06-26 11:29 ` [PATCH v2 1/4] ubifs: move #include "debug.h" above auth.c Torben Hohn 2020-06-26 11:29 ` [PATCH v2 2/4] ubifs: support authentication, for ro mount, when no key is given Torben Hohn 2020-06-26 11:29 ` [PATCH v2 3/4] ubifs: sprinkle ubifs_assert(c, !c->ro_mount) in hmac auth Torben Hohn 2020-06-26 11:29 ` [PATCH v2 4/4] ubifs: prevent remounting rw when no hmac key was given Torben Hohn 2020-06-26 12:27 ` Richard Weinberger 2020-06-29 8:53 ` Torben Hohn 2020-06-29 10:52 ` Richard Weinberger 2020-06-26 14:16 ` [PATCH v2 0/4] ubifs: support authentication without hmac Richard Weinberger 2020-06-26 14:36 ` Richard Weinberger 2020-06-29 9:13 ` Torben Hohn 2020-06-29 9:07 ` Torben Hohn 2020-06-29 10:46 ` Richard Weinberger 2020-07-02 14:40 ` Thomas Gleixner 2020-07-02 15:00 ` Richard Weinberger 2020-07-02 18:48 ` Thomas Gleixner 2020-07-02 19:03 ` Richard Weinberger 2020-07-03 8:16 ` bigeasy 2020-07-03 8:20 ` Richard Weinberger 2020-07-03 9:12 ` Thomas Gleixner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).