Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using subject_match is a bad idea #22

Open
nmeum opened this issue Oct 11, 2018 · 8 comments
Open

Using subject_match is a bad idea #22

nmeum opened this issue Oct 11, 2018 · 8 comments
Assignees

Comments

@nmeum
Copy link

nmeum commented Oct 11, 2018

The wpa_supplicant configuration generated by this app makes use of the subject_match configuration option for verifying the authentication server certificate.

The description of the subject_match option in the default wpa_supplicant configuration file is as follows:

# subject_match: Substring to be matched against the subject of the
#	authentication server certificate. If this string is set, the server
#	certificate is only accepted if it contains this string in the subject.
#	The subject string is in following format:
#	/C=US/ST=CA/L=San Francisco/CN=Test AS/emailAddress=as@example.com
#	Note: Since this is a substring match, this cannot be used securely to
#	do a suffix match against a possible domain name in the CN entry. For
#	such a use case, domain_suffix_match or domain_match should be used
#	instead.

Thus, if subject_match is set, for example, to radius.wlan.uni-foobar.de it also matches server certificates with the subject radius.wlan.uni-foo.de.pwned.de. This is especially critical since this app (at least in the configuration for my university) seems to pin against a TeleSec GlobalRoot CA. This might allow an attacker to register the domain radius.wlan.uni-foo.de.pwned.de, retrieve a certificate signed by the TeleSec GlobalRoot CA for it and then use that certificate with a rouge access point to sniff eduroam passwords of clients which used this app to generate their config.

Vanilla android uses domain_suffix_match instead. Which isn't perfect either but still better than subject_match.

@restena-sw
Copy link
Contributor

You are referring to WifiConfigAPI18.java line 204:

enterpriseConfig.setSubjectMatch(subjectMatch);

The name of the file kind of gives it away: we are maintaining compatibility with API 18 (Android 4.3).

WifiEnterpriseConfig got the new method setDomainSuffixMatch only in API level 23.

You are of course right in your analysis that domainSuffixMatch is the better option. But on API 18 it's not a bad idea at all; it's the only possibility. We can't thus unconditionally change that as it breaks compatibiltiy with some target platforms.

I would rather turn this around as:

Instead of unconditionally using setSubjectMatch, test for the actual API level on the device first, and use setDomainSuffixMatch if the API level is 23 or higher. I believe this is something we can easily implement. But I'm not the developer and might be wrong - I'm assigning this issue to Gareth now.

@egroeper
Copy link

It's indeed easy. But we switched to AltSubjectMatch, which is the direct successor:
hu-berlin-cms/hu-eduroam@309bf30

@egroeper
Copy link

It's indeed easy.

Sorry. I'm easily forgetting things...
In case of AltSubjectMatch, it's a tiny bit harder: hu-berlin-cms/hu-eduroam@6765420

For the sake of completeness.

@martinpauly
Copy link

nmeum, thank you for the precise analysis.

This is especially critical since this app (at least in the configuration for my university) seems to
pin against a TeleSec GlobalRoot CA.
All German Universities I know of do that. But the issue would occur with any other CA in exactly
the same way, wouldn't it?

@nmeum
Copy link
Author

nmeum commented Nov 5, 2018

But the issue would occur with any other CA in exactly the same way, wouldn't it?

Yes, the issue being that it pins against the Root CA, not against a specific certificate signed by that CA though I am aware that this is not feasible in some situations.

@restena-sw
Copy link
Contributor

The pinning of the CA is not an "issue"; it is the way the trust fabric is designed to work.

If you pin an individual certificate, you lose all trust everytime you change the /certificate/. For a public CA, this typically means every two years. You will have to train your users to "Click Accept" whenever that cert changes. Which means: if someone sets up a rogue server with the same name, the same Click Accept reflex leads user to connect to the rogue server, which they shouldn't.

By pinning the CA, the trust anchor remains identical and no new security prompts come up so long as your exchange of the server certificate stays with the same CA.

The only issue is that there are multiple variants of name checks on the received server certificate; some of which do a better job at comparing the incoming name string against the expected one than others.

BTW, it is possible to make a deployment agnostic of the name check variant, by deploying a private root CAT exclusively for the RADIUS server. That way, the root is pinned as trusted and can be very long-lived; while any name checks on the server certificate itself are superfluous because the CA only issues one certificate for this exact purpose. This is however a deployment choice of the Identity Provider; nothing an end user can do about this.

For a more thorough read on the topic, I suggest this page: https://wiki.geant.org/display/H2eduroam/EAP+Server+Certificate+considerations

@nmeum
Copy link
Author

nmeum commented Nov 6, 2018

The pinning of the CA is not an "issue";

It's an issue in combination with the fact that subject_match is being used. subject_match doesn't properly verify the certificate (as explained above) thus wpa_supplicant can be tricked into excepting other certificates signed by the Root CA. My point is that this would not have been possible if you pinned directly against a specific certificate signed by the Root CA.

Pinning against a specific certificate signed by that CA has some disadvantages which @restena-sw described. Thus the proper way to fix this issues is using altsubject_match instead.

@restena-sw
Copy link
Contributor

I think we all agree that altsubject_match is the way to go :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants