Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Paul Menzel]

Dear Caleb,


Thank you for the report. Linux has a no regression policy, so the 
correct forum to report this to is the Linux kernel folks. I am adding 
the crypto and stable folks to the receiver list.

Am 26.08.20 um 07:51 schrieb caljorden@hotmail.com:

> I wanted to note an issue that I have hit with iwd when I upgraded to
> the Linux 5.8.3 stable kernel.  My office network uses WPA Enterprise
> with EAP-PEAPv0 + MSCHAPv2.  When using this office network,
> upgrading to Linux 5.8.3 caused my system to refuse to associate
> successfully to the network.  I get the following in my dmesg logs:
> 
> [   40.846535] wlan0: authenticate with <redacted>:60
> [   40.850570] wlan0: send auth to <redacted>:60 (try 1/3)
> [   40.854627] wlan0: authenticated
> [   40.855992] wlan0: associate with <redacted>:60 (try 1/3)
> [   40.860450] wlan0: RX AssocResp from <redacted>:60 (capab=0x411 status=0 aid=11)
> [   40.861620] wlan0: associated
> [   41.886503] wlan0: deauthenticating from <redacted>:60 by local choice (Reason: 23=IEEE8021X_FAILED)
> [   42.360127] wlan0: authenticate with <redacted>:22
> [   42.364584] wlan0: send auth to <redacted>:22 (try 1/3)
> [   42.370821] wlan0: authenticated
> [   42.372658] wlan0: associate with <redacted>:22 (try 1/3)
> [   42.377426] wlan0: RX AssocResp from <redacted>:22 (capab=0x411 status=0 aid=15)
> [   42.378607] wlan0: associated
> [   43.402009] wlan0: deauthenticating from <redacted>:22 by local choice (Reason: 23=IEEE8021X_FAILED)
> [   43.875921] wlan0: authenticate with <redacted>:60
> [   43.879988] wlan0: send auth to <redacted>:60 (try 1/3)
> [   43.886244] wlan0: authenticated
> [   43.889273] wlan0: associate with <redacted>:60 (try 1/3)
> [   43.894586] wlan0: RX AssocResp from <redacted>:60 (capab=0x411 status=0 aid=11)
> [   43.896077] wlan0: associated
> [   44.918504] wlan0: deauthenticating from <redacted>:60 by local choice (Reason: 23=IEEE8021X_FAILED)
> 
> This continues as long as I let iwd run.
> 
> I downgraded back to Linux 5.8.2, and verified that everything works
> as expected.  I also tried using Linux 5.8.3 on a different system at
> my home, which uses WPA2-PSK.  It worked fine (though it uses an
> Atheros wireless card instead of an Intel card - but I assume that is
> irrelevant).
> 
> I decided to try to figure out what caused the issue in the changes
> for Linux 5.8.3.  I assumed that it was something that changed in the
> crypto interface, which limited my bisection to a very few commits.
> Sure enough, I found that if I revert commit
> e91d82703ad0bc68942a7d91c1c3d993e3ad87f0 (crypto: algif_aead - Only
> wake up when ctx->more is zero), the problem goes away and I am able
> to associate to my WPA Enterprise network successfully, and use it.
> I found that in order to revert this commit, I also first had to
> revert 465c03e999102bddac9b1e132266c232c5456440 (crypto: af_alg - Fix
> regression on empty requests), because the two commits have coupled
> changes.
> 
> I normally would have assumed that this should be sent to the kernel
> list, but I thought I would first mention it here because of what I
> found in some email threads on the Linux-Crypto list about the crypto
> interfaces to the kernel being sub-optimal and needing to be fixed.
> The changes in these commits look like they are just trying to fix
> what could be broken interfaces, so I thought that it would make
> sense to see what the iwd team thinks about the situation first.
> 
> The wireless card I was using during this testing is an Intel
> Wireless 3165 (rev 81).  If there is any additional information I
> could help provide, please let me know.

It’d be great, if you verified, if the problem occurs with Linus’ master 
branch too.


Kind regards,

Paul

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Ard Biesheuvel]

On Wed, 26 Aug 2020 at 08:18, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
>
> Dear Caleb,
>
>
> Thank you for the report. Linux has a no regression policy, so the
> correct forum to report this to is the Linux kernel folks. I am adding
> the crypto and stable folks to the receiver list.
>
> Am 26.08.20 um 07:51 schrieb caljorden@hotmail.com:
>
> > I wanted to note an issue that I have hit with iwd when I upgraded to
> > the Linux 5.8.3 stable kernel.  My office network uses WPA Enterprise
> > with EAP-PEAPv0 + MSCHAPv2.  When using this office network,
> > upgrading to Linux 5.8.3 caused my system to refuse to associate
> > successfully to the network.  I get the following in my dmesg logs:
> >
> > [   40.846535] wlan0: authenticate with <redacted>:60
> > [   40.850570] wlan0: send auth to <redacted>:60 (try 1/3)
> > [   40.854627] wlan0: authenticated
> > [   40.855992] wlan0: associate with <redacted>:60 (try 1/3)
> > [   40.860450] wlan0: RX AssocResp from <redacted>:60 (capab=0x411 status=0 aid=11)
> > [   40.861620] wlan0: associated
> > [   41.886503] wlan0: deauthenticating from <redacted>:60 by local choice (Reason: 23=IEEE8021X_FAILED)
> > [   42.360127] wlan0: authenticate with <redacted>:22
> > [   42.364584] wlan0: send auth to <redacted>:22 (try 1/3)
> > [   42.370821] wlan0: authenticated
> > [   42.372658] wlan0: associate with <redacted>:22 (try 1/3)
> > [   42.377426] wlan0: RX AssocResp from <redacted>:22 (capab=0x411 status=0 aid=15)
> > [   42.378607] wlan0: associated
> > [   43.402009] wlan0: deauthenticating from <redacted>:22 by local choice (Reason: 23=IEEE8021X_FAILED)
> > [   43.875921] wlan0: authenticate with <redacted>:60
> > [   43.879988] wlan0: send auth to <redacted>:60 (try 1/3)
> > [   43.886244] wlan0: authenticated
> > [   43.889273] wlan0: associate with <redacted>:60 (try 1/3)
> > [   43.894586] wlan0: RX AssocResp from <redacted>:60 (capab=0x411 status=0 aid=11)
> > [   43.896077] wlan0: associated
> > [   44.918504] wlan0: deauthenticating from <redacted>:60 by local choice (Reason: 23=IEEE8021X_FAILED)
> >
> > This continues as long as I let iwd run.
> >
> > I downgraded back to Linux 5.8.2, and verified that everything works
> > as expected.  I also tried using Linux 5.8.3 on a different system at
> > my home, which uses WPA2-PSK.  It worked fine (though it uses an
> > Atheros wireless card instead of an Intel card - but I assume that is
> > irrelevant).
> >
> > I decided to try to figure out what caused the issue in the changes
> > for Linux 5.8.3.  I assumed that it was something that changed in the
> > crypto interface, which limited my bisection to a very few commits.
> > Sure enough, I found that if I revert commit
> > e91d82703ad0bc68942a7d91c1c3d993e3ad87f0 (crypto: algif_aead - Only
> > wake up when ctx->more is zero), the problem goes away and I am able
> > to associate to my WPA Enterprise network successfully, and use it.
> > I found that in order to revert this commit, I also first had to
> > revert 465c03e999102bddac9b1e132266c232c5456440 (crypto: af_alg - Fix
> > regression on empty requests), because the two commits have coupled
> > changes.
> >
> > I normally would have assumed that this should be sent to the kernel
> > list, but I thought I would first mention it here because of what I
> > found in some email threads on the Linux-Crypto list about the crypto
> > interfaces to the kernel being sub-optimal and needing to be fixed.
> > The changes in these commits look like they are just trying to fix
> > what could be broken interfaces, so I thought that it would make
> > sense to see what the iwd team thinks about the situation first.
> >
> > The wireless card I was using during this testing is an Intel
> > Wireless 3165 (rev 81).  If there is any additional information I
> > could help provide, please let me know.
>
> It’d be great, if you verified, if the problem occurs with Linus’ master
> branch too.
>

