Re: AF_ALG broken?
On Tue, Aug 09, 2016 at 08:27:17AM +0100, Russell King - ARM Linux wrote: > > From: Russell King > Subject: [PATCH] crypto: caam - fix non-hmac hashes > > Since 6de62f15b581 ("crypto: algif_hash - Require setkey before > accept(2)"), the AF_ALG interface requires userspace to provide a key > to any algorithm that has a setkey method. However, the non-HMAC > algorithms are not keyed, so setting a key is unnecessary. > > Fix this by removing the setkey method from the non-keyed hash > algorithms. > > Fixes: 6de62f15b581 ("crypto: algif_hash - Require setkey before accept(2)") > Cc: > Signed-off-by: Russell King Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AF_ALG broken?
On Tue, Aug 09, 2016 at 03:14:02PM +0800, Herbert Xu wrote: > On Tue, Aug 09, 2016 at 08:08:59AM +0100, Russell King - ARM Linux wrote: > > > > I thought I gave the commands and link to your example code. The > > openssl case is md5, though sha* also gives the same result. Your > > example code was sha1 iirc. I guess none of these would be using > > HMAC - the openssl cases used to give results compatible with the > > md5sum/ sha1sum etc userspace commands. > > > > /proc/crypto: > > > > name : md5 > > driver : md5-caam > > Right, caam is providing a setkey function for md5, which leads the > API to think that a key is required. We should fix it so that setkey > is only set for the HMAC-variant. Thanks, that works nicely again, and passes my tests. 8< From: Russell King Subject: [PATCH] crypto: caam - fix non-hmac hashes Since 6de62f15b581 ("crypto: algif_hash - Require setkey before accept(2)"), the AF_ALG interface requires userspace to provide a key to any algorithm that has a setkey method. However, the non-HMAC algorithms are not keyed, so setting a key is unnecessary. Fix this by removing the setkey method from the non-keyed hash algorithms. Fixes: 6de62f15b581 ("crypto: algif_hash - Require setkey before accept(2)") Cc: Signed-off-by: Russell King --- drivers/crypto/caam/caamhash.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c index ea284e3909ef..9d7fc9ec0b7e 100644 --- a/drivers/crypto/caam/caamhash.c +++ b/drivers/crypto/caam/caamhash.c @@ -1950,6 +1950,7 @@ caam_hash_alloc(struct caam_hash_template *template, template->name); snprintf(alg->cra_driver_name, CRYPTO_MAX_ALG_NAME, "%s", template->driver_name); + t_alg->ahash_alg.setkey = NULL; } alg->cra_module = THIS_MODULE; alg->cra_init = caam_hash_cra_init; -- 2.1.0 -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AF_ALG broken?
On Tue, Aug 09, 2016 at 08:08:59AM +0100, Russell King - ARM Linux wrote: > > I thought I gave the commands and link to your example code. The > openssl case is md5, though sha* also gives the same result. Your > example code was sha1 iirc. I guess none of these would be using > HMAC - the openssl cases used to give results compatible with the > md5sum/ sha1sum etc userspace commands. > > /proc/crypto: > > name : md5 > driver : md5-caam Right, caam is providing a setkey function for md5, which leads the API to think that a key is required. We should fix it so that setkey is only set for the HMAC-variant. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AF_ALG broken?
On Tue, Aug 09, 2016 at 11:18:20AM +0800, Herbert Xu wrote: > Russell King - ARM Linux wrote: > > Testing that code on 4.8-rc (and 4.7 fwiw) gives: > > > > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > > bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0 > > accept(3, 0, NULL) = 4 > > write(4, "abc", 3) = -1 ENOKEY (Required key not > > available) > > read(4, 0xbec50508, 20) = -1 ENOKEY (Required key not > > available) > > > > IOW, the same problem - and it seems not to be a recent regression. > > > > Since the last time I tested CESA or CAAM was back in 4.4 times, > > it's got to be something between 4.4 and 4.7. > > > > Looking at the history, my guess would be the setkey changes - > > crypto: algif_skcipher - Require setkey before accept(2) > > crypto: af_alg - Disallow bind/setkey/... after accept(2) > > crypto: af_alg - Add nokey compatibility path > > crypto: hash - Add crypto_ahash_has_setkey > > crypto: algif_hash - Require setkey before accept(2) > > This is definitely supposed to work. Basically if the algorithm > requires a key (e.g., HMAC) then you must set it. Otherwise it > should never return ENOKEY. > > Which algorithm were you testing and what does /proc/crypto say? Hi Herbert, I thought I gave the commands and link to your example code. The openssl case is md5, though sha* also gives the same result. Your example code was sha1 iirc. I guess none of these would be using HMAC - the openssl cases used to give results compatible with the md5sum/ sha1sum etc userspace commands. /proc/crypto: name : md5 driver : md5-caam module : caamhash priority : 3000 refcnt : 1 selftest : passed internal : no type : ahash async: yes blocksize: 64 digestsize : 16 name : hmac(md5) driver : hmac-md5-caam module : caamhash priority : 3000 refcnt : 1 selftest : passed internal : no type : ahash async: yes blocksize: 64 digestsize : 16 name : sha256 driver : sha256-caam module : caamhash priority : 3000 refcnt : 1 selftest : passed internal : no type : ahash async: yes blocksize: 64 digestsize : 32 name : hmac(sha256) driver : hmac-sha256-caam module : caamhash priority : 3000 refcnt : 1 selftest : passed internal : no type : ahash async: yes blocksize: 64 digestsize : 32 name : sha224 driver : sha224-caam module : caamhash priority : 3000 refcnt : 1 selftest : passed internal : no type : ahash async: yes blocksize: 64 digestsize : 28 name : hmac(sha224) driver : hmac-sha224-caam module : caamhash priority : 3000 refcnt : 1 selftest : passed internal : no type : ahash async: yes blocksize: 64 digestsize : 28 name : sha1 driver : sha1-caam module : caamhash priority : 3000 refcnt : 1 selftest : passed internal : no type : ahash async: yes blocksize: 64 digestsize : 20 name : hmac(sha1) driver : hmac-sha1-caam module : caamhash priority : 3000 refcnt : 1 selftest : passed internal : no type : ahash async: yes blocksize: 64 digestsize : 20 name : ecb(aes) driver : ecb(aes-asm) module : kernel priority : 200 refcnt : 3 selftest : passed internal : no type : blkcipher blocksize: 16 min keysize : 16 max keysize : 32 ivsize : 0 geniv: name : ghash driver : ghash-generic module : kernel priority : 100 refcnt : 1 selftest : passed internal : no type : shash blocksize: 16 digestsize : 16 name : jitterentropy_rng driver : jitterentropy_rng module : kernel priority : 100 refcnt : 1 selftest : passed internal : no type : rng seedsize : 0 name : stdrng driver : drbg_nopr_hmac_sha256 module : kernel priority : 207 refcnt : 2 selftest : passed internal : no type : rng seedsize : 0 name : stdrng driver : drbg_nopr_hmac_sha512 module : kernel priority : 206 refcnt : 1 selftest : passed internal : no type : rng seedsize : 0 name : stdrng driver : drbg_nopr_hmac_sha384 module : kernel priority : 205 refcnt : 1 selftest : passed internal : no type : rng seedsize : 0 name : stdrng driver : drbg_nopr_hmac_sha1 module : kernel priority : 204 refcnt : 1 selftest : passed internal : no type : rng seedsize : 0 name : hmac(sha256) driver : hmac(sha256-asm) module : kernel priori
Re: AF_ALG broken?
Russell King - ARM Linux wrote: > Testing that code on 4.8-rc (and 4.7 fwiw) gives: > > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0 > accept(3, 0, NULL) = 4 > write(4, "abc", 3) = -1 ENOKEY (Required key not > available) > read(4, 0xbec50508, 20) = -1 ENOKEY (Required key not > available) > > IOW, the same problem - and it seems not to be a recent regression. > > Since the last time I tested CESA or CAAM was back in 4.4 times, > it's got to be something between 4.4 and 4.7. > > Looking at the history, my guess would be the setkey changes - > crypto: algif_skcipher - Require setkey before accept(2) > crypto: af_alg - Disallow bind/setkey/... after accept(2) > crypto: af_alg - Add nokey compatibility path > crypto: hash - Add crypto_ahash_has_setkey > crypto: algif_hash - Require setkey before accept(2) This is definitely supposed to work. Basically if the algorithm requires a key (e.g., HMAC) then you must set it. Otherwise it should never return ENOKEY. Which algorithm were you testing and what does /proc/crypto say? Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AF_ALG broken?
From: Russell King - ARM Linux Date: Mon, 8 Aug 2016 23:58:51 +0100 > I don't know, but this seems to go completely against Linus' no > userspace regressions, which seems to be an absolute requirement of > all kernel development... Linus flames people for arguing against > that rule! Reading the explanation for the change given to you, it's impossible for some hash algorithms to function properly without being given the key first, and would crash. Therefore the requirement is reasonable, even though it is unfortunate that a critical piece of userspace infrastructure was coded this way. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AF_ALG broken?
On Mon, Aug 08, 2016 at 08:30:32PM +0200, Stephan Mueller wrote: > Am Montag, 8. August 2016, 20:18:32 CEST schrieb Stephan Mueller: > > Hi Stephan, > > > Am Montag, 8. August 2016, 17:44:27 CEST schrieb Russell King - ARM Linux: > > > > Hi Russell, > > > > > Hi, > > > > > > When trying to use the openssl AF_ALG module with 4.8-rc1 with imx > > > caam, I get this: > > > > > > $ OPENSSL_CONF=/shared/crypto/openssl-imx.cnf strace openssl dgst -md5 > > > > > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > > > close(3)= 0 > > > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > > > bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0 > > > accept(3, 0, NULL) = 4 > > > fstat64(0, {st_mode=S_IFREG|0755, st_size=666864, ...}) = 0 > > > mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) > > > = > > > 0xb6fab000 read(0, > > > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., > > > 8192) > > > = 8192 send(4, > > > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., > > > 8192, > > > MSG_MORE) = -1 ENOKEY (Required key not available) > > > > > > This used to work, so something in the kernel AF_ALG API has changed > > > which has broken userspace. Any ideas what's up, or where to look? > > > > This seems to be the the change added by Herbert to fix a security issue. > > This caused a similar stirr in the cryptsetup user space tool. > > > > I guess you are affected by 6de62f15b581f920ade22d758f4c338311c2f0d4 > > Just to be clear: the settings on the tfmfd must be completed before an > accept(). If make an operation on the tfmfd after the accept call, you get > the ENOKEY. As you can see from the above strace, there's no operations on fd 3 after the accept call. The only operation on the accepted fd (fd 4) is the send(). So, I'm not sure I follow what you're saying. I've also checked - there's no updates for af-alg-rr, so everyone who's using af-alg-rr must have been broken by this change - if there _are_ any users of AF_ALG with openssl. Maybe everyone just patches cryptodev into their kernel? I don't know, but this seems to go completely against Linus' no userspace regressions, which seems to be an absolute requirement of all kernel development... Linus flames people for arguing against that rule! -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AF_ALG broken?
Am Montag, 8. August 2016, 20:18:32 CEST schrieb Stephan Mueller: Hi Stephan, > Am Montag, 8. August 2016, 17:44:27 CEST schrieb Russell King - ARM Linux: > > Hi Russell, > > > Hi, > > > > When trying to use the openssl AF_ALG module with 4.8-rc1 with imx > > caam, I get this: > > > > $ OPENSSL_CONF=/shared/crypto/openssl-imx.cnf strace openssl dgst -md5 > > > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > > close(3)= 0 > > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > > bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0 > > accept(3, 0, NULL) = 4 > > fstat64(0, {st_mode=S_IFREG|0755, st_size=666864, ...}) = 0 > > mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) > > = > > 0xb6fab000 read(0, > > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., > > 8192) > > = 8192 send(4, > > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., > > 8192, > > MSG_MORE) = -1 ENOKEY (Required key not available) > > > > This used to work, so something in the kernel AF_ALG API has changed > > which has broken userspace. Any ideas what's up, or where to look? > > This seems to be the the change added by Herbert to fix a security issue. > This caused a similar stirr in the cryptsetup user space tool. > > I guess you are affected by 6de62f15b581f920ade22d758f4c338311c2f0d4 Just to be clear: the settings on the tfmfd must be completed before an accept(). If make an operation on the tfmfd after the accept call, you get the ENOKEY. This was my change to the issue: https://github.com/smuellerDD/libkcapi/commit/ 1d6c3b1b540caea784a95b1ca6e2cf38c174f585 Ciao Stephan -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AF_ALG broken?
Am Montag, 8. August 2016, 17:44:27 CEST schrieb Russell King - ARM Linux: Hi Russell, > Hi, > > When trying to use the openssl AF_ALG module with 4.8-rc1 with imx > caam, I get this: > > $ OPENSSL_CONF=/shared/crypto/openssl-imx.cnf strace openssl dgst -md5 > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > close(3)= 0 > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0 > accept(3, 0, NULL) = 4 > fstat64(0, {st_mode=S_IFREG|0755, st_size=666864, ...}) = 0 > mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = > 0xb6fab000 read(0, > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., 8192) > = 8192 send(4, > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., 8192, > MSG_MORE) = -1 ENOKEY (Required key not available) > > This used to work, so something in the kernel AF_ALG API has changed > which has broken userspace. Any ideas what's up, or where to look? This seems to be the the change added by Herbert to fix a security issue. This caused a similar stirr in the cryptsetup user space tool. I guess you are affected by 6de62f15b581f920ade22d758f4c338311c2f0d4 Ciao Stephan -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AF_ALG broken?
On Mon, Aug 08, 2016 at 01:47:33PM -0400, Jeffrey Walton wrote: > > When trying to use the openssl AF_ALG module with 4.8-rc1 with imx > > caam, I get this: > > > > $ OPENSSL_CONF=/shared/crypto/openssl-imx.cnf strace openssl dgst -md5 > > > ... > > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > > close(3)= 0 > > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > > bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0 > > accept(3, 0, NULL) = 4 > > fstat64(0, {st_mode=S_IFREG|0755, st_size=666864, ...}) = 0 > > mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = > > 0xb6fab000 > > read(0, > > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., 8192) > > = 8192 > > send(4, > > "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., 8192, > > MSG_MORE) = -1 ENOKEY (Required key not available) > > As far as I know from testing on x86, it has never worked as expected. > I believe you have to use 'sendto' and 'recvfrom' because 'send' and > 'recv' use default structures, and they configure the object > incorrectly. This used to work, because that's how I've tested my previous CAAM and Marvell CESA patches. So, this is not a case of "never worked" but is definitely a regression caused by some kernel change. There's also people's presentations that illustrate example code: https://events.linuxfoundation.org/sites/events/files/slides/lcj-2014-crypto-user.pdf which can also be found at: https://lwn.net/Articles/410833/ and that example code comes from Herbert, so one would assume that it's tested and was working. Note that it doesn't use any send*, but uses write(). Testing that code on 4.8-rc (and 4.7 fwiw) gives: socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0 accept(3, 0, NULL) = 4 write(4, "abc", 3) = -1 ENOKEY (Required key not available) read(4, 0xbec50508, 20) = -1 ENOKEY (Required key not available) IOW, the same problem - and it seems not to be a recent regression. Since the last time I tested CESA or CAAM was back in 4.4 times, it's got to be something between 4.4 and 4.7. Looking at the history, my guess would be the setkey changes - crypto: algif_skcipher - Require setkey before accept(2) crypto: af_alg - Disallow bind/setkey/... after accept(2) crypto: af_alg - Add nokey compatibility path crypto: hash - Add crypto_ahash_has_setkey crypto: algif_hash - Require setkey before accept(2) -- Rmk's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AF_ALG broken?
> When trying to use the openssl AF_ALG module with 4.8-rc1 with imx > caam, I get this: > > $ OPENSSL_CONF=/shared/crypto/openssl-imx.cnf strace openssl dgst -md5 > ... > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > close(3)= 0 > socket(PF_ALG, SOCK_SEQPACKET, 0) = 3 > bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0 > accept(3, 0, NULL) = 4 > fstat64(0, {st_mode=S_IFREG|0755, st_size=666864, ...}) = 0 > mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = > 0xb6fab000 > read(0, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., > 8192) = 8192 > send(4, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\2\0(\0\1\0\0\0\21'\2\0004\0\0\0"..., > 8192, MSG_MORE) = -1 ENOKEY (Required key not available) As far as I know from testing on x86, it has never worked as expected. I believe you have to use 'sendto' and 'recvfrom' because 'send' and 'recv' use default structures, and they configure the object incorrectly. Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
AF_ALG broken?
Hi, When trying to use the openssl AF_ALG module with 4.8-rc1 with imx caam, I get this: $ OPENSSL_CONF=/shared/crypto/openssl-imx.cnf strace openssl dgst -md5 http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html