try to find a valid certificate in the chain

[benoitc]

should fix #240 . I am not sure yet how secure it is. At least both sites tested pass while invalid don't.

1> application:ensure_all_started(hackney).
{ok,[crypto,asn1,public_key,ssl,idna,certifi,hackney]}
2> application:ensure_all_started(hackney).
{ok,[]}
3> hackney:get("https://friendpaste.com").
{ok,200,
    [{<<"Server">>,<<"nginx/0.7.62">>},
     {<<"Date">>,<<"Thu, 10 Sep 2015 16:10:47 GMT">>},
     {<<"Content-Type">>,<<"text/html; charset=utf-8">>},
     {<<"Transfer-Encoding">>,<<"chunked">>},
     {<<"Connection">>,<<"keep-alive">>},
     {<<"Set-Cookie">>,
      <<"FRIENDPASTE_SID=1bb8b0188ff993581f197300792ef0237a4e08c7; expires=Thu, 2"...>>},
     {<<"Access-Control-Allow-Origin">>,<<"None">>},
     {<<"Access-Control-Allow-Credentials">>,<<"true">>},
     {<<"Access-Control-Allow-Methods">>,
      <<"POST, GET, PUT, DELETE, OPTIONS">>},
     {<<"Access-Control-Allow-Headers">>,
      <<"X-Requested-With, X-HTTP-Method-Override, Content-Type, "...>>},
     {<<"Access-Control-Max-Age">>,<<"86400">>}],
    #Ref<0.0.3.86>}
4> hackney:get("https://rest-api.nl").
{ok,200,
    [{<<"Server">>,<<"nginx">>},
     {<<"Date">>,<<"Thu, 10 Sep 2015 16:20:24 GMT">>},
     {<<"Content-Type">>,<<"text/html">>},
     {<<"Content-Length">>,<<"2914">>},
     {<<"Connection">>,<<"keep-alive">>},
     {<<"X-Powered-By">>,<<"PleskLin">>},
     {<<"MS-Author-Via">>,<<"DAV">>}],
    #Ref<0.0.3.113>}
5> hackney:get("https://tv.eurosport.com/").

=ERROR REPORT==== 10-Sep-2015::18:21:24 ===
SSL: certify: ssl_handshake.erl:1492:Fatal error: handshake failure
{error,{tls_alert,"handshake failure"}}

[benmmurphy]

afaik this disables verification of the chain and all you need is a certificate with a matching hostname in order for it to verify. it doesn't look it requires the cert with the valid hostname to chain back to a root cert.

[benoitc]

@benmmurphy hrm what if it just ignore the self signed certificate and continue then?

Part of the queston is if there is any valid doc that could explain how to reorder a chain like the browsers or curl do :/

[rlipscombe]

Using openssl s_client -connect rest-api.pay.nl:443 -CAfile /etc/ssl/certs/ca-certificates.crt returns the certificate chain as follows:

Certificate chain
 0 s:/C=NL/postalCode=3201BB/ST=Zuid-Holland/L=Spijkenisse/street=Voorstraat 2/O=Tintel B.V./OU=Hosted by Cyso/OU=Wildcard SSL/CN=*.pay.nl
   i:/C=GB/ST=Greater Manchester/L=Salford/O=COMODO CA Limited/CN=COMODO RSA Organization Validation Secure Server CA
 1 s:/C=SE/O=AddTrust AB/OU=AddTrust External TTP Network/CN=AddTrust External CA Root
   i:/C=SE/O=AddTrust AB/OU=AddTrust External TTP Network/CN=AddTrust External CA Root
 2 s:/C=GB/ST=Greater Manchester/L=Salford/O=COMODO CA Limited/CN=COMODO RSA Certification Authority
   i:/C=SE/O=AddTrust AB/OU=AddTrust External TTP Network/CN=AddTrust External CA Root
 3 s:/C=GB/ST=Greater Manchester/L=Salford/O=COMODO CA Limited/CN=COMODO RSA Organization Validation Secure Server CA
   i:/C=GB/ST=Greater Manchester/L=Salford/O=COMODO CA Limited/CN=COMODO RSA Certification Authority

You can see that the chain is out of order in the response, but that if you connect the issuers (s: is subject, i: is issuer), you can build it back into a chain in the correct order: 0 -> 3 -> 2 -> 1. Assuming that certificate 1 is in the trusted roots, then you can trust the chain.

You can see that openssl has reordered the certificates here:

depth=3 C = SE, O = AddTrust AB, OU = AddTrust External TTP Network, CN = AddTrust External CA Root
verify return:1
depth=2 C = GB, ST = Greater Manchester, L = Salford, O = COMODO CA Limited, CN = COMODO RSA Certification Authority
verify return:1
depth=1 C = GB, ST = Greater Manchester, L = Salford, O = COMODO CA Limited, CN = COMODO RSA Organization Validation Secure Server CA
verify return:1
depth=0 C = NL, postalCode = 3201BB, ST = Zuid-Holland, L = Spijkenisse, street = Voorstraat 2, O = Tintel B.V., OU = Hosted by Cyso, OU = Wildcard SSL, CN = *.pay.nl
verify return:1

As far as how you should actually verify each step in the chain, I guess you'd need to look at the OTP SSL code, or in openssl, or maybe the PKI stuff in OTP will do it properly once the chain's in the correct order. And if it will, then probably a PR fixing it in OTP would be useful.

[rlipscombe]

And if it will, then probably a PR fixing it in OTP would be useful.

Except that I see that Ingela has already said on the mailing list that she'd prefer not to do that. Maybe a PR making it configurable would be useful...?

[benmmurphy]

my code is correct assuming you are able to substitute all the CA certs for the single CA cert is use. the correct solution is to find a path from the first certificate the in the chain to CA root. you can do this by a DFS. the go code by adam langley is the best example of this.

https://github.com/golang/go/blob/master/src/crypto/x509/verify.go

[voltone]

How about a project that bundles a set of custom verify_fun/2 implementations, with different capabilities? Any :ssl client, direct or indirect (e.g. through Hackney), can then pass a parameter to select the most appropriate one. Hackney could choose a preferred default, but users could override that. This could be an evolution of the ssl_verify_hostname package.

Examples include: reordering cert chain (yes/no), CRL/OCSP checking (yes/no), support for certain X.509 extensions, etc.

My concern is that if different libraries start implementing their own verify_fun to replace OTP's, it will be difficult for users to verify the security of each one. A centralised project would hopefully attract experts who can review and fix any security issues.

[benoitc]

@voltone sounds like a good idea. It would help a lot of people and make sure to use the right code among projects...

[benoitc]

@benmmurphy I was looking at it. I am not sure yet though how the hostname checking is working. It seems to me that such thing should happen after the chain have been reordered.. I am thinking it should work that way:

1. sort the chain in the right order by reordering issuers
2. verify the chain then the hostname.

The part I am not sure to follow in the code the pasted is the way you handle the self signed certificate. Can you elaborate on it?

[rlipscombe]

The part I am not sure to follow in the code the pasted is the way you handle the self signed certificate. Can you elaborate on it?

CA certificates are self-signed certificates. That is: the root of the chain is always self-signed. The only thing that makes them "CA" certificates is that they're in a trusted list[1].

If you find a self-signed certificate anywhere else in the chain, then it's not a chain, because subject == issuer and you've broken the chain.

[1] There's also an is-certificate-authority flag in the cert, but that doesn't make them trusted roots.

[deadtrickster]

@rlipscombe

you should actually verify each step in the chain

Sometimes servers send only 'first' certificate without intermediates bundle, to actually verify each step in the chain client should implement AIA which is ok for Hackney, not sure what to do with non-http clients.

btw. this is why most problem domains work in browsers - they can download intermediate certs themselves.

[benmmurphy]

@benoitc in the code i posted we don't verify the hostname because this is used internally with a custom CA and we trust all the certs generated by this CA. [which may actually be a mistake now i think about it :)]

but you can add hostname verification by passing a different function other than default_verify into the public_key:pkix_path_validation. basically pkix_path_validation is being called twice. once by ssl where we hook the validation function to get access to the full chain and then again from our validation function to validate the chain we generate.

so there are two major improvements that need to be made:

1. the code needs to access the CertDb to look up issuers instead of using a single root certificate
2. in the situation where is there is no errors it should not try to rebuild the chain and should just pass success from the verify function or do the next verification step (hostname).

and yeah. when you start to see the code for 1) you will understand why this needs to be done in OTP or for OTP to provide proper hooks for it.

-module(fixed_root_lenient_verifier).

-record(state, {certs = [], root_cert, max_path_length}).

-export([verify/3]).
-export([create_verify_function/2]).

create_verify_function(DerCertificate, MaxPathLength) ->
  DecodedCert = public_key:pkix_decode_cert(DerCertificate, otp),
  {fun fixed_root_lenient_verifier:verify/3, #state{root_cert = DecodedCert, max_path_length = MaxPathLength}}.

append_cert(State, Cert) ->
  State#state{certs = [Cert | State#state.certs]}.

default_verify(_Cert, Event, State) ->
  case Event of
    {bad_cert, Reason} ->  {fail, Reason};
    {extension, _} -> {unknown, State};
    valid -> {valid, State};
    valid_peer -> {valid, State}  
  end.


verify(Cert, Event, State) ->
  case Event of
    {bad_cert, _Reason} ->
      {valid, append_cert(State, Cert)};
    {extension, _} -> {unknown, State};
    valid -> {valid, append_cert(State, Cert)};
    valid_peer -> 
      case final_verification(State#state.certs, Cert, State#state.max_path_length, State#state.root_cert) of
        {ok, _} -> {valid, State};
        {error, Reason} -> {fail, Reason}
      end
  end.

final_verification(Certs, PeerCert, MaxPathLength, RootCertificate) ->
  Chain = fix_path(PeerCert, Certs),
  public_key:pkix_path_validation(RootCertificate, Chain, [{max_path_length, MaxPathLength}, {verify_fun, {fun default_verify/3, none}}]).


fix_path(Peer, RestOfChain) ->
  make_chain(Peer, RestOfChain, []).

make_chain(OtpCert, CertChain, ResultChain) ->
  case public_key:pkix_is_self_signed(OtpCert) of
    true ->
      [OtpCert | ResultChain ];
    false ->
        case find_issuer(OtpCert, CertChain) of
          {ok, NewCert, NewCertChain} ->
            % we remove the cert that was found from the chain
            % to prevent infinite loops where a chain becomes
            % a loop
            make_chain(NewCert, NewCertChain, [OtpCert | ResultChain]);
          {error, issuer_not_found} ->
            % assume it is the 'trusted' certificate
            [OtpCert | ResultChain]
        end
  end.

find_issuer(OtpCert, CertChain) ->
  {Not, Maybe} = lists:splitwith(fun(Candidate) ->
    not public_key:pkix_is_issuer(OtpCert, Candidate)
  end, CertChain),

  case Maybe of
    [Issuer | Rest] ->
      {ok, Issuer, Not ++ Rest};
    [] ->
      {error, issuer_not_found}
  end.

[benoitc]

@benmmurphy thanks for the code. I am trying right now to have a play but using a list of trusted root instead of one rootcertifcate. No success yet though :)

@deadtrickster do you have any link about that that could helps me to figure what to do exactly. Basically Hackney should act like a browser. Any patch that would help to implement it would be definitely accepted :)

[benoitc]

@rlipscombe right right, was more thinking to self signed certificate that are not trusted

[rlipscombe]

Basically Hackney should act like a browser.

I'd prefer this to be optional. That is: I'd like to be able to decide on a certificate verification function per connection (because I might want to use different trusted roots, different self-signed certificates, etc.); if we could invent a suite of different verification strategies, as @voltone suggests, then this could be pluggable.

[benoitc]

@rlipscombe i am speaking about the defaults. You can still override a connection with your own ssl options like usual :)

[benoitc]

@deadtrickster also I am agree we need an hih level framework that would allow to validate an SSL connection without having to know much about the openssl API.

[deadtrickster]

@benoitc
you can find more on AIA here -> https://tools.ietf.org/html/rfc5280#section-5.2.7
I'm totally agree about framework (something like erlang-ssl-toolbox) but why you mention openssl?

[benoitc]

@deadtrickster i mean the erlang SSL/public_key api :)

[deadtrickster]

Also very interesting reading Certification path building -> https://tools.ietf.org/html/rfc4158.

Speeking of openssl, what to do with older OTP releases? they are actually based on openssl aren't they?

Honestly I feel like this (stuff we are talking about here) has to be part of OTP itself actually

[benoitc]

@deadtrickster what would be the point of having it in OTP? Is this easier to handle at that level?

[deadtrickster]

@benoitc

OTP already has ssl, public_key and friends, I also know they rewrote everything from scratch to move away from openssl.
It looks very natural to me to include dunno hook (like verify_fun) that handles downloads from AIA section. Scenario: Head seritificate downloaded without intermediates, OTP extracts information from AIA section and calls that hook if enabled by settings.
This makes possible at least two things: 1) global intermediates caching (just like browsers cache downloaded intermediates) 2) custom download handler - this at least greatly improves error reporting - i.e. you will know not only that chain is invalid but also why and can deal with this information (i.e. get certs from private storage, just like now people playing with verify_fun)
Personally I think hostname verification should be part of OTP too.

