The thing is that signature verification is a very fundamental part of SAML SSO and I was too surprised and intrigued that this was not checked at all. I had to submit a report in Hackerone, but first I needed to know why.

So, in 30 mins time (counting the time it took to figure out how to run the VM with less than 14GB of RAM) I had a very serious bug in my hands. The impact of it was quite severe:

If you were too bored to refresh your SAML knowledge above, the equivalent of a Service Provider accepting unsigned SAML assertions is accepting a username without checking the password. Effectively on the flow described above, on step 5, GHE SAML SP accepted any SAML Assertion assuming it was well formed and valid without checking it's authenticity.

The first thing I tried was to disable signing the SAML Response and the SAML Assertion that my Identity Provider was sending to the GHE Service Provider. I did that more for due diligence so that I can move on to more promising test cases and almost couldn't believe it when the authentication succeeded.

A few greps later, I figured out that the SAML implementation is contained within the /data/github/current/lib/saml directory. Ruby is not my strong point but the code seemed straightforward enough. A quick grep for signature left me more perplexed than before as I could see that there are code paths to handle the verification of the Signatures in the SAML Response

The verification process for an incoming SAML Response starts at /data/github/current/lib/github/authentication/saml.rb which deals with the HTTP POST request to the Assertion Consuming Service Endpoint and specifically in the get_auth_failure_result method

def get_auth_failure_result ( saml_response , request , log_data ) unless saml_response . in_response_to || idp_initiated_sso? || :: SAML . mocked [ :skip_in_response_to_check ] return GitHub :: Authentication :: Result . external_response_ignored end unless saml_response . valid? ( :issuer => configuration [ :issuer ] , :idp_certificate => idp_certificate , :sp_url => configuration [ :sp_url ] ) log_auth_validation_event ( log_data , "failure - Invalid SAML response" , saml_response , request . params ) return GitHub :: Authentication :: Result . failure :message => INVALID_RESPONSE end if saml_response . request_denied? log_auth_validation_event ( log_data , "failure - RequestDenied" , saml_response , request . params ) return GitHub :: Authentication :: Result . failure :message => saml_response . status_message || REQUEST_DENIED_RESPONSE end unless saml_response . success? log_auth_validation_event ( log_data , "failure - Unauthorized" , saml_response , request . params ) return GitHub :: Authentication :: Result . failure :message => UNAUTHORIZED_RESPONSE end if request_tracking? && ! in_response_to_request? ( saml_response , request ) log_auth_validation_event ( log_data , "failure - Unauthorized - In Response To invalid" , saml_response , request . params ) return GitHub :: Authentication :: Result . failure :message => UNAUTHORIZED_RESPONSE end end

The interesting part starts when valid? is called:

unless saml_response . valid? ( :issuer => configuration [ :issuer ] , :idp_certificate => idp_certificate , :sp_url => configuration [ :sp_url ] ) log_auth_validation_event ( log_data , "failure - Invalid SAML response" , saml_response , request . params ) return GitHub :: Authentication :: Result . failure :message => INVALID_RESPONSE end

The valid? method of saml_response actually calls validate from of the Message class ( /lib/saml/message.rb )

# Public: Validates schema and custom validations. # # Returns false if instance is invalid. #errors will be non-empty if # invalid. def valid? ( options = {}) errors . clear validate_schema && validate ( options ) errors . empty? end

and in turn validate method called above is implemented in Response class, that implements Message in /data/github/current/lib/saml/message/response.rb

def validate ( options ) if ! SAML . mocked [ :skip_validate_signature ] && options [ :idp_certificate ] validate_has_signature validate_signatures ( options [ :idp_certificate ] ) end validate_issuer ( options [ :issuer ] ) validate_destination ( options [ :sp_url ] ) validate_recipient ( options [ :sp_url ] ) validate_conditions validate_audience ( options [ :sp_url ] ) validate_name_id_format ( options [ :name_id_format ] ) end

So I ended here and I had no clear way of knowing whether validate_has_signature and validate_signatures where executed or not. SAML.mocked would need to have been set to true somewhere and this would affect everything which seemed rather improbable, and I was certain that the idp_certificate was set since one cannot complete the SAML configuration part in the admin UI without setting this.

The only way to know was to debug the functionality, the way debugging was meant to be done: Print statements. Jokes aside, having limited exposure to Ruby and unicorn adding puts or pp statements was the easiest way for me to get some insights at that point.

So I replaced the obfuscated code with the de-obfuscated version of /data/github/current/lib/saml/message/response.rb and changed the following

def validate ( options ) pp options if ! SAML . mocked [ :skip_validate_signature ] && options [ :idp_certificate ] puts 'Going to validate the signature' validate_has_signature validate_signatures ( options [ :idp_certificate ] ) end ...

Next I had to figure out what runs the ruby application, so that I would know which logs to check for for the output.

I started of by seeing what listens on port 443 and figured out that it is haproxy that then passes on the request to nginx which then passes it to unicorn. Using systemctl list-units I then found that the name of the service is github-unicorn and from the data/github/current/config/unicorn.rb file the location of the log file at /var/log/githib/unicorn.log

Armed with the knowledge above, I restarted the service, performed an authentication and took a look at the log to see what's going on and saw the following:

{ :issuer => "https://idp.ikakavas.gr" , :idp_certificate => nil , :sp_url => "https://192.168.122.244" }

Since :idp_certificate was nil, !SAML.mocked[:skip_validate_signature] && options[:idp_certificate] validated to false, and validate_has_signature and validate_signatures that would actually check the validity of the signatures were never executed!!

Digging deeper to the source of the issue and the actual bug, I traced back to /data/github/current/lib/github/authentication/saml.rb where the valid is called

unless saml_response . valid? ( :issuer => configuration [ :issuer ] , :idp_certificate => idp_certificate , :sp_url => configuration [ :sp_url ] )

and the method idp_certificate . It looks like this:

# Public: Returns a string containing the IdP certificate or nil. def idp_certificate @idp_certificate ||= if configuration [ :idp_certificate ] configuration [ :idp_certificate ] elsif configuration [ :idp_certificate_path ] File . read ( configuration [ :idp_certificate_path ] ) end end

I kept staring at it, and nothing seemed off. I couldn't spot any error so "puts to the rescue!" A few minutes later (unicorn restart took quite some time with 4GB of RAM) I was looking at what the configuration Hash looked like

{ :sso_url => "http://idp.ikakavas.gr/sso" , :idp_initiated_sso => false , :disable_admin_demote => false , :issuer => "https://idp.ikakavas.gr" , :signature_method => "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" , :digest_method => "http://www.w3.org/2000/09/xmldsig#sha1" , :idp_certificate_file => "/data/user/common/idp.crt" , :sp_pkcs12_file => "/data/user/common/saml-sp.p12" , :admin => nil , :profile_name => nil , :profile_mail => nil , :profile_key => nil , :profile_gpg_key => nil , :sp_url => "https://192.168.122.244" }

The bug was staring me in the face. And it was a simple one.