It would be helpful if someone could explain for the non-mac80211
enlightened readers how iwd's EAP-PEAPv0 + MSCHAPv2 support relies on
the algif_aead socket interface, and which AEAD algorithms it uses. I
assume this is part of libell?

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Herbert Xu]

On Wed, Aug 26, 2020 at 12:40:14PM +0200, Ard Biesheuvel wrote:
>
> It would be helpful if someone could explain for the non-mac80211
> enlightened readers how iwd's EAP-PEAPv0 + MSCHAPv2 support relies on
> the algif_aead socket interface, and which AEAD algorithms it uses. I
> assume this is part of libell?

I see the problem.  libell/ell/checksum.c doesn't clear the MSG_MORE
flag before doing the recv(2).

I was hoping nobody out there was doing this but obviously I've
been proven wrong.

So what I'm going to do is to specifically allow this case of
a string of sendmsg(2)'s with MSG_MORE folloed by a recvmsg(2)
in the same thread.  I'll add a WARN_ON_ONCE so user-space can
eventually be fixed.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Ard Biesheuvel]

On Wed, 26 Aug 2020 at 13:50, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Wed, Aug 26, 2020 at 12:40:14PM +0200, Ard Biesheuvel wrote:
> >
> > It would be helpful if someone could explain for the non-mac80211
> > enlightened readers how iwd's EAP-PEAPv0 + MSCHAPv2 support relies on
> > the algif_aead socket interface, and which AEAD algorithms it uses. I
> > assume this is part of libell?
>
> I see the problem.  libell/ell/checksum.c doesn't clear the MSG_MORE
> flag before doing the recv(2).
>

But that code uses a hash not an aead, afaict.

> I was hoping nobody out there was doing this but obviously I've
> been proven wrong.
>
> So what I'm going to do is to specifically allow this case of
> a string of sendmsg(2)'s with MSG_MORE folloed by a recvmsg(2)
> in the same thread.  I'll add a WARN_ON_ONCE so user-space can
> eventually be fixed.
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Herbert Xu]

On Wed, Aug 26, 2020 at 01:59:53PM +0200, Ard Biesheuvel wrote:
> On Wed, 26 Aug 2020 at 13:50, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> >
> > On Wed, Aug 26, 2020 at 12:40:14PM +0200, Ard Biesheuvel wrote:
> > >
> > > It would be helpful if someone could explain for the non-mac80211
> > > enlightened readers how iwd's EAP-PEAPv0 + MSCHAPv2 support relies on
> > > the algif_aead socket interface, and which AEAD algorithms it uses. I
> > > assume this is part of libell?
> >
> > I see the problem.  libell/ell/checksum.c doesn't clear the MSG_MORE
> > flag before doing the recv(2).
> 
> But that code uses a hash not an aead, afaict.

Good point.  In that case can we please get a strace with a -s
option that's big enough to capture the crypto data?

Comparing the working strace and the non-working one should be
sufficient to identify the problem.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Andrew Zaborowski]

On Wed, 26 Aug 2020 at 14:10, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Wed, Aug 26, 2020 at 01:59:53PM +0200, Ard Biesheuvel wrote:
> > On Wed, 26 Aug 2020 at 13:50, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > >
> > > On Wed, Aug 26, 2020 at 12:40:14PM +0200, Ard Biesheuvel wrote:
> > > >
> > > > It would be helpful if someone could explain for the non-mac80211
> > > > enlightened readers how iwd's EAP-PEAPv0 + MSCHAPv2 support relies on
> > > > the algif_aead socket interface, and which AEAD algorithms it uses. I
> > > > assume this is part of libell?
> > >
> > > I see the problem.  libell/ell/checksum.c doesn't clear the MSG_MORE
> > > flag before doing the recv(2).
> >
> > But that code uses a hash not an aead, afaict.
>
> Good point.  In that case can we please get a strace with a -s
> option that's big enough to capture the crypto data?