That cache stuff leaves open questions however, about you know per application (library) cache, etc...
And yes I think this is not only easier to handle in OTP itself, but also as I said more natural since we are talking not about strange corner cases but rather formalized and wide-known stuff.

[deadtrickster]

this code extracts AIA extension and prints uri entries from google.com certificate. URL presented in aia extension points to intermediate GIAG2 certificate

Certificate chain for google.com

0 s:/C=US/ST=California/L=Mountain View/O=Google Inc/CN=*.google.com
   i:/C=US/O=Google Inc/CN=Google Internet Authority G2
 1 s:/C=US/O=Google Inc/CN=Google Internet Authority G2
   i:/C=US/O=GeoTrust Inc./CN=GeoTrust Global CA
 2 s:/C=US/O=GeoTrust Inc./CN=GeoTrust Global CA
   i:/C=US/O=Equifax/OU=Equifax Secure Certificate Authority

extensions_list(E) ->
  case E of
    asn1_NOVALUE -> [];
    _ -> E
  end.

select_extension(Id, Extensions) ->
  Matching = [Extension || #'Extension'{extnID = ExtId} = Extension <- Extensions, ExtId =:= Id],
  case Matching of
    [] -> undefined;
    [H|_] -> H
  end.

select_ca_issuers(ADs) ->
  Matching = [AccessDescription#'AccessDescription'.accessLocation || #'AccessDescription'{accessMethod = ADId} = AccessDescription <- ADs, ADId =:= ?'id-ad-caIssuers'],
  case Matching of
    [] ->
      undefined;
    [AD|_] -> AD
  end.


test_aia() ->
  Cert = google_cert(),
  TBSCert = Cert#'OTPCertificate'.tbsCertificate,
  Extensions = extensions_list(TBSCert#'OTPTBSCertificate'.extensions),
  AIAExt = select_extension(?'id-pe-authorityInfoAccess', Extensions),
  case AIAExt of
    undefined ->
      [];
    _ ->
      select_ca_issuers(AIAExt#'Extension'.extnValue)
  end.

Output:

ssl_verify_hostname_tests:test_aia().
{uniformResourceIdentifier,"http://pki.google.com/GIAG2.crt"}

the code is dead simple - more complex part is actually downloading certificates (also that LDAP...)

[benmmurphy]

Just a heads up. The code I've posted is broken as well. :( it accepts all self signed certs. yay. So depending on the bad_cert error it will skip the valid_peer or valid step so you can't rely on valid_peer or valid being called. :/

You can see it here where it deletes your verify callback and recursively calls path validation.

https://github.com/erlang/otp/blob/maint/lib/public_key/src/public_key.erl#L671

also i think this bug has been a little confused because the problem site was rest-api.pay.nl (which has reordering problems) but the test case at the top is for rest-api.nl (which is self-signed and should not be accepted)

[benmmurphy]

ok here is my attempt to use the ca certs cached by the ssl app.

so remaining issues are:

1. no control over the final verification function. but should be quite simple
2. doesn't decrement the ref count for the cached data
3. calls a bunch of internal OTP functions that are version specific (this works on 16B02): ssl_manager:connection_init and ssl_certificate:trusted_cert_and_path are different on different version of OTP. possibly we don't need to call trusted_cert_and_path because we already know the cert and path. i figured just run things through the OTP function just in case.

but OTP really needs to just supply a hook point which is. reorder_chain(PeerCert, Chain, CertDBHandle, CertDBRef) and you can just send back another chain that is however you want it to look like. if this was done right it would fix the broken partial chain function that is super dangerous.

also this might not work in all cases. i suspect the reordering will only work if the last certificate is either in the CA store or issues by a cert in the CA store because of the protection i've had to add to prevent accepting invalid chains. however, i think this can be fixed by a partial chain handler that always returns a CA (even if it can't find a proper one) then instead of bailing on unknown_ca it will bail on invalid_issuer which is safe to recover from.

-module(lenient_ssl).

-record(state, {certs = [], 
                cacerts, 
                max_path_length, 
                error = false, 
                verify_function, 
                verify_function_state}).

-export([verify/3]).
-export([create_verify_function/2]).

create_verify_function(CaCerts, MaxPathLength) ->
  {fun lenient_ssl:verify/3, 
    #state{cacerts = CaCerts, 
           max_path_length = MaxPathLength,
           verify_function = fun default_verify/3,
           verify_function_state = none}}.

append_cert(State, Cert) ->
  State#state{certs = [Cert | State#state.certs]}.

default_verify(_Cert, Event, State) ->
  case Event of
    {bad_cert, Reason} ->  {fail, Reason};
    {extension, _} -> {unknown, State};
    valid -> {valid, State};
    valid_peer -> {valid, State}  
  end.


verify(Cert, Event, State) ->
  case Event of
    {bad_cert, Reason} ->
      % known path validation errors that cause valid/valid_peer not be called
      % and the cert to be recognized
      case Reason =:= unknown_ca orelse Reason =:= selfsigned_peer of
        true ->
          {fail, Reason};
        false ->
          {valid, (append_cert(State, Cert))#state{error = true}}
      end;
    {extension, _} -> {unknown, State};
    valid ->  {valid, append_cert(State, Cert)};
    valid_peer ->
      case State#state.error of
        false ->
          (State#state.verify_function)(Cert, valid_peer, State#state.verify_function_state);
        true ->
          case final_verification(State, Cert) of
            {ok, _} -> {valid, State};
            {error, Reason} -> {fail, Reason}
          end
      end
  end.




final_verification(State, PeerCert) ->


  {ok, CertDbRef, CertDbHandle, _FileRefHandle, _PemCacheHandle, _CacheHandle} =
      ssl_manager:connection_init(State#state.cacerts, client),  

  % TODO: need to decrement ref count somewhere

  Chain = fix_path(CertDbHandle, CertDbRef, PeerCert, State#state.certs),


  DerChain = lists:reverse([public_key:pkix_encode('OTPCertificate', Cert, otp) || Cert <- Chain]),

  {TrustedErlCert, CertPath}  =
          ssl_certificate:trusted_cert_and_path(DerChain, CertDbHandle, CertDbRef),


  public_key:pkix_path_validation(TrustedErlCert, CertPath, [{max_path_length, State#state.max_path_length}, {verify_fun, {State#state.verify_function, State#state.verify_function_state}}]).


fix_path(CertDbHandle, CertDbRef, Peer, RestOfChain) ->

  make_chain(CertDbHandle, CertDbRef, Peer, RestOfChain, []).

make_chain(CertDbHandle, CertDbRef, OtpCert, CertChain, ResultChain) ->
  case public_key:pkix_is_self_signed(OtpCert) of
    true ->
      [OtpCert | ResultChain ];
    false ->
        case find_issuer(CertDbHandle, CertDbRef, OtpCert, CertChain) of
          {ok, NewCert, NewCertChain} ->
            % we remove the cert that was found from the chain
            % to prevent infinite loops where a chain becomes
            % a loop
            make_chain(CertDbHandle, CertDbRef, NewCert, NewCertChain, [OtpCert | ResultChain]);
          {error, issuer_not_found} ->
            % assume it is the 'trusted' certificate
            [OtpCert | ResultChain]
        end
  end.


find_issuer(CertDbHandle, CertDbRef, OtpCert, CertChain) ->
  case find_issuer_in_ca_db(CertDbHandle, CertDbRef, OtpCert) of
    {ok, IssuerCert} ->
      {ok, IssuerCert, []};
    {error, not_found} ->
      find_issuer_in_intermediates(OtpCert, CertChain)
  end.


find_issuer_in_ca_db(CertDbHandle, CertDbRef, OtpCert) ->
  case public_key:pkix_issuer_id(OtpCert, other) of
    {ok, {SerialNr, Issuer}} ->
      case ssl_manager:lookup_trusted_cert(CertDbHandle, CertDbRef, SerialNr, Issuer) of
        {ok, {BinCert,_}} ->
          {ok, public_key:pkix_decode_cert(BinCert, otp)};
        _ ->
          {error, not_found}
      end;
    _ ->
      {error, not_found}
  end.

find_issuer_in_intermediates(OtpCert, CertChain) ->
  {Not, Maybe} = lists:splitwith(fun(Candidate) ->
    not public_key:pkix_is_issuer(OtpCert, Candidate)
  end, CertChain),

  case Maybe of
    [Issuer | Rest] ->
      {ok, Issuer, Not ++ Rest};
    [] ->
      {error, issuer_not_found}
  end.



Erlang R16B02 (erts-5.10.3) [source] [smp:4:4] [async-threads:10] [hipe] [kernel-poll:false]

Eshell V5.10.3  (abort with ^G)
1> application:ensure_all_started(ssl).
{ok,[crypto,asn1,public_key,ssl]}
2> ssl:connect("rest-api.pay.nl", 443, [{verify, verify_peer}, {verify_fun, lenient_ssl:create_verify_function("/Users/benmurphy/git/hackney/ca-bundle.crt", 10)}, {cacertfile, "/Users/benmurphy/git/hackney/ca-bundle.crt"}]).
{ok,{sslsocket,{gen_tcp,#Port<0.1131>},<0.51.0>}}

[benoitc]

@benmmurphy Nice!

I think you're right It would be better to have this hook function. Are you working on a patch?

Also I wonder if waiting this hook function we couldn't add this verification directly to hackney and mark it as experimental since it rely on non documented function? Maybe you could create a PR with it? What is the effect of not decrementing the refcount?

[benoitc]

@deadtrickster That's neat. Sounds "easy" to add... It could eventually be combined with the partial function though not sure yet how to do it.

Btw the google_cert/0 function is missing your snippet.

[benmmurphy]

I'm not working on an OTP patch.

I've thought more deeply about this and I don't think generic reordering can be done without OTP support or taking over verification of SSL certs completely. so before i was hoping we would only hit the reorder code in a fallback but i think it will be necessary to always run the reorder code.

This is because:

1. if unknown_ca or selfsigned_peer is supplied into the verification handler it is unsafe to return a success result because no further calls will be made to the verification handler
2. if the last cert in the chain is not trusted an unknown_ca error will be generated. presumably this happens if we have Trusted CA -> Intermediate 1-> Intermediate 2 -> Server Cert and Intermediate 2 is the last cert in the chain.
3. if we use partial_chain to fix this problem we MUST return the last cert in the chain as the trusted root or OTP will remove certificates in the chain and we won't be able to verify the server cert.
4. in the partial chain function we don't know if 'Intermediate 2' is safe to trust or not. however, when partial_chain is called we could try and find a path from a root to 'Intermediate 2' and return the chain based on that. but if we are going to do this we may as well just blindly return the last cert in the path as trusted and then completely override the verification functionality using the callback.

[deadtrickster]

I played with incomplete chains more last weekend and here is what I found:

1. by default otp throws {tls_alert, unknown_ca} and calls both partial_chain and verify_fun
2. verify_fun and partial_chain looks completely useless unless as @benmmurphy said we completely reimplement chain validation.
3. although they look similar reordering and incomplete chains actually different because first (reordering) probably should be done 100% one the otp side (at least by default, maybe callback with reasonable defaults). For incomplete chains OTP should have different callback (like I said before )which either rejects or downloads certificate.

google_cert can be found in ssl_verify_hostname tests it simply loads google.com cert from the file

[benoitc]

that's a long time. closing it. feel free to reopen it if needed.