Make certauth eku module restrictive-only #694
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
greghudson
force-pushed
the
pkinit-eku
branch
3 times, most recently
from
7fd1190
to
b7af544
Compare
|
I combined in a fix for the san module, which was explicitly rejecting certs too aggressively, and added a test case using a no-extensions client cert. (A generic client cert doesn't actually trigger the san module bug because it has no SANs at all, but it is one of the primary use cases for the dbmatch module, so it seemed like a problem that we didn't have a test case for it.) |
|
CVE-2017-7562 was issued for this bug. |
frozencemetery
approved these changes
Aug 29, 2017
The PKINIT certauth eku module should never authoritatively authorize a certificate, because an extended key usage does not establish a relationship between the certificate and any specific user; it only establishes that the certificate was created for PKINIT client authentication. Therefore, pkinit_eku_authorize() should return KRB5_PLUGIN_NO_HANDLE on success, not 0. The certauth san module should pass if it does not find any SANs of the types it can match against; the presence of other types of SANs should not cause it to explicitly deny a certificate. Check for an empty result from crypto_retrieve_cert_sans() in verify_client_san(), instead of returning ENOENT from crypto_retrieve_cert_sans() when there are no SANs at all. ticket: 8561
Add commands to make-certs.sh to generate a test client certificate with no certificate extensions. Re-run make-certs.sh. ticket: 8562
In t_pkinit.py, add a test case where a client cert with no extensions is authorized via subject and issuer using a pkinit_cert_match string attribute. ticket: 8562
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[I noticed this when looking into feedback on the certauth interface. The impact isn't as bad as I initially thought, because a PKINIT client cert with the EKU will typically also contain a PKINIT SAN, and the san module will reject the cert if PKINIT SANs are present and don't match. But it would still have been a CVE bug, so it's fortunate that I ran into this before the release.]
The PKINIT certauth eku module should never authoritatively authorize
a certificate, because an extended key usage does not establish a
relationship between the certificate and any specific user; it only
establishes that the certificate was created for PKINIT client
authentication. Therefore, pkinit_eku_authorize() should return
KRB5_PLUGIN_NO_HANDLE on success, not 0.