mbox series

[00/11] Basic SAE support for AP mode

Message ID 20240421125050.6649-1-brandtwjohn@gmail.com (mailing list archive)
Headers show
Series Basic SAE support for AP mode | expand

Message

John Brandt April 21, 2024, 12:50 p.m. UTC
This set of patches adds basic SAE support for IWD in AP mode. It has
been tested by connecting to IWD AP using wpa_supplicant. Note that this
does not yet correspond to WPA3, since WPA3 would also require the
support of Management Frame Protection.

Normal client functionality has also been confirmed to still work. After
applying these patches it remains possible for IWD client to connect to
WPA3/SAE network.

Remaining TODOs are to include better sanity-checking of received
frames.

John Brandt (11):
  ap: ability to advertise PSK and SAE
  ap: accept PSK/SAE in auth depending on config
  sae: add function sae_set_group
  sae: refactor and add function sae_calculate_keys
  sae: make sae_process_commit callable in AP mode
  sae: verify offered group in AP mode
  sae: support reception of Confirm frame by AP
  ap: add support to handle SAE authentication
  ap: enable start of 4-way HS after SAE
  eapol: support PTK derivation with SHA256
  eapol: encrypt key data for AKM-defined ciphers

 src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
 src/eapol.c |  58 ++++++++++++-----
 src/sae.c   | 175 +++++++++++++++++++++++++++++++++-------------------
 3 files changed, 265 insertions(+), 103 deletions(-)

Comments

James Prestwood April 22, 2024, 1:52 p.m. UTC | #1
Hi John,

On 4/21/24 5:50 AM, John Brandt wrote:
> This set of patches adds basic SAE support for IWD in AP mode. It has
> been tested by connecting to IWD AP using wpa_supplicant. Note that this
> does not yet correspond to WPA3, since WPA3 would also require the
> support of Management Frame Protection.
>
> Normal client functionality has also been confirmed to still work. After
> applying these patches it remains possible for IWD client to connect to
> WPA3/SAE network.
>
> Remaining TODOs are to include better sanity-checking of received
> frames.

I took a quick pass and I'm impressed you took the initiative to 
implement this. I do need to take a closer look from the spec side of 
things but overall it looks good.

Assuming we are compliant with the spec my only concern merging this 
would be the TODOs. You do mention this is experimental and certain 
checks are not done, but you'd be surprised at the number of people 
using IWD in AP mode. The minute we merge this we're going to have 
people using it, which in its current form would be insecure. I think we 
first need to get the frame verification implemented as well as MFP. But 
anyways, this is a good start and I'll give it a full review when I have 
some time, hopefully this week.

Thanks,

James

>
> John Brandt (11):
>    ap: ability to advertise PSK and SAE
>    ap: accept PSK/SAE in auth depending on config
>    sae: add function sae_set_group
>    sae: refactor and add function sae_calculate_keys
>    sae: make sae_process_commit callable in AP mode
>    sae: verify offered group in AP mode
>    sae: support reception of Confirm frame by AP
>    ap: add support to handle SAE authentication
>    ap: enable start of 4-way HS after SAE
>    eapol: support PTK derivation with SHA256
>    eapol: encrypt key data for AKM-defined ciphers
>
>   src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
>   src/eapol.c |  58 ++++++++++++-----
>   src/sae.c   | 175 +++++++++++++++++++++++++++++++++-------------------
>   3 files changed, 265 insertions(+), 103 deletions(-)
>
James Prestwood April 24, 2024, 12:07 p.m. UTC | #2
On 4/22/24 6:52 AM, James Prestwood wrote:
> Hi John,
>
> On 4/21/24 5:50 AM, John Brandt wrote:
>> This set of patches adds basic SAE support for IWD in AP mode. It has
>> been tested by connecting to IWD AP using wpa_supplicant. Note that this
>> does not yet correspond to WPA3, since WPA3 would also require the
>> support of Management Frame Protection.
>>
>> Normal client functionality has also been confirmed to still work. After
>> applying these patches it remains possible for IWD client to connect to
>> WPA3/SAE network.
>>
>> Remaining TODOs are to include better sanity-checking of received
>> frames.
>
> I took a quick pass and I'm impressed you took the initiative to 
> implement this. I do need to take a closer look from the spec side of 
> things but overall it looks good.
>
> Assuming we are compliant with the spec my only concern merging this 
> would be the TODOs. You do mention this is experimental and certain 
> checks are not done, but you'd be surprised at the number of people 
> using IWD in AP mode. The minute we merge this we're going to have 
> people using it, which in its current form would be insecure. I think 
> we first need to get the frame verification implemented as well as 
> MFP. But anyways, this is a good start and I'll give it a full review 
> when I have some time, hopefully this week.
>
> Thanks,
>
> James

Reviewed. I also forgot to mention the CI we have showed test-sae failed 
after your patches, so that will need to be addressed as well. I keep 
getting reminded I need to also email the patch submitter.

Thanks,

James

>
>>
>> John Brandt (11):
>>    ap: ability to advertise PSK and SAE
>>    ap: accept PSK/SAE in auth depending on config
>>    sae: add function sae_set_group
>>    sae: refactor and add function sae_calculate_keys
>>    sae: make sae_process_commit callable in AP mode
>>    sae: verify offered group in AP mode
>>    sae: support reception of Confirm frame by AP
>>    ap: add support to handle SAE authentication
>>    ap: enable start of 4-way HS after SAE
>>    eapol: support PTK derivation with SHA256
>>    eapol: encrypt key data for AKM-defined ciphers
>>
>>   src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
>>   src/eapol.c |  58 ++++++++++++-----
>>   src/sae.c   | 175 +++++++++++++++++++++++++++++++++-------------------
>>   3 files changed, 265 insertions(+), 103 deletions(-)
>>
John Brandt April 29, 2024, 12:04 a.m. UTC | #3
On 4/24/24 05:07, James Prestwood wrote:
> 
> On 4/22/24 6:52 AM, James Prestwood wrote:
>> Hi John,
>>
>> On 4/21/24 5:50 AM, John Brandt wrote:
>>> This set of patches adds basic SAE support for IWD in AP mode. It has
>>> been tested by connecting to IWD AP using wpa_supplicant. Note that this
>>> does not yet correspond to WPA3, since WPA3 would also require the
>>> support of Management Frame Protection.
>>>
>>> Normal client functionality has also been confirmed to still work. After
>>> applying these patches it remains possible for IWD client to connect to
>>> WPA3/SAE network.
>>>
>>> Remaining TODOs are to include better sanity-checking of received
>>> frames.
>>
>> I took a quick pass and I'm impressed you took the initiative to 
>> implement this. I do need to take a closer look from the spec side of 
>> things but overall it looks good.
>>
>> Assuming we are compliant with the spec my only concern merging this 
>> would be the TODOs. You do mention this is experimental and certain 
>> checks are not done, but you'd be surprised at the number of people 
>> using IWD in AP mode. The minute we merge this we're going to have 
>> people using it, which in its current form would be insecure. I think 
>> we first need to get the frame verification implemented as well as 
>> MFP. But anyways, this is a good start and I'll give it a full review 
>> when I have some time, hopefully this week.
>>
>> Thanks,
>>
>> James
> 
> Reviewed. I also forgot to mention the CI we have showed test-sae failed 
> after your patches, so that will need to be addressed as well. I keep 
> getting reminded I need to also email the patch submitter.
> 
> Thanks,
> 
> James

Thanks, I've mostly incorporated the feedback, and also added extra 
frame checks. I can post the v2 version later this week.

I'm unsure whether I'll also have the time to experiment with MFP. Maybe 
it's possible to already add SAE support without yet mentioning WPA3 for 
the AP mode?

>>
>>>
>>> John Brandt (11):
>>>    ap: ability to advertise PSK and SAE
>>>    ap: accept PSK/SAE in auth depending on config
>>>    sae: add function sae_set_group
>>>    sae: refactor and add function sae_calculate_keys
>>>    sae: make sae_process_commit callable in AP mode
>>>    sae: verify offered group in AP mode
>>>    sae: support reception of Confirm frame by AP
>>>    ap: add support to handle SAE authentication
>>>    ap: enable start of 4-way HS after SAE
>>>    eapol: support PTK derivation with SHA256
>>>    eapol: encrypt key data for AKM-defined ciphers
>>>
>>>   src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
>>>   src/eapol.c |  58 ++++++++++++-----
>>>   src/sae.c   | 175 +++++++++++++++++++++++++++++++++-------------------
>>>   3 files changed, 265 insertions(+), 103 deletions(-)
>>>
James Prestwood April 29, 2024, noon UTC | #4
Hi John,

On 4/28/24 5:04 PM, John Brandt wrote:
>
>
> On 4/24/24 05:07, James Prestwood wrote:
>>
>> On 4/22/24 6:52 AM, James Prestwood wrote:
>>> Hi John,
>>>
>>> On 4/21/24 5:50 AM, John Brandt wrote:
>>>> This set of patches adds basic SAE support for IWD in AP mode. It has
>>>> been tested by connecting to IWD AP using wpa_supplicant. Note that 
>>>> this
>>>> does not yet correspond to WPA3, since WPA3 would also require the
>>>> support of Management Frame Protection.
>>>>
>>>> Normal client functionality has also been confirmed to still work. 
>>>> After
>>>> applying these patches it remains possible for IWD client to 
>>>> connect to
>>>> WPA3/SAE network.
>>>>
>>>> Remaining TODOs are to include better sanity-checking of received
>>>> frames.
>>>
>>> I took a quick pass and I'm impressed you took the initiative to 
>>> implement this. I do need to take a closer look from the spec side 
>>> of things but overall it looks good.
>>>
>>> Assuming we are compliant with the spec my only concern merging this 
>>> would be the TODOs. You do mention this is experimental and certain 
>>> checks are not done, but you'd be surprised at the number of people 
>>> using IWD in AP mode. The minute we merge this we're going to have 
>>> people using it, which in its current form would be insecure. I 
>>> think we first need to get the frame verification implemented as 
>>> well as MFP. But anyways, this is a good start and I'll give it a 
>>> full review when I have some time, hopefully this week.
>>>
>>> Thanks,
>>>
>>> James
>>
>> Reviewed. I also forgot to mention the CI we have showed test-sae 
>> failed after your patches, so that will need to be addressed as well. 
>> I keep getting reminded I need to also email the patch submitter.
>>
>> Thanks,
>>
>> James
>
> Thanks, I've mostly incorporated the feedback, and also added extra 
> frame checks. I can post the v2 version later this week.
>
> I'm unsure whether I'll also have the time to experiment with MFP. 
> Maybe it's possible to already add SAE support without yet mentioning 
> WPA3 for the AP mode?

MFP may be sort of automatic, it is for station mode. All IWD does is 
some minimal validation since it has an explicit profile setting to 
enable/disable/require MFP. It also checks the cipher support related to 
MFP. But I think for AP mode all you'd need to do is ensure that 
connecting stations are capable of MFP when connecting via WPA3. And set 
the proper bits in the RSNE.

Thanks,

James

>
>>>
>>>>
>>>> John Brandt (11):
>>>>    ap: ability to advertise PSK and SAE
>>>>    ap: accept PSK/SAE in auth depending on config
>>>>    sae: add function sae_set_group
>>>>    sae: refactor and add function sae_calculate_keys
>>>>    sae: make sae_process_commit callable in AP mode
>>>>    sae: verify offered group in AP mode
>>>>    sae: support reception of Confirm frame by AP
>>>>    ap: add support to handle SAE authentication
>>>>    ap: enable start of 4-way HS after SAE
>>>>    eapol: support PTK derivation with SHA256
>>>>    eapol: encrypt key data for AKM-defined ciphers
>>>>
>>>>   src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
>>>>   src/eapol.c |  58 ++++++++++++-----
>>>>   src/sae.c   | 175 
>>>> +++++++++++++++++++++++++++++++++-------------------
>>>>   3 files changed, 265 insertions(+), 103 deletions(-)
>>>>
KeithG April 30, 2024, 11:27 p.m. UTC | #5
James,

When this is ready to go, I can test on my Pis...

Just need to know how to tell AP mode how to use wpa3/sae

Keith

Keith

On Mon, Apr 29, 2024 at 7:04 AM James Prestwood <prestwoj@gmail.com> wrote:
>
> Hi John,
>
> On 4/28/24 5:04 PM, John Brandt wrote:
> >
> >
> > On 4/24/24 05:07, James Prestwood wrote:
> >>
> >> On 4/22/24 6:52 AM, James Prestwood wrote:
> >>> Hi John,
> >>>
> >>> On 4/21/24 5:50 AM, John Brandt wrote:
> >>>> This set of patches adds basic SAE support for IWD in AP mode. It has
> >>>> been tested by connecting to IWD AP using wpa_supplicant. Note that
> >>>> this
> >>>> does not yet correspond to WPA3, since WPA3 would also require the
> >>>> support of Management Frame Protection.
> >>>>
> >>>> Normal client functionality has also been confirmed to still work.
> >>>> After
> >>>> applying these patches it remains possible for IWD client to
> >>>> connect to
> >>>> WPA3/SAE network.
> >>>>
> >>>> Remaining TODOs are to include better sanity-checking of received
> >>>> frames.
> >>>
> >>> I took a quick pass and I'm impressed you took the initiative to
> >>> implement this. I do need to take a closer look from the spec side
> >>> of things but overall it looks good.
> >>>
> >>> Assuming we are compliant with the spec my only concern merging this
> >>> would be the TODOs. You do mention this is experimental and certain
> >>> checks are not done, but you'd be surprised at the number of people
> >>> using IWD in AP mode. The minute we merge this we're going to have
> >>> people using it, which in its current form would be insecure. I
> >>> think we first need to get the frame verification implemented as
> >>> well as MFP. But anyways, this is a good start and I'll give it a
> >>> full review when I have some time, hopefully this week.
> >>>
> >>> Thanks,
> >>>
> >>> James
> >>
> >> Reviewed. I also forgot to mention the CI we have showed test-sae
> >> failed after your patches, so that will need to be addressed as well.
> >> I keep getting reminded I need to also email the patch submitter.
> >>
> >> Thanks,
> >>
> >> James
> >
> > Thanks, I've mostly incorporated the feedback, and also added extra
> > frame checks. I can post the v2 version later this week.
> >
> > I'm unsure whether I'll also have the time to experiment with MFP.
> > Maybe it's possible to already add SAE support without yet mentioning
> > WPA3 for the AP mode?
>
> MFP may be sort of automatic, it is for station mode. All IWD does is
> some minimal validation since it has an explicit profile setting to
> enable/disable/require MFP. It also checks the cipher support related to
> MFP. But I think for AP mode all you'd need to do is ensure that
> connecting stations are capable of MFP when connecting via WPA3. And set
> the proper bits in the RSNE.
>
> Thanks,
>
> James
>
> >
> >>>
> >>>>
> >>>> John Brandt (11):
> >>>>    ap: ability to advertise PSK and SAE
> >>>>    ap: accept PSK/SAE in auth depending on config
> >>>>    sae: add function sae_set_group
> >>>>    sae: refactor and add function sae_calculate_keys
> >>>>    sae: make sae_process_commit callable in AP mode
> >>>>    sae: verify offered group in AP mode
> >>>>    sae: support reception of Confirm frame by AP
> >>>>    ap: add support to handle SAE authentication
> >>>>    ap: enable start of 4-way HS after SAE
> >>>>    eapol: support PTK derivation with SHA256
> >>>>    eapol: encrypt key data for AKM-defined ciphers
> >>>>
> >>>>   src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
> >>>>   src/eapol.c |  58 ++++++++++++-----
> >>>>   src/sae.c   | 175
> >>>> +++++++++++++++++++++++++++++++++-------------------
> >>>>   3 files changed, 265 insertions(+), 103 deletions(-)
> >>>>
>