spring-attic / spring-security-saml

SAML extension for the Spring Security project
Other
419 stars 484 forks source link

wrongly setting empty to a null Set<String> #510

Open TohidHeshmati opened 3 years ago

TohidHeshmati commented 3 years ago

Inside org/springframework/security/saml/trust/X509TrustManager.java line 91 we have an if statement with condition: trustEngine.validate(credential, criteriaSet). this condition is false so the else is executed and the UntrustedCertificateException(sb.toString(), x509Certificates); is thrown. the validate method in our trust engine org/opensaml/xml/security/x509/PKIXX509CredentialTrustEngine.java returns false. Why? it sets trustedNames to null and then checks another if statement which is always true and goes inside if statement executing resolveTrustedNames method. Inside this method creates an empty set with trustBasisCriteria which is empty and adds other which are nothing and returns empty set. so our trustedNames turns to empty instead of null. in return value the validate method gets trusted names as an argument (which now is empty instead of null)

Set<String> trustedNames = null;  
if (pkixResolver.supportsTrustedNameResolution()) {  
  trustedNames = pkixResolver.resolveTrustedNames(trustBasisCriteria);  
} else {  
  log.debug("PKIX resolver does not support resolution of trusted names, skipping name checking");  
}  

return validate(untrustedCredential, trustedNames, pkixResolver.resolve(trustBasisCriteria));

now inside this method there is another if statement:

if (!checkNames(trustedNames, untrustedX509Credential)) {  
  return false;  
}

this tries to check names from trustedNames which now is empty instead of null but the logic inside checkNames is:

if (trustedNames == null) {  
    return true;
  }

but it is not null, it is empty. so instead of true returns false so validation returns false and so on.

jzheaux commented 3 years ago

Appears related to https://github.com/spring-projects/spring-security-saml/issues/233#issuecomment-638565973