From: Artem Bityutskiy <dedekind1@gmail.com> To: Joel Reardon <joel@clambassador.com> Cc: linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [patch] UBIFS: Add cryptographic functionality when a key is passed to the compress / decompress functions Date: Mon, 02 Apr 2012 17:36:23 +0300 [thread overview] Message-ID: <1333377383.22146.14.camel@sauron.fi.intel.com> (raw) In-Reply-To: <alpine.DEB.2.00.1203291639200.912@eristoteles.iwoars.net> [-- Attachment #1: Type: text/plain, Size: 3361 bytes --] On Thu, 2012-03-29 at 16:39 +0200, Joel Reardon wrote: > /* Fake description object for the "none" compressor */ > static struct ubifs_compressor none_compr = { > .compr_type = UBIFS_COMPR_NONE, > @@ -75,6 +78,55 @@ static struct ubifs_compressor zlib_compr = { > struct ubifs_compressor *ubifs_compressors[UBIFS_COMPR_TYPES_CNT]; > > /** > + * ubifs_aes_crypt - encrypt / decrypt data. > + * @str: data to crypt > + * @len: length of the data > + * @crypto_key: the cryptographic key to use to crypt the data > + * @crypto_key_len: the length of the crypto_key > + * @iv: the initialization vector to use > + * @ivlen: the length of the initialization vector > + * > + * This function applies aes encryption to the data. It is done in counter > + * mode, which means that encryption and decryption are the same operation, > + * i.e., it XORs the same generated bitstream, so it can be used both for > + * encryption / decryption. The operation is done in-place, so str mutates. > + */ > +int ubifs_aes_crypt(void *str, int len, u8 *crypto_key, > + int crypto_key_len, u8 *iv, int ivlen) You support only one length - please, kill ivlen parameter. Also, should ubifs_aes_crypt be static? I do not see any users outside of compress.c. In this case remove the "ubifs_" prefix. But a non-written convention, in UBIFS we _tend_ to prefix only non-static functions with "ubifs_" and avoid having it for static functions. > +{ > + struct crypto_blkcipher *tfm; > + struct blkcipher_desc desc; > + struct scatterlist sg; > + int err = 0; > + > + tfm = crypto_alloc_blkcipher(UBIFS_CRYPTO_ALGORITHM, 0, 0); > + Unnecessary empty line. > + if (IS_ERR(tfm)) { > + ubifs_err("failed to load transform for aes: %ld", > + PTR_ERR(tfm)); > + return err; > + } > + > + err = crypto_blkcipher_setkey(tfm, crypto_key, crypto_key_len); > + desc.tfm = tfm; > + desc.flags = 0; > + if (err) { > + ubifs_err("crypto_blkcipher_setkey() failed flags=%#x", > + crypto_blkcipher_get_flags(tfm)); > + return err; > + } > + memset(&sg, 0, sizeof(struct scatterlist)); > + Empty lines mean grouping, and I think this memeset should be grouped with sg_set_buf instead. > no_compr: > memcpy(out_buf, in_buf, in_len); > *out_len = in_len; > *compr_type = UBIFS_COMPR_NONE; > + goto encrypt; > + > +encrypt: I guess the above goto is redundant? > + if (crypto_key) { > + u8 iv[UBIFS_CRYPTO_KEYSIZE]; > + > + memset(iv, 0, UBIFS_CRYPTO_KEYSIZE); > + ubifs_aes_crypt(out_buf, *out_len, crypto_key, > + UBIFS_CRYPTO_KEYSIZE, iv, UBIFS_CRYPTO_KEYSIZE); > + } > } > > /** > @@ -149,7 +211,7 @@ no_compr: > * The length of the uncompressed data is returned in @out_len. This functions > * returns %0 on success or a negative error code on failure. > */ > -int ubifs_decompress(const void *in_buf, int in_len, void *out_buf, > +int ubifs_decompress(void *in_buf, int in_len, void *out_buf, > int *out_len, int compr_type, u8 *crypto_key) Please, write a fat "WARNING" note in the comment and tell that this function modifies the input buffer. > +/* Size of 128 bits in bytes */ > +#define AES_KEYSIZE_128 16 If you have no plans to support keys larger than 128 just kill this constant please. -- Best Regards, Artem Bityutskiy [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Artem Bityutskiy <dedekind1@gmail.com> To: Joel Reardon <joel@clambassador.com> Cc: linux-fsdevel@vger.kernel.org, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [patch] UBIFS: Add cryptographic functionality when a key is passed to the compress / decompress functions Date: Mon, 02 Apr 2012 17:36:23 +0300 [thread overview] Message-ID: <1333377383.22146.14.camel@sauron.fi.intel.com> (raw) In-Reply-To: <alpine.DEB.2.00.1203291639200.912@eristoteles.iwoars.net> [-- Attachment #1: Type: text/plain, Size: 3361 bytes --] On Thu, 2012-03-29 at 16:39 +0200, Joel Reardon wrote: > /* Fake description object for the "none" compressor */ > static struct ubifs_compressor none_compr = { > .compr_type = UBIFS_COMPR_NONE, > @@ -75,6 +78,55 @@ static struct ubifs_compressor zlib_compr = { > struct ubifs_compressor *ubifs_compressors[UBIFS_COMPR_TYPES_CNT]; > > /** > + * ubifs_aes_crypt - encrypt / decrypt data. > + * @str: data to crypt > + * @len: length of the data > + * @crypto_key: the cryptographic key to use to crypt the data > + * @crypto_key_len: the length of the crypto_key > + * @iv: the initialization vector to use > + * @ivlen: the length of the initialization vector > + * > + * This function applies aes encryption to the data. It is done in counter > + * mode, which means that encryption and decryption are the same operation, > + * i.e., it XORs the same generated bitstream, so it can be used both for > + * encryption / decryption. The operation is done in-place, so str mutates. > + */ > +int ubifs_aes_crypt(void *str, int len, u8 *crypto_key, > + int crypto_key_len, u8 *iv, int ivlen) You support only one length - please, kill ivlen parameter. Also, should ubifs_aes_crypt be static? I do not see any users outside of compress.c. In this case remove the "ubifs_" prefix. But a non-written convention, in UBIFS we _tend_ to prefix only non-static functions with "ubifs_" and avoid having it for static functions. > +{ > + struct crypto_blkcipher *tfm; > + struct blkcipher_desc desc; > + struct scatterlist sg; > + int err = 0; > + > + tfm = crypto_alloc_blkcipher(UBIFS_CRYPTO_ALGORITHM, 0, 0); > + Unnecessary empty line. > + if (IS_ERR(tfm)) { > + ubifs_err("failed to load transform for aes: %ld", > + PTR_ERR(tfm)); > + return err; > + } > + > + err = crypto_blkcipher_setkey(tfm, crypto_key, crypto_key_len); > + desc.tfm = tfm; > + desc.flags = 0; > + if (err) { > + ubifs_err("crypto_blkcipher_setkey() failed flags=%#x", > + crypto_blkcipher_get_flags(tfm)); > + return err; > + } > + memset(&sg, 0, sizeof(struct scatterlist)); > + Empty lines mean grouping, and I think this memeset should be grouped with sg_set_buf instead. > no_compr: > memcpy(out_buf, in_buf, in_len); > *out_len = in_len; > *compr_type = UBIFS_COMPR_NONE; > + goto encrypt; > + > +encrypt: I guess the above goto is redundant? > + if (crypto_key) { > + u8 iv[UBIFS_CRYPTO_KEYSIZE]; > + > + memset(iv, 0, UBIFS_CRYPTO_KEYSIZE); > + ubifs_aes_crypt(out_buf, *out_len, crypto_key, > + UBIFS_CRYPTO_KEYSIZE, iv, UBIFS_CRYPTO_KEYSIZE); > + } > } > > /** > @@ -149,7 +211,7 @@ no_compr: > * The length of the uncompressed data is returned in @out_len. This functions > * returns %0 on success or a negative error code on failure. > */ > -int ubifs_decompress(const void *in_buf, int in_len, void *out_buf, > +int ubifs_decompress(void *in_buf, int in_len, void *out_buf, > int *out_len, int compr_type, u8 *crypto_key) Please, write a fat "WARNING" note in the comment and tell that this function modifies the input buffer. > +/* Size of 128 bits in bytes */ > +#define AES_KEYSIZE_128 16 If you have no plans to support keys larger than 128 just kill this constant please. -- Best Regards, Artem Bityutskiy [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --]
next prev parent reply other threads:[~2012-04-02 14:33 UTC|newest] Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-02-09 15:24 [patch] Adding Secure Deletion to UBIFS Joel Reardon 2012-02-09 15:24 ` Joel Reardon 2012-02-09 15:24 ` Joel Reardon 2012-02-13 16:54 ` Artem Bityutskiy 2012-02-13 16:54 ` Artem Bityutskiy 2012-02-23 14:59 ` Joel Reardon 2012-02-23 14:59 ` Joel Reardon 2012-02-23 15:29 ` [patch] Add encryption key parameter to compress/decompress functions Joel Reardon 2012-02-23 15:29 ` Joel Reardon 2012-03-09 7:17 ` Artem Bityutskiy 2012-03-09 7:17 ` Artem Bityutskiy 2012-03-19 16:54 ` [patch] Add design document for UBIFS secure deletion Joel Reardon 2012-03-19 16:54 ` Joel Reardon 2012-03-20 20:10 ` Randy Dunlap 2012-03-20 20:10 ` Randy Dunlap 2012-03-21 13:26 ` Joel Reardon 2012-03-21 13:26 ` Joel Reardon 2012-03-21 16:20 ` Artem Bityutskiy 2012-03-21 16:20 ` Artem Bityutskiy 2012-03-21 16:10 ` Artem Bityutskiy 2012-03-21 16:10 ` Artem Bityutskiy 2012-03-23 13:50 ` Joel Reardon 2012-03-23 13:50 ` Joel Reardon 2012-03-23 15:38 ` Artem Bityutskiy 2012-03-23 15:38 ` Artem Bityutskiy 2012-03-23 16:38 ` Joel Reardon 2012-03-23 16:38 ` Joel Reardon 2012-03-26 15:03 ` Artem Bityutskiy 2012-03-26 15:03 ` Artem Bityutskiy 2012-02-29 17:09 ` [patch] Adding Secure Deletion to UBIFS Artem Bityutskiy 2012-02-29 17:09 ` Artem Bityutskiy 2012-03-15 14:48 ` [patch] Remove notion of key schemes Joel Reardon 2012-03-15 14:48 ` Joel Reardon 2012-03-16 12:43 ` Artem Bityutskiy 2012-03-16 12:43 ` Artem Bityutskiy 2012-03-16 12:51 ` Artem Bityutskiy 2012-03-16 12:51 ` Artem Bityutskiy 2012-03-16 13:34 ` Joel Reardon 2012-03-16 13:34 ` Joel Reardon 2012-03-16 13:41 ` Artem Bityutskiy 2012-03-16 13:41 ` Artem Bityutskiy 2012-03-16 15:02 ` Joel Reardon 2012-03-16 15:02 ` Joel Reardon 2012-03-19 14:56 ` Artem Bityutskiy 2012-03-19 14:56 ` Artem Bityutskiy 2012-02-20 20:15 ` [patch] Move CRC computation to separate function Joel Reardon 2012-02-20 20:15 ` Joel Reardon 2012-02-29 16:10 ` Artem Bityutskiy 2012-02-29 16:10 ` Artem Bityutskiy 2012-03-19 22:46 ` Joel Reardon 2012-03-19 22:46 ` Joel Reardon 2012-03-23 14:09 ` Artem Bityutskiy 2012-03-23 14:09 ` Artem Bityutskiy 2012-03-23 16:45 ` Joel Reardon 2012-03-23 16:45 ` Joel Reardon 2012-03-23 16:51 ` Artem Bityutskiy 2012-03-23 16:51 ` Artem Bityutskiy 2012-03-25 20:38 ` Joel Reardon 2012-03-25 20:38 ` Joel Reardon 2012-03-26 15:34 ` Artem Bityutskiy 2012-03-26 15:34 ` Artem Bityutskiy 2012-03-25 21:11 ` [patch] Add a encryption key parameter to the compress / decompress function Joel Reardon 2012-03-25 21:11 ` Joel Reardon 2012-03-25 21:38 ` [patch] Add cryptographic functionality when a key is passed to the compress / decompress functions Joel Reardon 2012-03-25 21:38 ` Joel Reardon 2012-03-27 8:33 ` Artem Bityutskiy 2012-03-27 8:33 ` Artem Bityutskiy 2012-03-29 14:39 ` [patch] UBIFS: " Joel Reardon 2012-03-29 14:39 ` Joel Reardon 2012-04-02 14:36 ` Artem Bityutskiy [this message] 2012-04-02 14:36 ` Artem Bityutskiy 2012-04-02 14:48 ` Joel Reardon 2012-04-02 14:48 ` Joel Reardon 2012-04-02 14:57 ` Artem Bityutskiy 2012-04-02 14:57 ` Artem Bityutskiy 2012-04-02 14:58 ` Joel Reardon 2012-04-02 14:58 ` Joel Reardon 2012-04-03 10:29 ` Joel Reardon 2012-04-03 10:29 ` Joel Reardon 2012-04-03 10:41 ` Guillaume LECERF 2012-04-03 10:41 ` Guillaume LECERF 2012-04-03 10:41 ` Guillaume LECERF 2012-04-03 11:35 ` Joel Reardon 2012-04-03 11:35 ` Joel Reardon 2012-04-12 14:05 ` Artem Bityutskiy 2012-04-12 14:05 ` Artem Bityutskiy 2012-03-27 8:27 ` [patch] Add a encryption key parameter to the compress / decompress function Artem Bityutskiy 2012-03-27 8:27 ` Artem Bityutskiy 2012-03-29 14:11 ` [patch] UBIFS: " Joel Reardon 2012-03-29 14:11 ` Joel Reardon 2012-04-02 14:02 ` Artem Bityutskiy 2012-04-02 14:02 ` Artem Bityutskiy 2012-02-29 17:25 ` [patch] Adding Secure Deletion to UBIFS Artem Bityutskiy 2012-02-29 17:25 ` Artem Bityutskiy 2012-03-01 13:41 ` Joel Reardon 2012-03-01 13:41 ` Joel Reardon 2012-03-09 7:36 ` Artem Bityutskiy 2012-03-09 7:36 ` Artem Bityutskiy 2012-03-09 19:29 ` Joel Reardon 2012-03-09 19:29 ` Joel Reardon 2012-03-12 13:30 ` Artem Bityutskiy 2012-03-12 13:30 ` Artem Bityutskiy 2012-03-12 13:34 ` Joel Reardon 2012-03-12 13:34 ` Joel Reardon 2012-03-12 13:36 ` Artem Bityutskiy 2012-03-12 13:36 ` Artem Bityutskiy 2012-03-12 13:37 ` Joel Reardon 2012-03-12 13:37 ` Joel Reardon 2012-03-14 10:20 ` Joel Reardon 2012-03-14 10:20 ` Joel Reardon 2012-03-14 10:27 ` Artem Bityutskiy 2012-03-14 10:27 ` Artem Bityutskiy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1333377383.22146.14.camel@sauron.fi.intel.com \ --to=dedekind1@gmail.com \ --cc=joel@clambassador.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mtd@lists.infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.