Hi all,
I am learning CryptoAPI by porting stream ciphers into it
(particularly eSTREAM candidates at
<http://www.ecrypt.eu.org/stream/>). As eSTREAM ciphers manipulates IV
differently depending on algorithm design, it seems that standard
block modes cannot be used: ECB has no IV; CBC and CTR don't let
ciphers control how the IV is processed.
So I derived stream.c from Herbert's cbc.c and Joy's ctr.c. It wraps
around any cipher exporting cia_ivsize and cia_setiv_crypt() from
"struct cipher_alg". As a test case, I've used Daniel Bernstein's
Salsa20 implementation at <http://cr.yp.to/snuffle.html> as the
cipher. My patch against Herbert's cryptodev-2.6 git is attached.
Some questions come to mind:
1. Is this the way to go for general stream ciphers?
2. Would a separate "struct stream_alg" be better than hijacking
"struct cipher_alg"?
3. Currently cia_setiv_crypt() does setiv() and inplace encryption;
is this a good design?
4. Should the blocksize be 1 for the stream template?
Please feel free to discuss the patch so that it can be improved.
Regards,
Swee Heng
Hi.
On Fri, Oct 26, 2007 at 01:55:48AM +0800, Tan Swee Heng ([email protected]) wrote:
> Hi all,
>
> I am learning CryptoAPI by porting stream ciphers into it
> (particularly eSTREAM candidates at
> <http://www.ecrypt.eu.org/stream/>). As eSTREAM ciphers manipulates IV
> differently depending on algorithm design, it seems that standard
> block modes cannot be used: ECB has no IV; CBC and CTR don't let
> ciphers control how the IV is processed.
>
> So I derived stream.c from Herbert's cbc.c and Joy's ctr.c. It wraps
> around any cipher exporting cia_ivsize and cia_setiv_crypt() from
> "struct cipher_alg". As a test case, I've used Daniel Bernstein's
> Salsa20 implementation at <http://cr.yp.to/snuffle.html> as the
> cipher. My patch against Herbert's cryptodev-2.6 git is attached.
>
> Some questions come to mind:
> 1. Is this the way to go for general stream ciphers?
> 2. Would a separate "struct stream_alg" be better than hijacking
> "struct cipher_alg"?
> 3. Currently cia_setiv_crypt() does setiv() and inplace encryption;
> is this a good design?
> 4. Should the blocksize be 1 for the stream template?
It depends. I do not know any stream cipher, which operates on 1-byte
quantity in real life, but stream means 1-byte blocksize is a correct
assumption.
Your approach is likely the less intrusive for existing block-based
design, so I believe this is a way to go.
Besides number of codying style issues (like comments, () placement,
spaces and the like) it looks good.
--
Evgeniy Polyakov
Hi.
On Sun, Nov 04, 2007 at 01:35:14AM +0800, Tan Swee Heng ([email protected]) wrote:
> CHANGES
> 1. stream.c's blocksize is now 1 since (i) it makes more sense and
> (ii) the latest 2.6.24-rc1 git allows it.
> 2. SOSEMANUK, another eSTREAM candidate, has been ported.
> 3. Got rid of "coding style issues".. although I am not sure if I've
> eradicated all of them :-)
I have some minor comments inlined below.
Btw, it is much better if you inline patch and put them into several
mails if needed.
> TESTING
> 1. I've used "insmod tcrypt.ko mode=X" (X=34 for Salsa20 and X=35 for
> SOSEMANUK). The test vectors agree.
> 2. I've used "cryptsetup luksFormat -c salsa20-stream-plain
> /dev/loop0". That worked fine and I was able to create an encrypted
> ext2 filesystem using Salsa20 as the cipher.
> 3. However "cryptsetup luksFormat -c sosemanuk-stream-plain
> /dev/loop0" didn't work that well as I was unable to unlock using
> "cryptsetup luksOpen" later. (I am not sure where the problems lie.
> Any suggestion is welcomed.)
Add debug prints all over the place and find where the problem lies...
> BUG
> My patches currently set cia_encrypt and cia_decrypt to NULL. That
> isn't quite safe. Someone using luksFormat with "-c salsa20-cbc-plain"
> may cause the kernel to behave in an unbecoming manner as cbc will try
> to use salsa20's cia_encrypt. (Any suggestion is welcomed on how this
> can be avoided.)
> PATCHES
> Attached to this mail are the patches. I've split them into 3 this
> time so as to make clear what are the changes for stream, salsa20 and
> sosemanuk respectively. They need to be applied in succession to the
> latest 2.6.24-rc1 cryptodev git.
>
> Swee Heng
+static int crypto_stream_crypt_walk(struct blkcipher_walk *walk,
+ struct crypto_cipher *tfm)
+{
+ void (*fn)(struct crypto_tfm *, u8 *, const u8 *, u32) =
+ crypto_cipher_alg(tfm)->cia_setiv_crypt;
+ unsigned int nbytes = walk->nbytes;
+ u8 *src = walk->src.virt.addr;
+ u8 *dst = walk->dst.virt.addr;
+ u8 *iv = walk->iv;
+
+ if(dst != src)
Need a space.
+ memcpy(dst, src, nbytes);
+ fn(crypto_cipher_tfm(tfm), dst, iv, nbytes);
+ return 0;
+}
+ if (!(alg->cra_blocksize % 4))
+ inst->alg.cra_alignmask |= 3;
You have not changed align mask as wanted originally - is it intentional?
+ inst->alg.cra_blkcipher.ivsize = 1;
+ inst->alg.cra_blkcipher.min_keysize = alg->cra_cipher.cia_min_keysize;
+ inst->alg.cra_blkcipher.max_keysize = alg->cra_cipher.cia_max_keysize;
+
+ inst->alg.cra_ctxsize = sizeof(struct crypto_stream_ctx);
+
+ inst->alg.cra_init = crypto_stream_init_tfm;
+ inst->alg.cra_exit = crypto_stream_exit_tfm;
+
+ inst->alg.cra_blkcipher.setkey = crypto_stream_setkey;
+ inst->alg.cra_blkcipher.encrypt = crypto_stream_crypt;
+ inst->alg.cra_blkcipher.decrypt = crypto_stream_crypt;
+
+out_put_alg:
+ crypto_mod_put(alg);
+ return inst;
+}
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -214,6 +214,10 @@ struct cipher_alg {
unsigned int keylen);
void (*cia_encrypt)(struct crypto_tfm *tfm, u8 *dst, const u8 *src);
void (*cia_decrypt)(struct crypto_tfm *tfm, u8 *dst, const u8 *src);
+
+ unsigned int cia_ivsize;
+ void (*cia_setiv_crypt)(struct crypto_tfm *tfm, u8 *dst,
+ const u8 *iv, u32 nbytes);
};
I'm not sure it is a good way - you can save this iv and its size in the
context and get it from cipher's encrypt/decrypt callback.
Or modify every encrypt and decrypt callback to have iv as parameter,
which I believe not a good solution.
In any case, you have to disallow unsupported usage (like cbc) in your
cipher callbacks.
+static void xor_byte(u8 *a, const u8 *b, unsigned int bs)
+{
+ for (; bs; bs--)
+ *a++ ^= *b++;
+}
+
+static void xor_quad(u8 *dst, const u8 *src, unsigned int bs)
+{
+ u32 *a = (u32 *)dst;
+ u32 *b = (u32 *)src;
+
+ for (; bs >= 4; bs -= 4)
+ *a++ ^= *b++;
+
+ xor_byte((u8 *)a, (u8 *)b, bs);
+}
This ones exist in cbc at least - move them into common header.
The same applies to sosemanuk algo.
--
Evgeniy Polyakov
Hi Evgeniy,
Thanks for the reply. Unfortunately my previous email did not make it
on to the mailing list (it is blocked by Majorodomo as it exceeded
100,000 bytes) so only you and Herbert received it. The rest of the
reader would surely be confused by our current exchange. My apologies!
I have rewrote the patch to avoid setting "cia_encrypt = NULL" and
will be re-posting them. Also I will hold back the SOSEMANUK cipher
for now.
Swee Heng
On Nov 6, 2007 8:19 PM, Evgeniy Polyakov <[email protected]> wrote:
> Hi.
>
> On Sun, Nov 04, 2007 at 01:35:14AM +0800, Tan Swee Heng ([email protected]) wrote:
> > CHANGES
> > 1. stream.c's blocksize is now 1 since (i) it makes more sense and
> > (ii) the latest 2.6.24-rc1 git allows it.
> > 2. SOSEMANUK, another eSTREAM candidate, has been ported.
> > 3. Got rid of "coding style issues".. although I am not sure if I've
> > eradicated all of them :-)
>
> I have some minor comments inlined below.
> Btw, it is much better if you inline patch and put them into several
> mails if needed.
>
> > TESTING
> > 1. I've used "insmod tcrypt.ko mode=X" (X=34 for Salsa20 and X=35 for
> > SOSEMANUK). The test vectors agree.
> > 2. I've used "cryptsetup luksFormat -c salsa20-stream-plain
> > /dev/loop0". That worked fine and I was able to create an encrypted
> > ext2 filesystem using Salsa20 as the cipher.
> > 3. However "cryptsetup luksFormat -c sosemanuk-stream-plain
> > /dev/loop0" didn't work that well as I was unable to unlock using
> > "cryptsetup luksOpen" later. (I am not sure where the problems lie.
> > Any suggestion is welcomed.)
>
> Add debug prints all over the place and find where the problem lies...
>
> > BUG
> > My patches currently set cia_encrypt and cia_decrypt to NULL. That
> > isn't quite safe. Someone using luksFormat with "-c salsa20-cbc-plain"
> > may cause the kernel to behave in an unbecoming manner as cbc will try
> > to use salsa20's cia_encrypt. (Any suggestion is welcomed on how this
> > can be avoided.)
>
> > PATCHES
> > Attached to this mail are the patches. I've split them into 3 this
> > time so as to make clear what are the changes for stream, salsa20 and
> > sosemanuk respectively. They need to be applied in succession to the
> > latest 2.6.24-rc1 cryptodev git.
> >
> > Swee Heng
>
> +static int crypto_stream_crypt_walk(struct blkcipher_walk *walk,
> + struct crypto_cipher *tfm)
> +{
> + void (*fn)(struct crypto_tfm *, u8 *, const u8 *, u32) =
> + crypto_cipher_alg(tfm)->cia_setiv_crypt;
> + unsigned int nbytes = walk->nbytes;
> + u8 *src = walk->src.virt.addr;
> + u8 *dst = walk->dst.virt.addr;
> + u8 *iv = walk->iv;
> +
> + if(dst != src)
>
> Need a space.
>
> + memcpy(dst, src, nbytes);
> + fn(crypto_cipher_tfm(tfm), dst, iv, nbytes);
> + return 0;
> +}
>
> + if (!(alg->cra_blocksize % 4))
> + inst->alg.cra_alignmask |= 3;
>
> You have not changed align mask as wanted originally - is it intentional?
>
> + inst->alg.cra_blkcipher.ivsize = 1;
> + inst->alg.cra_blkcipher.min_keysize = alg->cra_cipher.cia_min_keysize;
> + inst->alg.cra_blkcipher.max_keysize = alg->cra_cipher.cia_max_keysize;
> +
> + inst->alg.cra_ctxsize = sizeof(struct crypto_stream_ctx);
> +
> + inst->alg.cra_init = crypto_stream_init_tfm;
> + inst->alg.cra_exit = crypto_stream_exit_tfm;
> +
> + inst->alg.cra_blkcipher.setkey = crypto_stream_setkey;
> + inst->alg.cra_blkcipher.encrypt = crypto_stream_crypt;
> + inst->alg.cra_blkcipher.decrypt = crypto_stream_crypt;
> +
> +out_put_alg:
> + crypto_mod_put(alg);
> + return inst;
> +}
>
>
> --- a/include/linux/crypto.h
> +++ b/include/linux/crypto.h
> @@ -214,6 +214,10 @@ struct cipher_alg {
> unsigned int keylen);
> void (*cia_encrypt)(struct crypto_tfm *tfm, u8 *dst, const u8 *src);
> void (*cia_decrypt)(struct crypto_tfm *tfm, u8 *dst, const u8 *src);
> +
> + unsigned int cia_ivsize;
> + void (*cia_setiv_crypt)(struct crypto_tfm *tfm, u8 *dst,
> + const u8 *iv, u32 nbytes);
> };
>
> I'm not sure it is a good way - you can save this iv and its size in the
> context and get it from cipher's encrypt/decrypt callback.
> Or modify every encrypt and decrypt callback to have iv as parameter,
> which I believe not a good solution.
>
> In any case, you have to disallow unsupported usage (like cbc) in your
> cipher callbacks.
>
> +static void xor_byte(u8 *a, const u8 *b, unsigned int bs)
> +{
> + for (; bs; bs--)
> + *a++ ^= *b++;
> +}
> +
> +static void xor_quad(u8 *dst, const u8 *src, unsigned int bs)
> +{
> + u32 *a = (u32 *)dst;
> + u32 *b = (u32 *)src;
> +
> + for (; bs >= 4; bs -= 4)
> + *a++ ^= *b++;
> +
> + xor_byte((u8 *)a, (u8 *)b, bs);
> +}
>
> This ones exist in cbc at least - move them into common header.
> The same applies to sosemanuk algo.
>
>
> --
> Evgeniy Polyakov
>