Need suggestion/help in back porting the fix for vulnerability CVE-2017-7562 (backporting from Kerberos 1.16.1 to Kerberos 1.9)

[Greg Hudson]

On 09/23/2018 03:40 AM, Shivakumar Nadarajan -X (shinadar - HCL 
TECHNOLOGIES LIMITED at Cisco) wrote:
> Before commit(b619ce84470519bea65470be3263cd85fba94f57):
> 	1. It’s a sequential checks that are done against the certificate, meaning that first san check is done using verify_client_san()
> 	2. Then eku check is done using verify_client_eku()

The vulnerability was that a client certificate with an Extended Key 
Usage attribute indicating that it was issued for PKINIT, but no Subject 
Alternative Name attributes at all, would be allowed to authorize any 
client principal, even though there is nothing in the certificate naming 
the client.

The old code does not contain this vulnerability because 
pkinit_server_verify_padata() returns an error if either 
verify_client_san() returns with *valid_san == 0 or if 
verify_client_eku() returns with *valid_eku == 0.  It does not allow a 
valid EKU to override the lack of a valid SAN.

There was a second bug concomittant with the bug creating the 
vulnerability: a certificate with SAN attributes that have nothing to do 
with PKINIT would be rejected even if another module (like the new 
dbmatch module) would have authorized it.  This second bug does not 
create a vulnerability; in fact, it created an over-restrictiveness 
which reduced the impact of the vulnerability.

> After commit:
> 	1. The code has been refactored such that san and eku check are done through function pointers pkinit_san_authorize() and pkinit_eku_authorize()
> 	2. Both these function pointers still call verify_client_san() and verify_client_eku() and do the same checks as before commit.
> 	3. Additional check related to db match is been added after the commit
> 	
> Common note: Either before the commit or after the commit, the retrieve san information is done in crypto_retrieve_X509_sans() which hasn't changed before or after the commit.

Actually, crypto_retrieve_X509_sans() did change in this commit, to 
return ENOENT if there are no SAN fields.

> And the main vulnerability fix is done in crypto_retrieve_X509_sans() and hence its applicable for all the versions which has this code.

The vulnerability fix is in pkinit_eku_authorize(), where it returns 
KRB5_PLUGIN_NO_HANDLE (meaning "I don't see any reason to reject the 
cert, nor any reason to accept it") instead of returning 0 (meaning "I 
see a reason to accept the cert").  Everything else in the same commit 
fixes the second bug.

To fix the second bug, crypto_retrieve_X509_sans() needed to change to 
make it easier to determine when a certificate contains no 
PKINIT-related SAN attributes (as opposed to no SAN attributes at all). 
So the change in commit b619ce84470519bea65470be3263cd85fba94f57 was 
reverted (it no longer returns ENOENT if there are no SAN attributes) 
and it is instead changed to return null lists instead of empty lists. 
verify_client_san() is then changed to look for both lists of 
PKINIT-related SAN attributes being null, instead of looking for an 
ENOENT result.

For the pre-certauth code, the old behavior of 
crypto_retrieve_X509_sans() was sufficient because there was no way to 
authorize a client certificate except via a PKINIT-related SAN attribute 
matching the client principal.  Therefore there was no need to determine 
whether any PKINIT-related SAN attributes were present.

> Please correct me if I am wrong and also request you to help me further on this.
> 
> Also please refer the below link specific to Cisco
> 
> https://tools.cisco.com/security/center/viewAlert.x?alertId=58543

That page is wrong.  No upstream versions of MIT krb5 are affected by 
the vulnerability, and 1.16.1 does not contain a fix for the 
vulnerability (because 1.16 already had the fix).