Running iwd's and ell's unit tests I can see that at least the
following algorithms give EINVAL errors:
ecb(aes)
cbc(aes)
ctr(aes)

The first one fails in recv() and only for some input lengths.  The
latter two fail in send().  The relevant ell code starts at
https://git.kernel.org/pub/scm/libs/ell/ell.git/tree/ell/cipher.c#n271

The tests didn't get to the point where aead is used.

Best regards

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Herbert Xu]

On Wed, Aug 26, 2020 at 02:58:02PM +0200, Andrew Zaborowski wrote:
>
> Running iwd's and ell's unit tests I can see that at least the
> following algorithms give EINVAL errors:
> ecb(aes)
> cbc(aes)
> ctr(aes)
> 
> The first one fails in recv() and only for some input lengths.  The
> latter two fail in send().  The relevant ell code starts at
> https://git.kernel.org/pub/scm/libs/ell/ell.git/tree/ell/cipher.c#n271
> 
> The tests didn't get to the point where aead is used.

Yes ell needs to set MSG_MORE after sending the control message.
Any sendmsg(2) without a MSG_MORE will be interpreted as the end
of a request.

I'll work around this in the kernel though for the case where there
is no actual data, with a WARN_ON_ONCE.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH] crypto: af_alg - Work around empty control messages without MSG_MORE

[Herbert Xu]

The iwd daemon uses libell which sets up the skcipher operation with
two separate control messages.  This is fine by itself but the first
control message is sent without MSG_MORE.  This means that the first
control message is interpreted as an empty request.

While libell should be fixed to use MSG_MORE where appropriate, this
patch works around the bug in the kernel so that existing binaries
continue to work.

We will print a warning however.

Reported-by: Caleb Jorden <caljorden@hotmail.com>
Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when...")
Cc: <stable@vger.kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a6f581ab200c..3da21cadc326 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/net.h>
 #include <linux/rwsem.h>
+#include <linux/sched.h>
 #include <linux/sched/signal.h>
 #include <linux/security.h>
 
@@ -846,8 +847,14 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
 
 	lock_sock(sk);
 	if (ctx->init && (init || !ctx->more)) {
-		err = -EINVAL;
-		goto unlock;
+		if (ctx->used) {
+			err = -EINVAL;
+			goto unlock;
+		}
+
+		pr_info_once(
+			"%s sent an empty control message without MSG_MORE.\n",
+			current->comm);
 	}
 	ctx->init = true;
 
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: af_alg - Work around empty control messages without MSG_MORE

[Ard Biesheuvel]

On Wed, 26 Aug 2020 at 15:30, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> The iwd daemon uses libell which sets up the skcipher operation with
> two separate control messages.  This is fine by itself but the first
> control message is sent without MSG_MORE.  This means that the first
> control message is interpreted as an empty request.
>
> While libell should be fixed to use MSG_MORE where appropriate, this
> patch works around the bug in the kernel so that existing binaries
> continue to work.
>
> We will print a warning however.
>
> Reported-by: Caleb Jorden <caljorden@hotmail.com>
> Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when...")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>

Applied this onto v5.4.60, and it makes the iwd selftests pass again

Acked-by: Ard Biesheuvel <ardb@kernel.org>
Tested-by: Ard Biesheuvel <ardb@kernel.org>

> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index a6f581ab200c..3da21cadc326 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -16,6 +16,7 @@
>  #include <linux/module.h>
>  #include <linux/net.h>
>  #include <linux/rwsem.h>
> +#include <linux/sched.h>
>  #include <linux/sched/signal.h>
>  #include <linux/security.h>
>
> @@ -846,8 +847,14 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
>
>         lock_sock(sk);
>         if (ctx->init && (init || !ctx->more)) {
> -               err = -EINVAL;
> -               goto unlock;
> +               if (ctx->used) {
> +                       err = -EINVAL;
> +                       goto unlock;
> +               }
> +
> +               pr_info_once(
> +                       "%s sent an empty control message without MSG_MORE.\n",
> +                       current->comm);
>         }
>         ctx->init = true;
>
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Denis Kenzior]

