CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issues
Subsystems (5)
PATCH 1/1 CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issues
In reply to a message not in the archiveHey all,
Here's an updated patch which addresses a couple of build issues and coding style complaints.
I still can't get it to run via testmgr I get
[ 162.407807] alg: No test for cmac(aes) (cmac(aes-generic))
Despite the fact I have an entry for cmac(aes) (much like xcbc(aes)...).
Here's the patch to bring 3.8-rc4 up with CMAC ...
--- crypto/Kconfig | 8 ++ crypto/Makefile | 1 + crypto/cmac.c | 317 +++++++++++++++++++++++++++++++++++++++++++ crypto/testmgr.c | 9 ++ crypto/testmgr.h | 52 +++++++ include/uapi/linux/pfkeyv2.h | 1 + net/xfrm/xfrm_algo.c | 17 +++ 7 files changed, 405 insertions(+) create mode 100644 crypto/cmac.c
crypto/Kconfig +8 -0
@@ -301,6 +301,14 @@ config CRYPTO_XCBC http://csrc.nist.gov/encryption/modes/proposedmodes/ xcbc-mac/xcbc-mac-spec.pdf +config CRYPTO_CMAC + tristate "CMAC support" + depends on EXPERIMENTAL + select CRYPTO_HASH + select CRYPTO_MANAGER + help + NIST CMAC cipher based MAC + config CRYPTO_VMAC tristate "VMAC support" depends on EXPERIMENTAL
crypto/Makefile +1 -0
@@ -35,6 +35,7 @@ obj-$(CONFIG_CRYPTO_USER) += crypto_user.o obj-$(CONFIG_CRYPTO_HMAC) += hmac.o obj-$(CONFIG_CRYPTO_VMAC) += vmac.o obj-$(CONFIG_CRYPTO_XCBC) += xcbc.o +obj-$(CONFIG_CRYPTO_CMAC) += cmac.o obj-$(CONFIG_CRYPTO_NULL) += crypto_null.o obj-$(CONFIG_CRYPTO_MD4) += md4.o obj-$(CONFIG_CRYPTO_MD5) += md5.o
crypto/cmac.c new file +317 -0
317 lines not on this page · raw message
crypto/testmgr.c +9 -0
@@ -2859,6 +2859,15 @@ static const struct alg_test_desc alg_test_descs[] = { } } }, { + .alg = "cmac(aes)", + .test = alg_test_hash, + .suite = { + .hash = { + .vecs = aes_cmac128_tv_template, + .count = CMAC_AES_TEST_VECTORS + } + } + }, { .alg = "xcbc(aes)", .test = alg_test_hash, .suite = {
crypto/testmgr.h +52 -0
58 lines not on this page · raw message
include/uapi/linux/pfkeyv2.h +1 -0
@@ -296,6 +296,7 @@ struct sadb_x_kmaddress {#define SADB_X_AALG_SHA2_512HMAC 7#define SADB_X_AALG_RIPEMD160HMAC 8#define SADB_X_AALG_AES_XCBC_MAC 9 +#define SADB_X_AALG_AES_CMAC_MAC 10#define SADB_X_AALG_NULL 251/* kame */ #define SADB_AALG_MAX 251
net/xfrm/xfrm_algo.c +17 -0
@@ -265,6 +265,23 @@ static struct xfrm_algo_desc aalg_list[] = { } }, { + .name = "cmac(aes)", + + .uinfo = { + .auth = { + .icv_truncbits = 96, + .icv_fullbits = 128, + } + }, + + .desc = { + .sadb_alg_id = SADB_X_AALG_AES_CMAC_MAC, + .sadb_alg_ivlen = 0, + .sadb_alg_minbits = 128, + .sadb_alg_maxbits = 128 + } +}, +{ .name = "xcbc(aes)", .uinfo = {
Signature · 1 line
1.8.1.rc2.6.g18499ba
I hesitate to reward the squeaky wheel, but in the community spirit, here goes...
Please fix the subject for future submissions. The subject should be a short, one line description of the patch. It helps if the subject includes the section of the code you are affecting. Also, if you are resending a patch after addressing review comments, change the tag to [PATCH v2] etc. will indicate that. For example:
[PATCH v2] crypto: add support for the NIST CMAC hash
This keeps the maintainers from having to hand edit your patch, which dramatically slows them down. The goal is to get to a patch that can be applied as-is after review.
Quoted 5 lines · Tom St Denis
Hey all,
Here's an updated patch which addresses a couple of build issues and coding style complaints.
Quoted 7 lines
I still can't get it to run via testmgr I get
[ 162.407807] alg: No test for cmac(aes) (cmac(aes-generic))Despite the fact I have an entry for cmac(aes) (much like xcbc(aes)...).
Here's the patch to bring 3.8-rc4 up with CMAC ...
All of this commentary should go after the '---' separator; the maintainer will have to hand edit it out otherwise. It's good information, it's just the wrong place.
There should be a short description here of the patch, if needed. You may be fine with the one line description in this case, or you could point to the RFC, etc.
Quoted 10 lines
Signed-off-by Tom St Denis <[email protected]>--- crypto/Kconfig | 8 ++ crypto/Makefile | 1 + crypto/cmac.c | 317 +++++++++++++++++++++++++++++++++++++++++++ crypto/testmgr.c | 9 ++ crypto/testmgr.h | 52 +++++++ include/uapi/linux/pfkeyv2.h | 1 + net/xfrm/xfrm_algo.c | 17 +++
You may be asked to split out the net changes into a separate patch, but since you are adding the user at the time you add the code, you may not.
Quoted 8 lines
7 files changed, 405 insertions(+) create mode 100644 crypto/cmac.ccrypto/Kconfig +0
-0
crypto/Kconfig config CRYPTO_XCBC · line 310
Quoted 10 lines
crypto/Kconfig +7
-0@@ -301,6 +301,14 @@ config CRYPTO_XCBC http://csrc.nist.gov/encryption/modes/proposedmodes/ xcbc-mac/xcbc-mac-spec.pdf +config CRYPTO_CMAC + tristate "CMAC support" + depends on EXPERIMENTAL + select CRYPTO_HASH + select CRYPTO_MANAGER + help + NIST CMAC cipher based MAC
Pointers to the docs as for XCBC above would be useful here.
crypto/cmac.c line 2
Quoted 8 lines
crypto/cmac.c new file +2
-0@@ -0,0 +1,317 @@ +/* +* Copyright (C)2006 USAGI/WIDE Project
Add your copyright info, 2013?
Quoted 9 lines
crypto/cmac.c new file +9
-0+static int crypto_cmac_digest_setkey (struct crypto_shash *parent, +const u8 *inkey,unsigned int keylen) +{ +unsigned long alignmask =crypto_shash_alignmask (parent); +struct cmac_tfm_ctx *ctx =crypto_shash_ctx (parent); +int bs =crypto_shash_blocksize (parent); +u8 *consts =PTR_ALIGN (&ctx->ctx[0 ], alignmask +1 ); +int x, y, err =0 ; +u8 msb, mask;
crypto/cmac.c line 78
Quoted 10 lines
crypto/cmac.c new file +10
-0+ +switch (bs) { +case 16 : + mask =0x87 ; +break ; +case 8 : + mask =0x1B ; +break ; +default : +return -EINVAL ;/* only support 64/128 bit block ciphers */
Checkpatch doesn't warn, but this comment would probably be preferred to be on a line by itself.
crypto/cmac.c line 91
Quoted 3 lines
crypto/cmac.c new file +3
-0+for (x =0 ; x <2 ; x++) { +/* if msb(L * u^(x+1)) = 0 then just shift, +otherwise shift and xor constant mask */
This comment is incorrectly formatted; I see checkpatch.pl from 3.7-rc4 didn't pick it up, which is interesting.
/* * if msb(L * u^(x+1)) = 0 then just shift, * otherwise shift and xor constant mask */
Though I'll note that doing
grep '/\*' crypto/*.c | grep -v '\*/' | less
provides evidence that it also commonly
/* is msb.... */
crypto/cmac.c line 98
Quoted 5 lines
crypto/cmac.c new file +5
-0+/* shift left */ +for (y =0 ; y < (bs -1 ); y++) + consts[x*bs + y] = + ((consts[x*bs + y] <<1 ) | + (consts[x*bs + y+1 ] >>7 )) &255 ;
So, here is a case where you fixed two warnings at the same time, but made one of them irrelevant in the process -- this should have braces around the single statement. checkpatch.pl complained initially because there was a one-line statement, but would not have complained if it looked like you have it now. Also, probably want a spaces in the y+1, if not the multiplication.
crypto/cmac.c line 104
Quoted 6 lines
crypto/cmac.c new file +6
-0+ + consts[x*bs + bs -1 ] = + ((consts[x*bs + bs -1 ] <<1 ) ^ + (msb ? mask :0 )) &255 ; + +/* copy up as require */
Minor English nit: required?
crypto/cmac.c line 106
Quoted 2 lines
crypto/cmac.c new file +2
-0+if (x ==0 ) +memcpy (&consts[(x+1 )*bs], &consts[x*bs], bs);
perhaps some spacing, though I'm personally OK with it.
Quoted 5 lines
crypto/testmgr.h +0
-0
crypto/testmgr.h static struct hash_testvec hmac_sha256_tv_template[] = { · line 1647
Quoted 9 lines
crypto/testmgr.h +6
-0@@ -1639,6 +1639,58 @@ static struct hash_testvec hmac_sha256_tv_template[] = { }, }; +#define CMAC_AES_TEST_VECTORS 4 + +static struct hash_testvec aes_cmac128_tv_template[] = { + { + .key ="\x2b\x7e\x15\x16\x28\xae\xd2\xa6\xab" +"\xf7\x15\x88\x09\xcf\x4f\x3c" ,
There does seem to be some inconsistencies in the test vector formatting, but it seems to be pretty common for the hex dumps to put 8 bytes in each part of the string, and to most line of the strings up.
Doing this alignment is how I found the missing \x in your first submission.
crypto/testmgr.h static struct hash_testvec hmac_sha256_tv_template[] = { · line 1655
Quoted 8 lines
crypto/testmgr.h +8
-0+ .plaintext = zeroed_string, + .digest = "\xbb\x1d\x69\x29\xe9\x59\x37\x28\x7f" + "\xa3\x7d\x12\x9b\x75\x67\x46", + .psize = 0, + .ksize = 16, + }, + + {
The rest of the file uses "}, { " between test vectors.
Because you are working in the networking code, you should cc netdev -- they need to review the following hunks. I don't know what the allocation policy is for adding algorithm numbers in pfkeyv2, but they would know if you are creating a conflict with other work.
Quoted 40 lines
include/uapi/linux/pfkeyv2.h +1
-0@@ -296,6 +296,7 @@ struct sadb_x_kmaddress {#define SADB_X_AALG_SHA2_512HMAC 7#define SADB_X_AALG_RIPEMD160HMAC 8#define SADB_X_AALG_AES_XCBC_MAC 9 +#define SADB_X_AALG_AES_CMAC_MAC 10#define SADB_X_AALG_NULL 251/* kame */ #define SADB_AALG_MAX 251net/xfrm/xfrm_algo.c +17
-0@@ -265,6 +265,23 @@ static struct xfrm_algo_desc aalg_list[] = { } }, { + .name = "cmac(aes)", + + .uinfo = { + .auth = { + .icv_truncbits = 96, + .icv_fullbits = 128, + } + }, + + .desc = { + .sadb_alg_id = SADB_X_AALG_AES_CMAC_MAC, + .sadb_alg_ivlen = 0, + .sadb_alg_minbits = 128, + .sadb_alg_maxbits = 128 + } +}, +{ .name = "xcbc(aes)", .uinfo = {
[Apologies for the dupe, fixing stupid typo for netdev address... -ENOCAFFEINE]
I hesitate to reward the squeaky wheel, but in the community spirit, here goes...
Please fix the subject for future submissions. The subject should be a short, one line description of the patch. It helps if the subject includes the section of the code you are affecting. Also, if you are resending a patch after addressing review comments, change the tag to [PATCH v2] etc. will indicate that. For example:
[PATCH v2] crypto: add support for the NIST CMAC hash
This keeps the maintainers from having to hand edit your patch, which dramatically slows them down. The goal is to get to a patch that can be applied as-is after review.
Quoted 5 lines · Tom St Denis
Hey all,
Here's an updated patch which addresses a couple of build issues and coding style complaints.
Quoted 7 lines
I still can't get it to run via testmgr I get
[ 162.407807] alg: No test for cmac(aes) (cmac(aes-generic))Despite the fact I have an entry for cmac(aes) (much like xcbc(aes)...).
Here's the patch to bring 3.8-rc4 up with CMAC ...
All of this commentary should go after the '---' separator; the maintainer will have to hand edit it out otherwise. It's good information, it's just the wrong place.
There should be a short description here of the patch, if needed. You may be fine with the one line description in this case, or you could point to the RFC, etc.
Quoted 10 lines
Signed-off-by Tom St Denis <[email protected]>--- crypto/Kconfig | 8 ++ crypto/Makefile | 1 + crypto/cmac.c | 317 +++++++++++++++++++++++++++++++++++++++++++ crypto/testmgr.c | 9 ++ crypto/testmgr.h | 52 +++++++ include/uapi/linux/pfkeyv2.h | 1 + net/xfrm/xfrm_algo.c | 17 +++
You may be asked to split out the net changes into a separate patch, but since you are adding the user at the time you add the code, you may not.
Quoted 8 lines
7 files changed, 405 insertions(+) create mode 100644 crypto/cmac.ccrypto/Kconfig +0
-0
crypto/Kconfig config CRYPTO_XCBC · line 310
Quoted 10 lines
crypto/Kconfig +7
-0@@ -301,6 +301,14 @@ config CRYPTO_XCBC http://csrc.nist.gov/encryption/modes/proposedmodes/ xcbc-mac/xcbc-mac-spec.pdf +config CRYPTO_CMAC + tristate "CMAC support" + depends on EXPERIMENTAL + select CRYPTO_HASH + select CRYPTO_MANAGER + help + NIST CMAC cipher based MAC
Pointers to the docs as for XCBC above would be useful here.
crypto/cmac.c line 2
Quoted 8 lines
crypto/cmac.c new file +2
-0@@ -0,0 +1,317 @@ +/* +* Copyright (C)2006 USAGI/WIDE Project
Add your copyright info, 2013?
Quoted 9 lines
crypto/cmac.c new file +9
-0+static int crypto_cmac_digest_setkey (struct crypto_shash *parent, +const u8 *inkey,unsigned int keylen) +{ +unsigned long alignmask =crypto_shash_alignmask (parent); +struct cmac_tfm_ctx *ctx =crypto_shash_ctx (parent); +int bs =crypto_shash_blocksize (parent); +u8 *consts =PTR_ALIGN (&ctx->ctx[0 ], alignmask +1 ); +int x, y, err =0 ; +u8 msb, mask;
crypto/cmac.c line 78
Quoted 10 lines
crypto/cmac.c new file +10
-0+ +switch (bs) { +case 16 : + mask =0x87 ; +break ; +case 8 : + mask =0x1B ; +break ; +default : +return -EINVAL ;/* only support 64/128 bit block ciphers */
Checkpatch doesn't warn, but this comment would probably be preferred to be on a line by itself.
crypto/cmac.c line 91
Quoted 3 lines
crypto/cmac.c new file +3
-0+for (x =0 ; x <2 ; x++) { +/* if msb(L * u^(x+1)) = 0 then just shift, +otherwise shift and xor constant mask */
This comment is incorrectly formatted; I see checkpatch.pl from 3.7-rc4 didn't pick it up, which is interesting.
/* * if msb(L * u^(x+1)) = 0 then just shift, * otherwise shift and xor constant mask */
Though I'll note that doing
grep '/\*' crypto/*.c | grep -v '\*/' | less
provides evidence that it also commonly
/* is msb.... */
crypto/cmac.c line 98
Quoted 5 lines
crypto/cmac.c new file +5
-0+/* shift left */ +for (y =0 ; y < (bs -1 ); y++) + consts[x*bs + y] = + ((consts[x*bs + y] <<1 ) | + (consts[x*bs + y+1 ] >>7 )) &255 ;
So, here is a case where you fixed two warnings at the same time, but made one of them irrelevant in the process -- this should have braces around the single statement. checkpatch.pl complained initially because there was a one-line statement, but would not have complained if it looked like you have it now. Also, probably want a spaces in the y+1, if not the multiplication.
crypto/cmac.c line 104
Quoted 6 lines
crypto/cmac.c new file +6
-0+ + consts[x*bs + bs -1 ] = + ((consts[x*bs + bs -1 ] <<1 ) ^ + (msb ? mask :0 )) &255 ; + +/* copy up as require */
Minor English nit: required?
crypto/cmac.c line 106
Quoted 2 lines
crypto/cmac.c new file +2
-0+if (x ==0 ) +memcpy (&consts[(x+1 )*bs], &consts[x*bs], bs);
perhaps some spacing, though I'm personally OK with it.
Quoted 5 lines
crypto/testmgr.h +0
-0
crypto/testmgr.h static struct hash_testvec hmac_sha256_tv_template[] = { · line 1647
Quoted 9 lines
crypto/testmgr.h +6
-0@@ -1639,6 +1639,58 @@ static struct hash_testvec hmac_sha256_tv_template[] = { }, }; +#define CMAC_AES_TEST_VECTORS 4 + +static struct hash_testvec aes_cmac128_tv_template[] = { + { + .key ="\x2b\x7e\x15\x16\x28\xae\xd2\xa6\xab" +"\xf7\x15\x88\x09\xcf\x4f\x3c" ,
There does seem to be some inconsistencies in the test vector formatting, but it seems to be pretty common for the hex dumps to put 8 bytes in each part of the string, and to most line of the strings up.
Doing this alignment is how I found the missing \x in your first submission.
crypto/testmgr.h static struct hash_testvec hmac_sha256_tv_template[] = { · line 1655
Quoted 8 lines
crypto/testmgr.h +8
-0+ .plaintext = zeroed_string, + .digest = "\xbb\x1d\x69\x29\xe9\x59\x37\x28\x7f" + "\xa3\x7d\x12\x9b\x75\x67\x46", + .psize = 0, + .ksize = 16, + }, + + {
The rest of the file uses "}, { " between test vectors.
Because you are working in the networking code, you should cc netdev -- they need to review the following hunks. I don't know what the allocation policy is for adding algorithm numbers in pfkeyv2, but they would know if you are creating a conflict with other work.
Quoted 40 lines
include/uapi/linux/pfkeyv2.h +1
-0@@ -296,6 +296,7 @@ struct sadb_x_kmaddress {#define SADB_X_AALG_SHA2_512HMAC 7#define SADB_X_AALG_RIPEMD160HMAC 8#define SADB_X_AALG_AES_XCBC_MAC 9 +#define SADB_X_AALG_AES_CMAC_MAC 10#define SADB_X_AALG_NULL 251/* kame */ #define SADB_AALG_MAX 251net/xfrm/xfrm_algo.c +17
-0@@ -265,6 +265,23 @@ static struct xfrm_algo_desc aalg_list[] = { } }, { + .name = "cmac(aes)", + + .uinfo = { + .auth = { + .icv_truncbits = 96, + .icv_fullbits = 128, + } + }, + + .desc = { + .sadb_alg_id = SADB_X_AALG_AES_CMAC_MAC, + .sadb_alg_ivlen = 0, + .sadb_alg_minbits = 128, + .sadb_alg_maxbits = 128 + } +}, +{ .name = "xcbc(aes)", .uinfo = {
Steffen Klassert
maintainer(Cc [email protected])
Quoted 4 lines · Tom St Denis
Hey all,
Here's an updated patch which addresses a couple of build issues and coding style complaints.
Quoted 8 lines
I still can't get it to run via testmgr I get
[ 162.407807] alg: No test for cmac(aes) (cmac(aes-generic))Despite the fact I have an entry for cmac(aes) (much like xcbc(aes)...).
Here's the patch to bring 3.8-rc4 up with CMAC ...
As already pointed out by David Dillow, the above comments should go after the '---' separator. Please use the subject line and commit message from the previous version of this patch when you resend, this was ok.
Quoted 18 lines
Signed-off-by Tom St Denis <[email protected]>--- crypto/Kconfig | 8 ++ crypto/Makefile | 1 + crypto/cmac.c | 317 +++++++++++++++++++++++++++++++++++++++++++ crypto/testmgr.c | 9 ++ crypto/testmgr.h | 52 +++++++ include/uapi/linux/pfkeyv2.h | 1 + net/xfrm/xfrm_algo.c | 17 +++ 7 files changed, 405 insertions(+) create mode 100644 crypto/cmac.ccrypto/Kconfig +0
-0
crypto/Kconfig config CRYPTO_XCBC · line 306
Quoted 6 lines
crypto/Kconfig +3
-0@@ -301,6 +301,14 @@ config CRYPTO_XCBC http://csrc.nist.gov/encryption/modes/proposedmodes/ xcbc-mac/xcbc-mac-spec.pdf +config CRYPTO_CMAC + tristate "CMAC support" + depends on EXPERIMENTAL
CONFIG_EXPERIMENTAL is going to be removed in linux-next, please remove this dependency.
crypto/Kconfig config CRYPTO_XCBC · line 311
Quoted 5 lines
crypto/Kconfig +5
-0+ select CRYPTO_HASH + select CRYPTO_MANAGER + help + NIST CMAC cipher based MAC +
Please add some more information to the help text. Everybody who knows what a 'NIST CMAC cipher based MAC' is will not need this help text, all the others are left with almost no information. This help text is for users in the first place, not for developers. So it should contain some informations about this cipher and/or some pointers where to find informations.
Please Cc me on your next version because we plan to route your patch via the ipsec-next tree into the mainline.
Thanks!
Quoting Tom St Denis <[email protected]>:
Quoted 14 lines
Hey all,
Here's an updated patch which addresses a couple of build issues and coding style complaints.
I still can't get it to run via testmgr I get
[ 162.407807] alg: No test for cmac(aes) (cmac(aes-generic))Despite the fact I have an entry for cmac(aes) (much like xcbc(aes)...).
Here's the patch to bring 3.8-rc4 up with CMAC ...
Signed-off-by Tom St Denis <[email protected]>
<snip>
include/uapi/linux/pfkeyv2.h struct sadb_x_kmaddress { · line 301
Quoted 11 lines
include/uapi/linux/pfkeyv2.h +1
-0@@ -296,6 +296,7 @@ struct sadb_x_kmaddress {#define SADB_X_AALG_SHA2_512HMAC 7#define SADB_X_AALG_RIPEMD160HMAC 8#define SADB_X_AALG_AES_XCBC_MAC 9 +#define SADB_X_AALG_AES_CMAC_MAC 10#define SADB_X_AALG_NULL 251/* kame */ #define SADB_AALG_MAX 251
Should these values be based on IANA assigned IPSEC AH transform identifiers?
https://www.iana.org/assignments/isakmp-registry/isakmp-registry.xml#isakmp-registry-6
-Jussi
----- Original Message -----
Quoted 43 lines
From: "Jussi Kivilinna" <[email protected]>
To: "Tom St Denis" <[email protected]>
Cc: [email protected], "Herbert Xu" <[email protected]>, "David Miller" <[email protected]>, [email protected], "Steffen Klassert" <[email protected]>, [email protected]
Sent: Wednesday, 23 January, 2013 9:36:44 AM
Subject: Re: [PATCH] CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issuesQuoting Tom St Denis <[email protected]>:
Quoted 17 lines
Hey all,
Here's an updated patch which addresses a couple of build issues and
coding style complaints.I still can't get it to run via testmgr I get
[ 162.407807] alg: No test for cmac(aes) (cmac(aes-generic))Despite the fact I have an entry for cmac(aes) (much like xcbc(aes)...).
Here's the patch to bring 3.8-rc4 up with CMAC ...
Signed-off-by Tom St Denis <[email protected]><snip>
include/uapi/linux/pfkeyv2.h struct sadb_x_kmaddress { · line 301
Quoted 12 lines
include/uapi/linux/pfkeyv2.h +1
-0@@ -296,6 +296,7 @@ struct sadb_x_kmaddress {#define SADB_X_AALG_SHA2_512HMAC 7#define SADB_X_AALG_RIPEMD160HMAC 8#define SADB_X_AALG_AES_XCBC_MAC 9 +#define SADB_X_AALG_AES_CMAC_MAC 10#define SADB_X_AALG_NULL 251/* kame */ #define SADB_AALG_MAX 251Should these values be based on IANA assigned IPSEC AH transform identifiers?
There is no CMAC entry apparently ... despite the fact that CMAC is a proposed RFC standard for IPsec.
It might be safer to move that to 14 since it's currently unassigned and then go through whatever channels are required to allocate it. Mostly this affects key setting. So this means my patch would break AH_RSA setkey calls (which the kernel doesn't support anyways).
Tom
Quoting Tom St Denis <[email protected]>:
Quoted 44 lines
----- Original Message -----
Quoted 43 lines
From: "Jussi Kivilinna" <[email protected]>
To: "Tom St Denis" <[email protected]>
Cc: [email protected], "Herbert Xu" <[email protected]>, "David Miller" <[email protected]>, [email protected], "Steffen Klassert" <[email protected]>, [email protected]
Sent: Wednesday, 23 January, 2013 9:36:44 AM
Subject: Re: [PATCH] CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issuesQuoting Tom St Denis <[email protected]>:
Quoted 17 lines
Hey all,
Here's an updated patch which addresses a couple of build issues and
coding style complaints.I still can't get it to run via testmgr I get
[ 162.407807] alg: No test for cmac(aes) (cmac(aes-generic))Despite the fact I have an entry for cmac(aes) (much like xcbc(aes)...).
Here's the patch to bring 3.8-rc4 up with CMAC ...
Signed-off-by Tom St Denis <[email protected]><snip>
include/uapi/linux/pfkeyv2.h struct sadb_x_kmaddress { · line 301
Quoted 12 lines
include/uapi/linux/pfkeyv2.h +1
-0@@ -296,6 +296,7 @@ struct sadb_x_kmaddress {#define SADB_X_AALG_SHA2_512HMAC 7#define SADB_X_AALG_RIPEMD160HMAC 8#define SADB_X_AALG_AES_XCBC_MAC 9 +#define SADB_X_AALG_AES_CMAC_MAC 10#define SADB_X_AALG_NULL 251/* kame */ #define SADB_AALG_MAX 251Should these values be based on IANA assigned IPSEC AH transform identifiers?
Quoted 6 lines
There is no CMAC entry apparently ... despite the fact that CMAC is a proposed RFC standard for IPsec.
It might be safer to move that to 14 since it's currently unassigned and then go through whatever channels are required to allocate it. Mostly this affects key setting. So this means my patch would break
AH_RSAsetkey calls (which the kernel doesn't support anyways).
Problem seems to be that PFKEYv2 does not quite work with IKEv2, and XFRM API should be used instead. There is new numbers assigned for IKEv2: https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xml#ikev2-parameters-7
For new SADB_X_AALG_*, I'd think you should use value from "Reserved for private use" range. Maybe 250?
But maybe better solution might be to not make AES-CMAC (or other new algorithms) available throught PFKEY API at all, just XFRM?
-Jussi
----- Original Message -----
Quoted 75 lines
From: "Jussi Kivilinna" <[email protected]>
To: "Tom St Denis" <[email protected]>
Cc: [email protected], "Herbert Xu" <[email protected]>, "David Miller" <[email protected]>, [email protected], "Steffen Klassert" <[email protected]>, [email protected]
Sent: Wednesday, 23 January, 2013 10:35:10 AM
Subject: Re: [PATCH] CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issuesQuoting Tom St Denis <[email protected]>:
Quoted 53 lines
----- Original Message -----
Quoted 47 lines
From: "Jussi Kivilinna" <[email protected]>
To: "Tom St Denis" <[email protected]>
Cc: [email protected], "Herbert Xu" <[email protected]>, "David Miller" <[email protected]>, [email protected], "Steffen Klassert" <[email protected]>, [email protected]
Sent: Wednesday, 23 January, 2013 9:36:44 AM
Subject: Re: [PATCH] CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issuesQuoting Tom St Denis <[email protected]>:
Quoted 17 lines
Hey all,
Here's an updated patch which addresses a couple of build issues and
coding style complaints.I still can't get it to run via testmgr I get
[ 162.407807] alg: No test for cmac(aes) (cmac(aes-generic))Despite the fact I have an entry for cmac(aes) (much like xcbc(aes)...).
Here's the patch to bring 3.8-rc4 up with CMAC ...
Signed-off-by Tom St Denis <[email protected]><snip>
include/uapi/linux/pfkeyv2.h struct sadb_x_kmaddress { · line 301
Quoted 12 lines
include/uapi/linux/pfkeyv2.h +1
-0@@ -296,6 +296,7 @@ struct sadb_x_kmaddress {#define SADB_X_AALG_SHA2_512HMAC 7#define SADB_X_AALG_RIPEMD160HMAC 8#define SADB_X_AALG_AES_XCBC_MAC 9 +#define SADB_X_AALG_AES_CMAC_MAC 10#define SADB_X_AALG_NULL 251/* kame */ #define SADB_AALG_MAX 251Should these values be based on IANA assigned IPSEC AH transform identifiers?
There is no CMAC entry apparently ... despite the fact that CMAC is a proposed RFC standard for IPsec.
Quoted 7 lines
It might be safer to move that to 14 since it's currently unassigned
and then go through whatever channels are required to allocate it. Mostly this affects key setting. So this means my patch would breakAH_RSAsetkey calls (which the kernel doesn't support anyways).Problem seems to be that PFKEYv2 does not quite work with IKEv2, and XFRM API should be used instead. There is new numbers assigned for IKEv2:
https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xml#ikev2-parameters-7
Quoted 5 lines
For new SADB_X_AALG_*, I'd think you should use value from "Reserved for private use" range. Maybe 250?
But maybe better solution might be to not make AES-CMAC (or other new algorithms) available throught PFKEY API at all, just XFRM?
This is where I'm getting out of my area of knowledge. Internally we use the "ip" tool to set SAs and that's about all I know about key management for IPsec in the kernel.
For testing we only use static keys since IKE is not part of our hardware offering anyways.
As long as I can use "ip" to set the keys I'm ok with whatever changes we make to this entry.
Tom
Quoted 18 lines · Jussi Kivilinna
Quoted 18 lines
Quoted 17 lines
include/uapi/linux/pfkeyv2.h struct sadb_x_kmaddress { · line 301
Quoted 12 lines
include/uapi/linux/pfkeyv2.h +1
-0@@ -296,6 +296,7 @@ struct sadb_x_kmaddress {#define SADB_X_AALG_SHA2_512HMAC 7#define SADB_X_AALG_RIPEMD160HMAC 8#define SADB_X_AALG_AES_XCBC_MAC 9 +#define SADB_X_AALG_AES_CMAC_MAC 10#define SADB_X_AALG_NULL 251/* kame */ #define SADB_AALG_MAX 251Should these values be based on IANA assigned IPSEC AH transform identifiers?
https://www.iana.org/assignments/isakmp-registry/isakmp-registry.xml#isakmp-registry-6
Quoted 8 lines
Quoted 4 lines
There is no CMAC entry apparently ... despite the fact that CMAC is a proposed RFC standard for IPsec.
It might be safer to move that to 14 since it's currently unassigned and then go through whatever channels are required to allocate it. Mostly this affects key setting. So this means my patch would break
AH_RSAsetkey calls (which the kernel doesn't support anyways).Problem seems to be that PFKEYv2 does not quite work with IKEv2, and XFRM API should be used instead. There is new numbers assigned for IKEv2: https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xml#ikev2-parameters-7
For new SADB_X_AALG_*, I'd think you should use value from "Reserved for private use" range. Maybe 250?
We can choose any value unless we do not break existing binaries. When IKE used, the daemon is responsible for translation.
--yoshfuji
Quoted 25 lines · YOSHIFUJI Hideaki
Quoted 23 lines · Jussi Kivilinna
Quoted 22 lines
Quoted 16 lines
include/uapi/linux/pfkeyv2.h struct sadb_x_kmaddress { · line 301
Quoted 12 lines
include/uapi/linux/pfkeyv2.h +1
-0@@ -296,6 +296,7 @@ struct sadb_x_kmaddress {#define SADB_X_AALG_SHA2_512HMAC 7#define SADB_X_AALG_RIPEMD160HMAC 8#define SADB_X_AALG_AES_XCBC_MAC 9 +#define SADB_X_AALG_AES_CMAC_MAC 10#define SADB_X_AALG_NULL 251/* kame */ #define SADB_AALG_MAX 251Should these values be based on IANA assigned IPSEC AH transform identifiers?
There is no CMAC entry apparently ... despite the fact that CMAC is a proposed RFC standard for IPsec.
It might be safer to move that to 14 since it's currently unassigned and then go through whatever channels are required to allocate it. Mostly this affects key setting. So this means my patch would break
AH_RSAsetkey calls (which the kernel doesn't support anyways).
Quoted 7 lines
Quoted 3 lines
Problem seems to be that PFKEYv2 does not quite work with IKEv2, and XFRM API should be used instead. There is new numbers assigned for IKEv2: https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xml#ikev2-parameters-7
For new SADB_X_AALG_*, I'd think you should use value from "Reserved for private use" range. Maybe 250?
We can choose any value unless we do not break existing binaries. When IKE used, the daemon is responsible for translation.
I meant, we can choose any values "if" we do not break ...
--yoshfuji
Quoting YOSHIFUJI Hideaki <[email protected]>:
Quoted 28 lines
Quoted 27 lines · YOSHIFUJI Hideaki
Quoted 25 lines · Jussi Kivilinna
Quoted 16 lines
Quoted 16 lines
include/uapi/linux/pfkeyv2.h struct sadb_x_kmaddress { · line 301
Quoted 12 lines
include/uapi/linux/pfkeyv2.h +1
-0@@ -296,6 +296,7 @@ struct sadb_x_kmaddress {#define SADB_X_AALG_SHA2_512HMAC 7#define SADB_X_AALG_RIPEMD160HMAC 8#define SADB_X_AALG_AES_XCBC_MAC 9 +#define SADB_X_AALG_AES_CMAC_MAC 10#define SADB_X_AALG_NULL 251/* kame */ #define SADB_AALG_MAX 251Should these values be based on IANA assigned IPSEC AH transform identifiers?
Quoted 6 lines
There is no CMAC entry apparently ... despite the fact that CMAC is a proposed RFC standard for IPsec.
It might be safer to move that to 14 since it's currently unassigned and then go through whatever channels are required to allocate it. Mostly this affects key setting. So this means my patch would break
AH_RSAsetkey calls (which the kernel doesn't support anyways).Problem seems to be that PFKEYv2 does not quite work with IKEv2, and XFRM API should be used instead. There is new numbers assigned for IKEv2: https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xml#ikev2-parameters-7
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
For new SADB_X_AALG_*, I'd think you should use value from "Reserved for private use" range. Maybe 250?
We can choose any value unless we do not break existing binaries. When IKE used, the daemon is responsible for translation.
I meant, we can choose any values "if" we do not break ...
Ok, so giving '10' to AES-CMAC is fine after all?
And if I'd want to add Camellia-CTR and Camellia-CCM support, I can choose next free numbers from SADB_X_EALG_*?
-Jussi
Steffen Klassert
maintainerQuoted 7 lines · Jussi Kivilinna
Problem seems to be that PFKEYv2 does not quite work with IKEv2, and
XFRM API should be used instead. There is new numbers assigned for
IKEv2: https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xml#ikev2-parameters-7For new SADB_X_AALG_*, I'd think you should use value from "Reserved for private use" range. Maybe 250?
This would be an option, but we have just a few slots for private algorithms.
Quoted 4 lines
But maybe better solution might be to not make AES-CMAC (or other new algorithms) available throught PFKEY API at all, just XFRM?
It is probably the best to make new algorithms unavailable for pfkey as long as they have no official ikev1 iana transform identifier.
But how to do that? Perhaps we can assign SADB_X_AALG_NOPFKEY to the private value 255 and return -EINVAL if pfkey tries to register such an algorithm. The netlink interface does not use these identifiers, everything should work as expected. So it should be possible to use these algoritms with iproute2 and the most modern ike deamons.
Quoting Steffen Klassert <[email protected]>:
Quoted 21 lines · Jussi Kivilinna
Quoted 8 lines
Problem seems to be that PFKEYv2 does not quite work with IKEv2, and XFRM API should be used instead. There is new numbers assigned for IKEv2:
https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xml#ikev2-parameters-7For new SADB_X_AALG_*, I'd think you should use value from "Reserved for private use" range. Maybe 250?
This would be an option, but we have just a few slots for private algorithms.
Quoted 4 lines
But maybe better solution might be to not make AES-CMAC (or other new algorithms) available throught PFKEY API at all, just XFRM?
It is probably the best to make new algorithms unavailable for pfkey as long as they have no official ikev1 iana transform identifier.
Quoted 6 lines
But how to do that? Perhaps we can assign
SADB_X_AALG_NOPFKEYto the private value 255 and return-EINVALif pfkey tries to register such an algorithm. The netlink interface does not use these identifiers, everything should work as expected. So it should be possible to use these algoritms with iproute2 and the most modern ike deamons.
Maybe it would be cleaner to not mess with pfkeyv2.h at all, but instead mark algorithms that do not support pfkey with flag. See patch below.
Then I started looking up if sadb_alg_id is being used somewhere outside pfkey. Seems that its value is just being copied around.. but at "http://lxr.linux.no/linux+v3.7/net/xfrm/xfrm_policy.c#L1991" it's used as bit-index. So do larger values than 31 break some stuff? Can multiple algorithms have same sadb_alg_id value? Also in af_key.c, sadb_alg_id being used as bit-index.
-Jussi
---
ONLY COMPILE TESTED!
--- include/net/xfrm.h | 5 +++-- net/key/af_key.c | 39 +++++++++++++++++++++++++++++++-------- net/xfrm/xfrm_algo.c | 12 ++++++------ 3 files changed, 40 insertions(+), 16 deletions(-)
include/net/xfrm.h +3 -2
@@ -1320,6 +1320,7 @@ struct xfrm_algo_desc {char *name;char *compat; u8 available:1; + u8 sadb_disabled:1;union {struct xfrm_algo_aead_info aead;struct xfrm_algo_auth_info auth;
@@ -1561,8 +1562,8 @@ extern void xfrm_input_init(void);extern int xfrm_parse_spi (struct sk_buff *skb,u8 nexthdr,__be32 *spi,__be32 *seq);extern void xfrm_probe_algs (void ); -extern int xfrm_count_auth_supported (void ); -extern int xfrm_count_enc_supported (void ); +extern int xfrm_count_sadb_auth_supported (void ); +extern int xfrm_count_sadb_enc_supported (void );extern struct xfrm_algo_desc *xfrm_aalg_get_byidx (unsigned int idx);extern struct xfrm_algo_desc *xfrm_ealg_get_byidx (unsigned int idx);extern struct xfrm_algo_desc *xfrm_aalg_get_byid (int alg_id);
net/key/af_key.c +31 -8
@@ -816,18 +816,21 @@ static struct sk_buff *__pfkey_xfrm_state2msg(const struct xfrm_state *x, sa->sadb_sa_auth =0 ;if (x->aalg) {struct xfrm_algo_desc *a =xfrm_aalg_get_byname (x->aalg->alg_name,0 ); - sa->sadb_sa_auth = a ? a->desc.sadb_alg_id :0 ; + sa->sadb_sa_auth = (a && !a->sadb_disabled) ? + a->desc.sadb_alg_id :0 ; } sa->sadb_sa_encrypt =0 ;BUG_ON (x->ealg && x->calg);if (x->ealg) {struct xfrm_algo_desc *a =xfrm_ealg_get_byname (x->ealg->alg_name,0 ); - sa->sadb_sa_encrypt = a ? a->desc.sadb_alg_id :0 ; + sa->sadb_sa_encrypt = (a && !a->sadb_disabled) ? + a->desc.sadb_alg_id :0 ; }/* KAME compatible: sadb_sa_encrypt is overloaded with calg id */ if (x->calg) {struct xfrm_algo_desc *a =xfrm_calg_get_byname (x->calg->alg_name,0 ); - sa->sadb_sa_encrypt = a ? a->desc.sadb_alg_id :0 ; + sa->sadb_sa_encrypt = (a && !a->sadb_disabled) ? + a->desc.sadb_alg_id :0 ; } sa->sadb_sa_flags =0 ;
@@ -1138,7 +1141,7 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net,if (sa->sadb_sa_auth) {int keysize =0 ;struct xfrm_algo_desc *a =xfrm_aalg_get_byid (sa->sadb_sa_auth); -if (!a) { +if (!a || a->sadb_disabled) { err = -ENOSYS ; goto out; }
@@ -1160,7 +1163,7 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net,if (sa->sadb_sa_encrypt) {if (hdr->sadb_msg_satype ==SADB_X_SATYPE_IPCOMP ) {struct xfrm_algo_desc *a =xfrm_calg_get_byid (sa->sadb_sa_encrypt); -if (!a) { +if (!a || a->sadb_disabled) { err = -ENOSYS ; goto out; }
@@ -1172,7 +1175,7 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net, }else {int keysize =0 ;struct xfrm_algo_desc *a =xfrm_ealg_get_byid (sa->sadb_sa_encrypt); -if (!a) { +if (!a || a->sadb_disabled) { err = -ENOSYS ; goto out; }
@@ -1578,13 +1581,13 @@ static struct sk_buff *compose_sadb_supported(const struct sadb_msg *orig,struct sadb_msg *hdr;int len, auth_len, enc_len, i; - auth_len =xfrm_count_auth_supported (); + auth_len =xfrm_count_sadb_auth_supported ();if (auth_len) { auth_len *=sizeof (struct sadb_alg ); auth_len +=sizeof (struct sadb_supported ); } - enc_len =xfrm_count_enc_supported (); + enc_len =xfrm_count_sadb_enc_supported ();if (enc_len) { enc_len *=sizeof (struct sadb_alg ); enc_len +=sizeof (struct sadb_supported );
@@ -1615,6 +1618,8 @@ static struct sk_buff *compose_sadb_supported(const struct sadb_msg *orig,struct xfrm_algo_desc *aalg =xfrm_aalg_get_byidx (i);if (!aalg)break ; +if (aalg->sadb_disabled) +continue ;if (aalg->available) *ap++ = aalg->desc; }
@@ -1634,6 +1639,8 @@ static struct sk_buff *compose_sadb_supported(const struct sadb_msg *orig,struct xfrm_algo_desc *ealg =xfrm_ealg_get_byidx (i);if (!ealg)break ; +if (ealg->sadb_disabled) +continue ;if (ealg->available) *ap++ = ealg->desc; }
@@ -2825,6 +2832,8 @@ static int count_ah_combs(const struct xfrm_tmpl *t)const struct xfrm_algo_desc *aalg =xfrm_aalg_get_byidx (i);if (!aalg)break ; +if (aalg->sadb_disabled) +continue ;if (aalg_tmpl_set (t, aalg) && aalg->available) sz +=sizeof (struct sadb_comb ); }
@@ -2840,6 +2849,9 @@ static int count_esp_combs(const struct xfrm_tmpl *t)if (!ealg)break ; +if (ealg->sadb_disabled) +continue ; +if (!(ealg_tmpl_set (t, ealg) && ealg->available))continue ;
@@ -2848,6 +2860,9 @@ static int count_esp_combs(const struct xfrm_tmpl *t)if (!aalg)break ; +if (aalg->sadb_disabled) +continue ; +if (aalg_tmpl_set (t, aalg) && aalg->available) sz +=sizeof (struct sadb_comb ); }
@@ -2871,6 +2886,9 @@ static void dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)if (!aalg)break ; +if (aalg->sadb_disabled) +continue ; +if (aalg_tmpl_set (t, aalg) && aalg->available) {struct sadb_comb *c; c = (struct sadb_comb *)skb_put (skb,sizeof (struct sadb_comb ));
@@ -2903,6 +2921,9 @@ static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)if (!ealg)break ; +if (ealg->sadb_disabled) +continue ; +if (!(ealg_tmpl_set (t, ealg) && ealg->available))continue ;
@@ -2911,6 +2932,8 @@ static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)const struct xfrm_algo_desc *aalg =xfrm_aalg_get_byidx (k);if (!aalg)break ; +if (aalg->sadb_disabled) +continue ;if (!(aalg_tmpl_set (t, aalg) && aalg->available))continue ; c = (struct sadb_comb *)skb_put (skb,sizeof (struct sadb_comb ));
net/xfrm/xfrm_algo.c +6 -6
@@ -713,27 +713,27 @@ void xfrm_probe_algs(void) }EXPORT_SYMBOL_GPL (xfrm_probe_algs); -int xfrm_count_auth_supported (void ) +int xfrm_count_sadb_auth_supported (void ) {int i, n;for (i =0 , n =0 ; i <aalg_entries (); i++) -if (aalg_list[i].available) +if (aalg_list[i].available && !aalg_list[i].sadb_disabled) n++;return n; } -EXPORT_SYMBOL_GPL (xfrm_count_auth_supported); +EXPORT_SYMBOL_GPL (xfrm_count_sadb_auth_supported); -int xfrm_count_enc_supported (void ) +int xfrm_count_sadb_enc_supported (void ) {int i, n;for (i =0 , n =0 ; i <ealg_entries (); i++) -if (ealg_list[i].available) +if (ealg_list[i].available && !ealg_list[i].sadb_disabled) n++;return n; } -EXPORT_SYMBOL_GPL (xfrm_count_enc_supported); +EXPORT_SYMBOL_GPL (xfrm_count_sadb_enc_supported);#if defined(CONFIG_INET_ESP ) || defined(CONFIG_INET_ESP_MODULE ) || defined(CONFIG_INET6_ESP ) || defined(CONFIG_INET6_ESP_MODULE )
Steffen Klassert
maintainerQuoted 3 lines · Jussi Kivilinna
Maybe it would be cleaner to not mess with
pfkeyv2.hat all, but instead mark algorithms that do not support pfkey with flag. See patch below.
Yes, would be an option too. I would be fine with that, but let's here if someone else has an opinion on this. Anyway, we need a solution to integrate Tom's patch soon.
Quoted 2 lines
Then I started looking up if
sadb_alg_idis being used somewhere outside pfkey. Seems that its value is just being copied around.. but at "http://lxr.linux.no/linux+v3.7/net/xfrm/xfrm_policy.c#L1991" it's used as bit-index. So do larger values than 31 break some stuff? Can multiple algorithms have samesadb_alg_idvalue? Also inaf_key.c,sadb_alg_idbeing used as bit-index.
Herbert tried to address this already in git commit c5d18e984 ([IPSEC]: Fix catch-22 with algorithm IDs above 31) some years ago.
But this looks still messy. If the aalgos, ealgos and calgos mask is ~0, we allow all algorithms. If this is not the case, xfrm and pfkey check the aalgos mask against the algorithm ID, only pfkey checks the ealgo mask and noone checks the calgos mask.
----- Original Message -----
Quoted 9 lines
From: "Steffen Klassert" <[email protected]>
To: "Jussi Kivilinna" <[email protected]>
Cc: "Herbert Xu" <[email protected]>, [email protected], [email protected], [email protected], "Tom St Denis" <[email protected]>, "David Miller" <[email protected]>
Sent: Thursday, 24 January, 2013 7:32:10 AM
Subject: Re: [PATCH] CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issuesQuoted 1 line · Jussi Kivilinna
Quoted 8 lines
Quoted 4 lines
Maybe it would be cleaner to not mess with
pfkeyv2.hat all, but instead mark algorithms that do not support pfkey with flag. See patch below.Yes, would be an option too. I would be fine with that, but let's here if someone else has an opinion on this. Anyway, we need a solution to integrate Tom's patch soon.
Has anyone addressed the testmgr issues too? I wasn't able to run the self-tests so I don't even know if the vectors were copied correctly.
Also a question for the netdev folk, in order to be timely would it be acceptable to patch ah4 and then ah6 with the AEAD changes? Or would the team require both to be patched simultaneously?
Tom
Steffen Klassert
maintainerQuoted 3 lines · Tom St Denis
Also a question for the netdev folk, in order to be timely would it be acceptable to patch ah4 and then ah6 with the AEAD changes? Or would the team require both to be patched simultaneously?
We would need patches for both, but the changes to ah4/ah6 should e very similar.
----- Original Message -----
Quoted 9 lines
From: "Steffen Klassert" <[email protected]>
To: "Tom St Denis" <[email protected]>
Cc: "Herbert Xu" <[email protected]>, [email protected], [email protected], [email protected], "David Miller" <[email protected]>, "Jussi Kivilinna" <[email protected]>
Sent: Thursday, 24 January, 2013 7:52:34 AM
Subject: Re: [PATCH] CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issuesQuoted 1 line · Tom St Denis
Quoted 7 lines
Quoted 4 lines
Also a question for the netdev folk, in order to be timely would it be acceptable to patch ah4 and then ah6 with the AEAD changes? Or would the team require both to be patched simultaneously?
We would need patches for both, but the changes to ah4/ah6 should e very similar.
I suspect, I just don't know how my time table will look like so I was trying to not block the submission of one by the other. Fortunately, I don't need hardware to test these changes so I can probably skunkwork them from home.
Tom
Steffen Klassert
maintainerQuoted 3 lines · Jussi Kivilinna
Maybe it would be cleaner to not mess with
pfkeyv2.hat all, but instead mark algorithms that do not support pfkey with flag. See patch below.
As nobody seems to have another opinion, we could go either with your approach, or we can invert the logic and mark all existing algorithms as pfkey supported. Then we would not need to bother about pfkey again.
I'd be fine with both. Do you want to submit a patch?
Quoting Steffen Klassert <[email protected]>:
Quoted 11 lines · Jussi Kivilinna
Quoted 3 lines
Maybe it would be cleaner to not mess with
pfkeyv2.hat all, but instead mark algorithms that do not support pfkey with flag. See patch below.As nobody seems to have another opinion, we could go either with your approach, or we can invert the logic and mark all existing algorithms as pfkey supported. Then we would not need to bother about pfkey again.
I'd be fine with both. Do you want to submit a patch?
Ok, I'll invert the logic and send new patch.
-Jussi