Is :idp_cert_fingerprint_validator required?

[pitbulk]

I checked omniauth-saml's settings/code and I don't understand the use of

 :idp_cert_fingerprint_validator     => lambda { |fingerprint| fingerprint },

At the ruby toolkit, in order to check embedded Signatures (of the HTTP-POST binding), when you add a :idp_cert_fingerprint instead the :idp_cert, doesn't matter what you use, at the end the idp_cert is turned in a idp_cert_fingerprint to validate the document.

The certificate of the SAMLResponse is fingerprinted and compared with the value of the idp_cert_fingerprint.

I think this is already done at omniauth here

P.S I always recommend to set the idp_cert and not the idp_cert_fingerprint because HTTP-Redirect binding signature validations requires it (since the IdP's public certificate is not at the SAML Message).
As you plan to add SLO soon, recommend the use of certificates vs fingerprints.
Related topic: certFingerprint versus certificate/certData - simpleSAMLphp

[md5]

idp_cert_fingerprint_validator is not required. It was added in #31, so you can read more about the use case there.

[pitbulk]

Ok, is an optional parameter, but why exist? since you can already provide dynamically a value for idp_cert_fingerprint, or do you want to support multiple idp_cert_fingerprint at once?

[md5]

@pitbulk I'm not sure beyond what's described in #31. Perhaps it wasn't possible to dynamically provide the value of idp_cert_fingerprint when that PR was merged in late 2014?

Perhaps @bpedro can provide some context, but he hasn't been active on this project for a while.

[pitbulk]

ruby-saml requires to be set a certificate or a fingerprint (only 1 value) and it will validate it againstthe cert on the SAMLResponse, so if a wrong value is set, the SAMLResponse will be consider invalid, so the extra check of the idp_cert_fingerprint_validator I think is useless since already it was invalidated.

[md5]

@pitbulk The code that deals with this "validator" is at

omniauth-saml/lib/omniauth/strategies/saml.rb

Lines 52 to 59 in 59eeeb1

	if options.idp_cert_fingerprint_validator
	fingerprint_exists = options.idp_cert_fingerprint_validator[response_fingerprint]
	unless fingerprint_exists
	raise OmniAuth::Strategies::SAML::ValidationError.new("Non-existent fingerprint")
	end
	# id_cert_fingerprint becomes the given fingerprint if it exists
	options.idp_cert_fingerprint = fingerprint_exists
	end

(and in the response_fingerprint method).

Since that code is extracting the fingerprint from the SAML response directly, it is happening before ruby-saml does its own validation of the response.

I agree with you that this feature is rather suspect and equivalent functionality could be provided by the OmniAuth setup phase functionality in any use case I could think of. That being said, I could be missing something.

[bufferoverflow]

closed via c573690

[bdeterling]

One of my projects was using this for logging issues with fingerprints. It allowed me to configure it, but it was silently not being called. Either having it raise an error that it's an invalid config option or a deprecation would have been helpful. Not complaining, I appreciate you providing this, just something to consider going forward when removing options. Thanks.