Hi Herbert,

On 8/26/20 8:00 AM, Herbert Xu wrote:
> On Wed, Aug 26, 2020 at 02:58:02PM +0200, Andrew Zaborowski wrote:
>>
>> Running iwd's and ell's unit tests I can see that at least the
>> following algorithms give EINVAL errors:
>> ecb(aes)
>> cbc(aes)
>> ctr(aes)
>>
>> The first one fails in recv() and only for some input lengths.  The
>> latter two fail in send().  The relevant ell code starts at
>> https://git.kernel.org/pub/scm/libs/ell/ell.git/tree/ell/cipher.c#n271
>>
>> The tests didn't get to the point where aead is used.
> 
> Yes ell needs to set MSG_MORE after sending the control message.
> Any sendmsg(2) without a MSG_MORE will be interpreted as the end
> of a request.

I'm just waking up now, so I might seem dense, but for my education, can you 
tell me why we need to set MSG_MORE when we issue just a single sendmsg followed 
immediately by recv/recvmsg? ell/iwd operates on small buffers, so we don't 
really feed the kernel data in multiple send operations.  You can see this in 
the ell git tree link referenced in Andrew's reply.

According to https://www.kernel.org/doc/html/latest/crypto/userspace-if.html:

The send system call family allows the following flag to be specified:

     MSG_MORE: If this flag is set, the send system call acts like a cipher 
update function where more input data is expected with a subsequent invocation 
of the send system call.

So given what I said above, the documentation seems to indicate that MSG_MORE 
flag should not be used in our case?

Regards,
-Denis

> 
> I'll work around this in the kernel though for the case where there
> is no actual data, with a WARN_ON_ONCE.
> 
> Thanks,
> 

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Herbert Xu]

On Wed, Aug 26, 2020 at 08:57:17AM -0500, Denis Kenzior wrote:
> 
> I'm just waking up now, so I might seem dense, but for my education, can you
> tell me why we need to set MSG_MORE when we issue just a single sendmsg
> followed immediately by recv/recvmsg? ell/iwd operates on small buffers, so
> we don't really feed the kernel data in multiple send operations.  You can
> see this in the ell git tree link referenced in Andrew's reply.

You obviously don't need MSG_MORE if you're doing a single sendmsg.

The problematic code is in l_cipher_set_iv.  It does a sendmsg(2)
that expects to be followed by more sendmsg(2) calls before a
recvmsg(2).  That's the one that needs a MSG_MORE.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Denis Kenzior]

Hi Herbert,

On 8/26/20 9:19 AM, Herbert Xu wrote:
> On Wed, Aug 26, 2020 at 08:57:17AM -0500, Denis Kenzior wrote:
>>
>> I'm just waking up now, so I might seem dense, but for my education, can you
>> tell me why we need to set MSG_MORE when we issue just a single sendmsg
>> followed immediately by recv/recvmsg? ell/iwd operates on small buffers, so
>> we don't really feed the kernel data in multiple send operations.  You can
>> see this in the ell git tree link referenced in Andrew's reply.
> 
> You obviously don't need MSG_MORE if you're doing a single sendmsg.
> 
> The problematic code is in l_cipher_set_iv.  It does a sendmsg(2)
> that expects to be followed by more sendmsg(2) calls before a
> recvmsg(2).  That's the one that needs a MSG_MORE.
> 

Gotcha.  I fixed the set_iv part now in ell:
https://git.kernel.org/pub/scm/libs/ell/ell.git/commit/?id=87c76bbc85fe286925cbdb53d733fc9f9fd2ed12

Regards,
-Denis

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Ard Biesheuvel]

On Wed, 26 Aug 2020 at 17:33, Denis Kenzior <denkenz@gmail.com> wrote:
>
> Hi Herbert,
>
> On 8/26/20 9:19 AM, Herbert Xu wrote:
> > On Wed, Aug 26, 2020 at 08:57:17AM -0500, Denis Kenzior wrote:
> >>
> >> I'm just waking up now, so I might seem dense, but for my education, can you
> >> tell me why we need to set MSG_MORE when we issue just a single sendmsg
> >> followed immediately by recv/recvmsg? ell/iwd operates on small buffers, so
> >> we don't really feed the kernel data in multiple send operations.  You can
> >> see this in the ell git tree link referenced in Andrew's reply.
> >
> > You obviously don't need MSG_MORE if you're doing a single sendmsg.
> >
> > The problematic code is in l_cipher_set_iv.  It does a sendmsg(2)
> > that expects to be followed by more sendmsg(2) calls before a
> > recvmsg(2).  That's the one that needs a MSG_MORE.
> >
>
> Gotcha.  I fixed the set_iv part now in ell:
> https://git.kernel.org/pub/scm/libs/ell/ell.git/commit/?id=87c76bbc85fe286925cbdb53d733fc9f9fd2ed12
>

Interestingly, that change alone (without the kernel side fix that
Herbert just provided) is not sufficient to make the self tests work
again.

I still get a failure in aes_siv_encrypt(), which does not occur with
the kernel side fix applied.

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Herbert Xu]

On Wed, Aug 26, 2020 at 05:42:27PM +0200, Ard Biesheuvel wrote:
>
> I still get a failure in aes_siv_encrypt(), which does not occur with
> the kernel side fix applied.

Where is this test from? I can't find it in the ell git tree.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Ard Biesheuvel]

On Thu, 27 Aug 2020 at 00:19, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Wed, Aug 26, 2020 at 05:42:27PM +0200, Ard Biesheuvel wrote:
> >
> > I still get a failure in aes_siv_encrypt(), which does not occur with
> > the kernel side fix applied.
>
> Where is this test from? I can't find it in the ell git tree.
>

It is part of iwd - just build that and run 'make check'

With your patch applied, the occurrence of sendmsg() in
operate_cipher() triggers the warn_once(), but if I add MSG_MORE
there, the test hangs.

[v2 PATCH] crypto: af_alg - Work around empty control messages without MSG_MORE

[Herbert Xu]

On Thu, Aug 27, 2020 at 08:40:01AM +0200, Ard Biesheuvel wrote:
>
> It is part of iwd - just build that and run 'make check'
> 
> With your patch applied, the occurrence of sendmsg() in
> operate_cipher() triggers the warn_once(), but if I add MSG_MORE
> there, the test hangs.

I see.  This is a different issue.  The original kernel change
was a bit too strict here and it is barfing at the fact that two
successive sendmsg's of the same request both contain a control
message.

Here's an updated patch to allow this.

---8<---
The iwd daemon uses libell which sets up the skcipher operation with
two separate control messages.  As the first control message is sent
without MSG_MORE, it is interpreted as an empty request.

While libell should be fixed to use MSG_MORE where appropriate, this
patch works around the bug in the kernel so that existing binaries
continue to work.

We will print a warning however.

A separate issue is that the new kernel code no longer allows the
control message to be sent twice within the same request.  This
restriction is obviously incompatible with what iwd was doing (first
setting an IV and then sending the real control message).  This
patch changes the kernel so that this is explicitly allowed.

Reported-by: Caleb Jorden <caljorden@hotmail.com>
Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when...")
Cc: <stable@vger.kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a6f581ab200c..8be8bec07cdd 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/net.h>
 #include <linux/rwsem.h>
+#include <linux/sched.h>
 #include <linux/sched/signal.h>
 #include <linux/security.h>
 
@@ -845,9 +846,15 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	}
 
 	lock_sock(sk);
-	if (ctx->init && (init || !ctx->more)) {
-		err = -EINVAL;
-		goto unlock;
+	if (ctx->init && !ctx->more) {
+		if (ctx->used) {
+			err = -EINVAL;
+			goto unlock;
+		}
+
+		pr_info_once(
+			"%s sent an empty control message without MSG_MORE.\n",
+			current->comm);
 	}
 	ctx->init = true;
 
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [v2 PATCH] crypto: af_alg - Work around empty control messages without MSG_MORE

[Ard Biesheuvel]

On Thu, 27 Aug 2020 at 09:15, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Thu, Aug 27, 2020 at 08:40:01AM +0200, Ard Biesheuvel wrote:
> >
> > It is part of iwd - just build that and run 'make check'
> >
> > With your patch applied, the occurrence of sendmsg() in
> > operate_cipher() triggers the warn_once(), but if I add MSG_MORE
> > there, the test hangs.
>
> I see.  This is a different issue.  The original kernel change
> was a bit too strict here and it is barfing at the fact that two
> successive sendmsg's of the same request both contain a control
> message.
>
> Here's an updated patch to allow this.
>
> ---8<---
> The iwd daemon uses libell which sets up the skcipher operation with
> two separate control messages.  As the first control message is sent
> without MSG_MORE, it is interpreted as an empty request.
>
> While libell should be fixed to use MSG_MORE where appropriate, this
> patch works around the bug in the kernel so that existing binaries
> continue to work.
>
> We will print a warning however.
>
> A separate issue is that the new kernel code no longer allows the
> control message to be sent twice within the same request.  This
> restriction is obviously incompatible with what iwd was doing (first
> setting an IV and then sending the real control message).  This
> patch changes the kernel so that this is explicitly allowed.
>
> Reported-by: Caleb Jorden <caljorden@hotmail.com>
> Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when...")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index a6f581ab200c..8be8bec07cdd 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -16,6 +16,7 @@
>  #include <linux/module.h>
>  #include <linux/net.h>
>  #include <linux/rwsem.h>
> +#include <linux/sched.h>
>  #include <linux/sched/signal.h>
>  #include <linux/security.h>
>
> @@ -845,9 +846,15 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
>         }
>
>         lock_sock(sk);
> -       if (ctx->init && (init || !ctx->more)) {
> -               err = -EINVAL;
> -               goto unlock;
> +       if (ctx->init && !ctx->more) {
> +               if (ctx->used) {
> +                       err = -EINVAL;
> +                       goto unlock;
> +               }
> +
> +               pr_info_once(
> +                       "%s sent an empty control message without MSG_MORE.\n",
> +                       current->comm);
>         }
>         ctx->init = true;
>

Yep, that works.

Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise

[Ard Biesheuvel]

On Thu, 27 Aug 2020 at 06:56, Caleb Jorden <caljorden@hotmail.com> wrote:
>
> I can tell you all assumed this, but just by way as a quick update on the original issue:
>
> I have confirmed that Herbert's patch (crypto: af_alg - Work around empty control messages without MSG_MORE) does indeed fix the original iwd 1.8 + WPA Enterprise issue.
>
> Thank you!
>
> Caleb Jorden
>

Thanks for confirming.

> ________________________________________
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Sent: Thursday, August 27, 2020 3:49 AM
> To: Ard Biesheuvel
> Cc: Denis Kenzior; Andrew Zaborowski; Paul Menzel; Caleb Jorden; Sasha Levin; iwd@lists.01.org; # 3.4.x; Greg KH; LKML; David S. Miller; Linux Crypto Mailing List
> Subject: Re: Issue with iwd + Linux 5.8.3 + WPA Enterprise
>
> On Wed, Aug 26, 2020 at 05:42:27PM +0200, Ard Biesheuvel wrote:
> >
> > I still get a failure in aes_siv_encrypt(), which does not occur with
> > the kernel side fix applied.
>
> Where is this test from? I can't find it in the ell git tree.
>
> Thanks,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt