Re: AF_ALG zero-size hash fails

[Herbert Xu]

On Tue, Aug 09, 2016 at 05:45:25PM +0800, Herbert Xu wrote:
>
> > tested with the Freescale CAAM driver with SHA1 and MD5 hashes, and
> > the ARM SHA1 shash implementation.  Should there at least be a single
> > write to the socket (of zero size) in this case, or should the kernel
> > return the correct hash on the first read without a preceding
> > write/send?
> 
> It's an oversight in the algif_hash code.  I'll get it fixed.

---8<---
Subject: crypto: algif_hash - Handle NULL hashes correctly

Right now attempting to read an empty hash simply returns zeroed
bytes, this patch corrects this by calling the digest function
using an empty input.

Reported-by: Russell King - ARM Linux 
Signed-off-by: Herbert Xu 

diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 68a5cea..2d8466f 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -39,6 +39,37 @@ struct algif_hash_tfm {
bool has_key;
 };
 
+static int hash_alloc_result(struct sock *sk, struct hash_ctx *ctx)
+{
+   unsigned ds;
+
+   if (ctx->result)
+   return 0;
+
+   ds = crypto_ahash_digestsize(crypto_ahash_reqtfm(>req));
+
+   ctx->result = sock_kmalloc(sk, ds, GFP_KERNEL);
+   if (!ctx->result)
+   return -ENOMEM;
+
+   memset(ctx->result, 0, ds);
+
+   return 0;
+}
+
+static void hash_free_result(struct sock *sk, struct hash_ctx *ctx)
+{
+   unsigned ds;
+
+   if (!ctx->result)
+   return;
+
+   ds = crypto_ahash_digestsize(crypto_ahash_reqtfm(>req));
+
+   sock_kzfree_s(sk, ctx->result, ds);
+   ctx->result = NULL;
+}
+
 static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
size_t ignored)
 {
@@ -54,6 +85,9 @@ static int hash_sendmsg(struct socket *sock, struct msghdr 
*msg,
 
lock_sock(sk);
if (!ctx->more) {
+   if ((msg->msg_flags & MSG_MORE))
+   hash_free_result(sk, ctx);
+
err = af_alg_wait_for_completion(crypto_ahash_init(>req),
>completion);
if (err)
@@ -90,6 +124,10 @@ static int hash_sendmsg(struct socket *sock, struct msghdr 
*msg,
 
ctx->more = msg->msg_flags & MSG_MORE;
if (!ctx->more) {
+   err = hash_alloc_result(sk, ctx);
+   if (err)
+   goto unlock;
+
ahash_request_set_crypt(>req, NULL, ctx->result, 0);
err = af_alg_wait_for_completion(crypto_ahash_final(>req),
 >completion);
@@ -116,6 +154,13 @@ static ssize_t hash_sendpage(struct socket *sock, struct 
page *page,
sg_init_table(ctx->sgl.sg, 1);
sg_set_page(ctx->sgl.sg, page, size, offset);
 
+   if (!(flags & MSG_MORE)) {
+   err = hash_alloc_result(sk, ctx);
+   if (err)
+   goto unlock;
+   } else if (!ctx->more)
+   hash_free_result(sk, ctx);
+
ahash_request_set_crypt(>req, ctx->sgl.sg, ctx->result, size);
 
if (!(flags & MSG_MORE)) {
@@ -153,6 +198,7 @@ static int hash_recvmsg(struct socket *sock, struct msghdr 
*msg, size_t len,
struct alg_sock *ask = alg_sk(sk);
struct hash_ctx *ctx = ask->private;
unsigned ds = crypto_ahash_digestsize(crypto_ahash_reqtfm(>req));
+   bool result;
int err;
 
if (len > ds)
@@ -161,17 +207,29 @@ static int hash_recvmsg(struct socket *sock, struct 
msghdr *msg, size_t len,
msg->msg_flags |= MSG_TRUNC;
 
lock_sock(sk);
+   result = ctx->result;
+   err = hash_alloc_result(sk, ctx);
+   if (err)
+   goto unlock;
+
+   ahash_request_set_crypt(>req, NULL, ctx->result, 0);
+
if (ctx->more) {
ctx->more = 0;
-   ahash_request_set_crypt(>req, NULL, ctx->result, 0);
err = af_alg_wait_for_completion(crypto_ahash_final(>req),
 >completion);
if (err)
goto unlock;
+   } else if (!result) {
+   err = af_alg_wait_for_completion(
+   crypto_ahash_digest(>req),
+   >completion);
}
 
err = memcpy_to_msg(msg, ctx->result, len);
 
+   hash_free_result(sk, ctx);
+
 unlock:
release_sock(sk);
 
@@ -394,8 +452,7 @@ static void hash_sock_destruct(struct sock *sk)
struct alg_sock *ask = alg_sk(sk);
struct hash_ctx *ctx = ask->private;
 
-   sock_kzfree_s(sk, ctx->result,
- crypto_ahash_digestsize(crypto_ahash_reqtfm(>req)));
+   hash_free_result(sk, ctx);
sock_kfree_s(sk, ctx, ctx->len);
af_alg_release_parent(sk);
 }
@@ -407,20 +464,12 @@ static int hash_accept_parent_nokey(void *private, struct 
sock *sk)
struct 

Re: AF_ALG zero-size hash fails

[Herbert Xu]

On Tue, Aug 09, 2016 at 10:18:34AM +0100, Russell King - ARM Linux wrote:
> Hi,
> 
> While testing AF_ALG with openssl af-alg-rr, I've found that:
> 
> OPENSSL_CONF=/shared/crypto/openssl-imx.cnf openssl dgst -sha1  
> fails with a zero hash result:
> 
> 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_IFCHR|0666, st_rdev=makedev(1, 3), ...}) = 0
> ioctl(0, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS, 
> 0xbed50d5c) = -1 ENOTTY (Inappropriate ioctl for device)
> mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 
> 0xb6fe4000
> read(0, "", 8192)   = 0
> read(4, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", 20) = 20
> close(4)= 0
> close(3)= 0
> 
> tested with the Freescale CAAM driver with SHA1 and MD5 hashes, and
> the ARM SHA1 shash implementation.  Should there at least be a single
> write to the socket (of zero size) in this case, or should the kernel
> return the correct hash on the first read without a preceding
> write/send?

It's an oversight in the algif_hash code.  I'll get it fixed.

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

AF_ALG zero-size hash fails

[Russell King - ARM Linux]

Hi,

While testing AF_ALG with openssl af-alg-rr, I've found that:

OPENSSL_CONF=/shared/crypto/openssl-imx.cnf openssl dgst -sha1 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