geteduroam / ionic-app

iOS and Android app for geteduroam
BSD 3-Clause "New" or "Revised" License
16 stars 12 forks source link

Support inner auth method other than MSCHAPv2 #72

Closed jornane closed 3 years ago

jornane commented 3 years ago

Currently, only MSCHAPv2 is configured. The eap-config specifies which inner method should be configured, and although MSCHAPv2 is very common, there are other options that might need supporting.

The supported inner methods are documented here https://github.com/GEANT/CAT/blob/master/devices/eap_config/eap-metadata.xsd#L24-L42

I noticed that we also have a method/number mapping in the following files, I think we should make that in sync with the XSD file. I noticed the mapping in the following files

If the outer method is TTLS, either <NonEAPAuthMethod> or <EAPMethod> must be set in the file (files that have TTLS but neither <NonEAPAuthMethod> or <EAPMethod> are invalid). I have not checked Android, but on iOS all <NonEAPAuthMethod>s are supported, but only MSCHAPv2 is supported for EAPMethod, in which case you set NEHotspotEAPSettings.TTLSInnerAuthenticationType.eapttlsInnerAuthenticationEAP

jornane commented 3 years ago

So we must support the following methods:

NOTE ~TTLS-EAP-MSCHAPv2~ can be interpreted as TTLS-MSCHAPv2 as long as it's documented in the code.

NOTE For PEAP-MSCHAPv2, the INNER method may be hardcoded and assumed, as long as it's documented in the code.

NOTE For TLS, the INNER must be ignored, but it's okay to still set it as auth, even if it's an invalid value (we've seen the value 999 in the wild)

The app must thus parse the eap-config file, and get the correct values from it. It MUST NOT make assumptions for missing values, so no defaults.

jornane commented 3 years ago

In more implementation specific terms terms, consider the function private configConnection() { which returns a dictionary with configuration settings.

In this dictionary:

jornane commented 3 years ago

For the purpose of this change, setting the dictionary to the correct value is enough. In other words: No changes to the Swift or Java code are needed. We will update those to reflect the changes.

amachadogarcia commented 3 years ago

Fixed in commit f92a648d796b8670c5a9c2524dd3d6c6a1f206aa

amachadogarcia commented 3 years ago

Last fix in commit 579a841a313865f36ae7d4d49136965d174d660d