This patch adds countersize to CTR mode.
The template is now ctr(algo,noncesize,ivsize,countersize).
For example, ctr(aes,4,8,4) indicates the counterblock
will be composed of a salt/nonce that is 4 bytes, an iv
that is 8 bytes and the counter is 4 bytes.
When noncesize + ivsize + countersize == blocksize,
CTR initializes the counter portion of the block and
begins encrypting with counter set to 1.
If noncesize + ivsize == blocksize, then this indicates that
user is passing in entire counterblock. Thus countersize
indicates the amount of bytes in counterblock to use as
the counter for incrementing. CTR will increment counter
portion by 1, and begin encryption with that value.
Note that CTR assumes the counter portion of the block that
will be incremented is stored in big endian.
Please let me know if this looks ok.
Regards,
Joy
Signed-off-by: Joy Latten <[email protected]>
diff -urpN linux-2.6.22.aead.patch/crypto/ctr.c linux-2.6.22.aead.patch2/crypto/ctr.c
--- linux-2.6.22.aead.patch/crypto/ctr.c 2007-10-09 12:12:54.000000000 -0500
+++ linux-2.6.22.aead.patch2/crypto/ctr.c 2007-10-23 14:41:35.000000000 -0500
@@ -23,6 +23,7 @@ struct ctr_instance_ctx {
struct crypto_spawn alg;
unsigned int noncesize;
unsigned int ivsize;
+ unsigned int countersize;
};
struct crypto_ctr_ctx {
@@ -186,7 +187,6 @@ static int crypto_ctr_crypt(struct blkci
unsigned long alignmask = crypto_cipher_alignmask(child);
u8 cblk[bsize + alignmask];
u8 *counterblk = (u8 *)ALIGN((unsigned long)cblk, alignmask + 1);
- unsigned int countersize;
int err;
blkcipher_walk_init(&walk, dst, src, nbytes);
@@ -198,18 +198,18 @@ static int crypto_ctr_crypt(struct blkci
memcpy(counterblk + ictx->noncesize, walk.iv, ictx->ivsize);
/* initialize counter portion of counter block */
- countersize = bsize - ictx->noncesize - ictx->ivsize;
- ctr_inc_quad(counterblk + (bsize - countersize), countersize);
+ ctr_inc_quad(counterblk + (bsize - ictx->countersize),
+ ictx->countersize);
while (walk.nbytes) {
if (walk.src.virt.addr == walk.dst.virt.addr)
nbytes = crypto_ctr_crypt_inplace(&walk, child,
counterblk,
- countersize);
+ ictx->countersize);
else
nbytes = crypto_ctr_crypt_segment(&walk, child,
counterblk,
- countersize);
+ ictx->countersize);
err = blkcipher_walk_done(desc, &walk, nbytes);
}
@@ -256,6 +256,7 @@ static struct crypto_instance *crypto_ct
struct ctr_instance_ctx *ictx;
unsigned int noncesize;
unsigned int ivsize;
+ unsigned int countersize;
int err;
err = crypto_check_attr_type(tb, CRYPTO_ALG_TYPE_BLKCIPHER);
@@ -275,11 +276,16 @@ static struct crypto_instance *crypto_ct
if (err)
goto out_put_alg;
+ err = crypto_attr_u32(tb[4], &countersize);
+ if (err)
+ goto out_put_alg;
+
/* verify size of nonce + iv + counter */
err = -EINVAL;
- if ((noncesize + ivsize) >= alg->cra_blocksize)
+ if (((noncesize + ivsize) > alg->cra_blocksize) ||
+ (countersize > alg->cra_blocksize))
goto out_put_alg;
-
+
inst = kzalloc(sizeof(*inst) + sizeof(*ictx), GFP_KERNEL);
err = -ENOMEM;
if (!inst)
@@ -287,20 +293,21 @@ static struct crypto_instance *crypto_ct
err = -ENAMETOOLONG;
if (snprintf(inst->alg.cra_name, CRYPTO_MAX_ALG_NAME,
- "ctr(%s,%u,%u)", alg->cra_name, noncesize,
- ivsize) >= CRYPTO_MAX_ALG_NAME) {
+ "ctr(%s,%u,%u,%u)", alg->cra_name, noncesize,
+ ivsize, countersize) >= CRYPTO_MAX_ALG_NAME) {
goto err_free_inst;
}
if (snprintf(inst->alg.cra_driver_name, CRYPTO_MAX_ALG_NAME,
- "ctr(%s,%u,%u)", alg->cra_driver_name, noncesize,
- ivsize) >= CRYPTO_MAX_ALG_NAME) {
+ "ctr(%s,%u,%u,%u)", alg->cra_driver_name, noncesize,
+ ivsize, countersize) >= CRYPTO_MAX_ALG_NAME) {
goto err_free_inst;
}
ictx = crypto_instance_ctx(inst);
ictx->noncesize = noncesize;
ictx->ivsize = ivsize;
+ ictx->countersize = countersize;
err = crypto_init_spawn(&ictx->alg, alg, inst,
CRYPTO_ALG_TYPE_MASK | CRYPTO_ALG_ASYNC);
diff -urpN linux-2.6.22.aead.patch/crypto/tcrypt.c linux-2.6.22.aead.patch2/crypto/tcrypt.c
--- linux-2.6.22.aead.patch/crypto/tcrypt.c 2007-10-09 11:40:58.000000000 -0500
+++ linux-2.6.22.aead.patch2/crypto/tcrypt.c 2007-10-23 14:41:46.000000000 -0500
@@ -955,9 +955,9 @@ static void do_test(void)
AES_LRW_ENC_TEST_VECTORS);
test_cipher("lrw(aes)", DECRYPT, aes_lrw_dec_tv_template,
AES_LRW_DEC_TEST_VECTORS);
- test_cipher("ctr(aes,4,8)", ENCRYPT, aes_ctr_enc_tv_template,
+ test_cipher("ctr(aes,4,8,4)", ENCRYPT, aes_ctr_enc_tv_template,
AES_CTR_ENC_TEST_VECTORS);
- test_cipher("ctr(aes,4,8)", DECRYPT, aes_ctr_dec_tv_template,
+ test_cipher("ctr(aes,4,8,4)", DECRYPT, aes_ctr_dec_tv_template,
AES_CTR_DEC_TEST_VECTORS);
//CAST5
@@ -1136,9 +1136,9 @@ static void do_test(void)
AES_LRW_ENC_TEST_VECTORS);
test_cipher("lrw(aes)", DECRYPT, aes_lrw_dec_tv_template,
AES_LRW_DEC_TEST_VECTORS);
- test_cipher("ctr(aes,4,8)", ENCRYPT, aes_ctr_enc_tv_template,
+ test_cipher("ctr(aes,4,8,4)", ENCRYPT, aes_ctr_enc_tv_template,
AES_CTR_ENC_TEST_VECTORS);
- test_cipher("ctr(aes,4,8)", DECRYPT, aes_ctr_dec_tv_template,
+ test_cipher("ctr(aes,4,8,4)", DECRYPT, aes_ctr_dec_tv_template,
AES_CTR_DEC_TEST_VECTORS);
break;
On Tue, Oct 23, 2007 at 03:26:29PM -0500, Joy Latten wrote:
> + unsigned int countersize;
It's somewhat nicer to just use size_t in the kernel for these sorts
of data types. If you care about the exact number of bytes used by the
variable, types like u32 make the code more parsable.
> + err = crypto_attr_u32(tb[4], &countersize);
> + if (err)
> + goto out_put_alg;
It's also nice to have printk's along error paths. Syslogs down the
road tend to be less cryptic.
> - test_cipher("ctr(aes,4,8)", ENCRYPT, aes_ctr_enc_tv_template,
> + test_cipher("ctr(aes,4,8,4)", ENCRYPT, aes_ctr_enc_tv_template,
> AES_CTR_ENC_TEST_VECTORS);
> - test_cipher("ctr(aes,4,8)", DECRYPT, aes_ctr_dec_tv_template,
> + test_cipher("ctr(aes,4,8,4)", DECRYPT, aes_ctr_dec_tv_template,
> AES_CTR_DEC_TEST_VECTORS);
I have never been particularly thrilled about the the string-based
method of parameterizing block ciphers for in-kernel API calls.
Mike
On Tue, Oct 23, 2007 at 03:40:08PM -0500, Michael Halcrow wrote:
> On Tue, Oct 23, 2007 at 03:26:29PM -0500, Joy Latten wrote:
> > + unsigned int countersize;
>
> It's somewhat nicer to just use size_t in the kernel for these sorts
> of data types. If you care about the exact number of bytes used by the
> variable, types like u32 make the code more parsable.
I don't see how this makes a difference here at all.
> > + err = crypto_attr_u32(tb[4], &countersize);
> > + if (err)
> > + goto out_put_alg;
>
> It's also nice to have printk's along error paths. Syslogs down the
> road tend to be less cryptic.
Please don't. That's what error return values are for. Adding
printk's means that we'd have to think about limiting them later
when this is opened up for user-space access.
> > - test_cipher("ctr(aes,4,8)", ENCRYPT, aes_ctr_enc_tv_template,
> > + test_cipher("ctr(aes,4,8,4)", ENCRYPT, aes_ctr_enc_tv_template,
> > AES_CTR_ENC_TEST_VECTORS);
> > - test_cipher("ctr(aes,4,8)", DECRYPT, aes_ctr_dec_tv_template,
> > + test_cipher("ctr(aes,4,8,4)", DECRYPT, aes_ctr_dec_tv_template,
> > AES_CTR_DEC_TEST_VECTORS);
>
> I have never been particularly thrilled about the the string-based
> method of parameterizing block ciphers for in-kernel API calls.
If you have a better suggestion I'd love to hear it!
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, Oct 24, 2007 at 08:42:30AM +0800, Herbert Xu wrote:
> On Tue, Oct 23, 2007 at 03:40:08PM -0500, Michael Halcrow wrote:
> > On Tue, Oct 23, 2007 at 03:26:29PM -0500, Joy Latten wrote:
> > > + unsigned int countersize;
> >
> > It's somewhat nicer to just use size_t in the kernel for these sorts
> > of data types. If you care about the exact number of bytes used by the
> > variable, types like u32 make the code more parsable.
>
> I don't see how this makes a difference here at all.
Functionally, it makes no difference. It's just a common kernel
convention to use size_t rather than unsigned int. I don't think this
is a big deal though.
> > > + err = crypto_attr_u32(tb[4], &countersize);
> > > + if (err)
> > > + goto out_put_alg;
> >
> > It's also nice to have printk's along error paths. Syslogs down the
> > road tend to be less cryptic.
>
> Please don't. That's what error return values are for. Adding
> printk's means that we'd have to think about limiting them later
> when this is opened up for user-space access.
It is usually appropriate to print something to the system log when
there is an error condition in the kernel code. That can help triage
down the road when people have troubles.
The only reason I can think of as to why we would *not* want
explanations in the syslog for failures is if frequent failures are
expected in a significant fraction of deployments.
> > > - test_cipher("ctr(aes,4,8)", ENCRYPT, aes_ctr_enc_tv_template,
> > > + test_cipher("ctr(aes,4,8,4)", ENCRYPT, aes_ctr_enc_tv_template,
> > > AES_CTR_ENC_TEST_VECTORS);
> > > - test_cipher("ctr(aes,4,8)", DECRYPT, aes_ctr_dec_tv_template,
> > > + test_cipher("ctr(aes,4,8,4)", DECRYPT, aes_ctr_dec_tv_template,
> > > AES_CTR_DEC_TEST_VECTORS);
> >
> > I have never been particularly thrilled about the the string-based
> > method of parameterizing block ciphers for in-kernel API calls.
>
> If you have a better suggestion I'd love to hear it!
Well, for calls made internally from kernel functions to kernel
functions, pretty much anything other than writing sequences of
comma-delimited parameters into to a character string.
For instance, define regular old-fashioned C data structs for the
various ciphers and modes. Fill in those structures with their
requisite parameters, and pass pointers to those structs into the
crypto API. Just as we would do with objects for any other subsystem
of the kernel.
Mike
On Tue, Oct 23, 2007 at 07:59:22PM -0500, Michael Halcrow wrote:
>
> It is usually appropriate to print something to the system log when
> there is an error condition in the kernel code. That can help triage
> down the road when people have troubles.
>
> The only reason I can think of as to why we would *not* want
> explanations in the syslog for failures is if frequent failures are
> expected in a significant fraction of deployments.
These paths can be triggered from user-space in future so printks
are not appropriate.
> > > > - test_cipher("ctr(aes,4,8)", ENCRYPT, aes_ctr_enc_tv_template,
> > > > + test_cipher("ctr(aes,4,8,4)", ENCRYPT, aes_ctr_enc_tv_template,
> > > > AES_CTR_ENC_TEST_VECTORS);
> > > > - test_cipher("ctr(aes,4,8)", DECRYPT, aes_ctr_dec_tv_template,
> > > > + test_cipher("ctr(aes,4,8,4)", DECRYPT, aes_ctr_dec_tv_template,
> > > > AES_CTR_DEC_TEST_VECTORS);
> > >
> > > I have never been particularly thrilled about the the string-based
> > > method of parameterizing block ciphers for in-kernel API calls.
> >
> > If you have a better suggestion I'd love to hear it!
>
> Well, for calls made internally from kernel functions to kernel
> functions, pretty much anything other than writing sequences of
> comma-delimited parameters into to a character string.
Again these parameters ultimately come from user-space so this
doesn't sound very practical.
Even for the kernel users how would you type these parameters?
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, Oct 24, 2007 at 09:19:05AM +0800, Herbert Xu wrote:
> These paths can be triggered from user-space in future so printks
> are not appropriate.
I am familiar with CryptoDev and Cryproc. Will you be implementing
anything similar to what these projects are currently doing? Or do you
have something else entirely in mind?
Mike
Michael Halcrow <[email protected]> wrote:
>
> I am familiar with CryptoDev and Cryproc. Will you be implementing
> anything similar to what these projects are currently doing? Or do you
> have something else entirely in mind?
I don't have any concrete plans at the moment. So any proposals
would be welcome.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Tue, Oct 23, 2007 at 03:26:29PM -0500, Joy Latten wrote:
>
> + err = crypto_attr_u32(tb[4], &countersize);
> + if (err)
> + goto out_put_alg;
> +
> /* verify size of nonce + iv + counter */
> err = -EINVAL;
> - if ((noncesize + ivsize) >= alg->cra_blocksize)
> + if (((noncesize + ivsize) > alg->cra_blocksize) ||
> + (countersize > alg->cra_blocksize))
> goto out_put_alg;
Probably should check whether
noncesize + ivsize + countersize == blocksize
if
noncesize + ivsize < blocksize
Actually let's also require that countersize >= 4 as otherwise
wrapping will be a problem. This would also weed out stream
algorithms but we wouldn't want to apply CTR to them anyway.
Otherwise this looks pretty good to me. Thanks Joy!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Fri, 2007-10-26 at 14:59 +0800, Herbert Xu wrote:
>On Tue, Oct 23, 2007 at 03:26:29PM -0500, Joy Latten wrote:
> >
> > + err = crypto_attr_u32(tb[4], &countersize);
> > + if (err)
> > + goto out_put_alg;
> > +
> > /* verify size of nonce + iv + counter */
> > err = -EINVAL;
> > - if ((noncesize + ivsize) >= alg->cra_blocksize)
> > + if (((noncesize + ivsize) > alg->cra_blocksize) ||
> > + (countersize > alg->cra_blocksize))
> > goto out_put_alg;
>
> Probably should check whether
>
> noncesize + ivsize + countersize == blocksize
>
> if
>
> noncesize + ivsize < blocksize
>
> Actually let's also require that countersize >= 4 as otherwise
> wrapping will be a problem. This would also weed out stream
> algorithms but we wouldn't want to apply CTR to them anyway.
>
Ok, I added 2 additional checks; one check
that countersize >= 4 and the other that
noncesize+ivsize+countersize >= blocksize.
I think the addition of these two checks help
to cover instances where we will want to fail
because of our inputs.
The below patch covers the additional checks as
well as the changes for GCM.
Regards,
Joy
Signed-off-by: Joy Latten <[email protected]>
diff -urpN linux-2.6.22.aead.patch/crypto/ctr.c linux-2.6.22.aead.patch2/crypto/ctr.c
--- linux-2.6.22.aead.patch/crypto/ctr.c 2007-10-09 12:12:54.000000000 -0500
+++ linux-2.6.22.aead.patch2/crypto/ctr.c 2007-10-26 14:11:46.000000000 -0500
@@ -23,6 +23,7 @@ struct ctr_instance_ctx {
struct crypto_spawn alg;
unsigned int noncesize;
unsigned int ivsize;
+ unsigned int countersize;
};
struct crypto_ctr_ctx {
@@ -186,7 +187,6 @@ static int crypto_ctr_crypt(struct blkci
unsigned long alignmask = crypto_cipher_alignmask(child);
u8 cblk[bsize + alignmask];
u8 *counterblk = (u8 *)ALIGN((unsigned long)cblk, alignmask + 1);
- unsigned int countersize;
int err;
blkcipher_walk_init(&walk, dst, src, nbytes);
@@ -198,18 +198,18 @@ static int crypto_ctr_crypt(struct blkci
memcpy(counterblk + ictx->noncesize, walk.iv, ictx->ivsize);
/* initialize counter portion of counter block */
- countersize = bsize - ictx->noncesize - ictx->ivsize;
- ctr_inc_quad(counterblk + (bsize - countersize), countersize);
+ ctr_inc_quad(counterblk + (bsize - ictx->countersize),
+ ictx->countersize);
while (walk.nbytes) {
if (walk.src.virt.addr == walk.dst.virt.addr)
nbytes = crypto_ctr_crypt_inplace(&walk, child,
counterblk,
- countersize);
+ ictx->countersize);
else
nbytes = crypto_ctr_crypt_segment(&walk, child,
counterblk,
- countersize);
+ ictx->countersize);
err = blkcipher_walk_done(desc, &walk, nbytes);
}
@@ -256,6 +256,7 @@ static struct crypto_instance *crypto_ct
struct ctr_instance_ctx *ictx;
unsigned int noncesize;
unsigned int ivsize;
+ unsigned int countersize;
int err;
err = crypto_check_attr_type(tb, CRYPTO_ALG_TYPE_BLKCIPHER);
@@ -275,9 +276,17 @@ static struct crypto_instance *crypto_ct
if (err)
goto out_put_alg;
- /* verify size of nonce + iv + counter */
+ err = crypto_attr_u32(tb[4], &countersize);
+ if (err)
+ goto out_put_alg;
+
+ /* verify size of nonce + iv + counter
+ * counter must be >= 4 bytes.
+ */
err = -EINVAL;
- if ((noncesize + ivsize) >= alg->cra_blocksize)
+ if (((noncesize + ivsize + countersize) < alg->cra_blocksize) ||
+ ((noncesize + ivsize) > alg->cra_blocksize) ||
+ (countersize > alg->cra_blocksize) || (countersize < 4))
goto out_put_alg;
inst = kzalloc(sizeof(*inst) + sizeof(*ictx), GFP_KERNEL);
@@ -287,20 +296,21 @@ static struct crypto_instance *crypto_ct
err = -ENAMETOOLONG;
if (snprintf(inst->alg.cra_name, CRYPTO_MAX_ALG_NAME,
- "ctr(%s,%u,%u)", alg->cra_name, noncesize,
- ivsize) >= CRYPTO_MAX_ALG_NAME) {
+ "ctr(%s,%u,%u,%u)", alg->cra_name, noncesize,
+ ivsize, countersize) >= CRYPTO_MAX_ALG_NAME) {
goto err_free_inst;
}
if (snprintf(inst->alg.cra_driver_name, CRYPTO_MAX_ALG_NAME,
- "ctr(%s,%u,%u)", alg->cra_driver_name, noncesize,
- ivsize) >= CRYPTO_MAX_ALG_NAME) {
+ "ctr(%s,%u,%u,%u)", alg->cra_driver_name, noncesize,
+ ivsize, countersize) >= CRYPTO_MAX_ALG_NAME) {
goto err_free_inst;
}
ictx = crypto_instance_ctx(inst);
ictx->noncesize = noncesize;
ictx->ivsize = ivsize;
+ ictx->countersize = countersize;
err = crypto_init_spawn(&ictx->alg, alg, inst,
CRYPTO_ALG_TYPE_MASK | CRYPTO_ALG_ASYNC);
diff -urpN linux-2.6.22.aead.patch/crypto/tcrypt.c linux-2.6.22.aead.patch2/crypto/tcrypt.c
--- linux-2.6.22.aead.patch/crypto/tcrypt.c 2007-10-09 11:40:58.000000000 -0500
+++ linux-2.6.22.aead.patch2/crypto/tcrypt.c 2007-10-23 14:41:46.000000000 -0500
@@ -955,9 +955,9 @@ static void do_test(void)
AES_LRW_ENC_TEST_VECTORS);
test_cipher("lrw(aes)", DECRYPT, aes_lrw_dec_tv_template,
AES_LRW_DEC_TEST_VECTORS);
- test_cipher("ctr(aes,4,8)", ENCRYPT, aes_ctr_enc_tv_template,
+ test_cipher("ctr(aes,4,8,4)", ENCRYPT, aes_ctr_enc_tv_template,
AES_CTR_ENC_TEST_VECTORS);
- test_cipher("ctr(aes,4,8)", DECRYPT, aes_ctr_dec_tv_template,
+ test_cipher("ctr(aes,4,8,4)", DECRYPT, aes_ctr_dec_tv_template,
AES_CTR_DEC_TEST_VECTORS);
//CAST5
@@ -1136,9 +1136,9 @@ static void do_test(void)
AES_LRW_ENC_TEST_VECTORS);
test_cipher("lrw(aes)", DECRYPT, aes_lrw_dec_tv_template,
AES_LRW_DEC_TEST_VECTORS);
- test_cipher("ctr(aes,4,8)", ENCRYPT, aes_ctr_enc_tv_template,
+ test_cipher("ctr(aes,4,8,4)", ENCRYPT, aes_ctr_enc_tv_template,
AES_CTR_ENC_TEST_VECTORS);
- test_cipher("ctr(aes,4,8)", DECRYPT, aes_ctr_dec_tv_template,
+ test_cipher("ctr(aes,4,8,4)", DECRYPT, aes_ctr_dec_tv_template,
AES_CTR_DEC_TEST_VECTORS);
break;
On Fri, Oct 26, 2007 at 02:22:28PM -0500, Joy Latten wrote:
>
> Ok, I added 2 additional checks; one check
> that countersize >= 4 and the other that
> noncesize+ivsize+countersize >= blocksize.
>
> I think the addition of these two checks help
> to cover instances where we will want to fail
> because of our inputs.
>
> The below patch covers the additional checks as
> well as the changes for GCM.
Looks great. Thanks Joy!
BTW, please check your patches for white space errors with
git apply --check --whitespace=error-all your-patch
I've fixed them up this time. Actually, I must've fixed them
up last time too because your patch didn't apply against my
tree :)
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt