Uploaded image for project: 'Erlang/OTP'
  1. Erlang/OTP
  2. ERL-1030

SSL failing to handle a 'cRLIssuer' entry in a CRL distribution point extension

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: OTP 20.3
    • Component/s: ssl
    • Labels:
      None

      Description

      OTP19.3.3

      Attempting an SSL connection with a cert containing a CRL distribution point extension like that below:

      [{'DistributionPoint',
           {fullName,[{uniformResourceIdentifier,"http://blah.com/BLAH/certdist"}]},
           asn1_NOVALUE,
           [{directoryName,
             {rdnSequence,[[{'AttributeTypeAndValue',{2,5,4,3},<<12,6,65,65,65,65,65,65>>}],[{'AttributeTypeAndValue',{2,5,4,10},<<12,3,65,65,65>>}]]}
            }
           ]
       }
      ]
      
      

      i.e. with a non-asn1_NOVALUE cRLIssuer.

      It requires a little instrumentation to get the stacktrace for the
      exception. The top of it is:

          {public_key,short_name_hash,[[{directoryName,{rdnSequence,[[{'AttributeTypeAndValue',{2,5,4,3},<<12,6,65,65,65,65,65,65>>}],[{'AttributeTypeAndValue',{2,5,4,10},<<12,3,65,65,65>>}]]}}]],
                                             [{file,"public_key.erl"},{line,956}]},
          {ssl_crl_hash_dir,find_crls,2,     [{file,"ssl_crl_hash_dir.erl"},{line,69}]},
          {ssl_crl_hash_dir,select,2,        [{file,"ssl_crl_hash_dir.erl"},{line,49}]},
          {ssl_crl_hash_dir,lookup,3,        [{file,"ssl_crl_hash_dir.erl"},{line,40}]},
          {ssl_handshake,distpoints_lookup,4,[{file,"ssl_handshake.erl"},{line,2276}]},
          {ssl_handshake,dps_and_crls,4,     [{file,"ssl_handshake.erl"},{line,2251}]},
          {ssl_handshake,crl_check,7,        [{file,"ssl_handshake.erl"},{line,2219}]},
      
      

      The root cause of the problem seems to be in ssl_crl_hash_dir:lookup/3
      which effectively has to choose between two similar looking 'Issuer'
      entities: 'CertIssuer' is a singleton, but 'CRLIssuer' is potentially
      a list, but both are treated as if singletons.

      I think this selection is as required by [RFC5280,6.3.3(b)1], roughly:

        if   the DP includes cRLIssuer
        then verify that the issuer field in the complete CRL matches
             cRLIssuer in the DP
             and
             that the complete CRL contains an issuing distribution point
             extension with the indirectCRL boolean asserted
        else verify that the CRL issuer matches the certificate issuer
      

      Perhaps only the 'else' branch has been exercised until now?

      Investigatory patch
      -------------------

      diff --git a/lib/public_key/src/pubkey_crl.erl b/lib/public_key/src/pubkey_crl.erl
      index 33bef91..642d109 100644
      --- a/lib/public_key/src/pubkey_crl.erl
      +++ b/lib/public_key/src/pubkey_crl.erl
      @@ -23,7 +23,7 @@
       -include("public_key.hrl").
       
       -export([validate/7, init_revokation_state/0, fresh_crl/3, verify_crl_signature/4,
      -	 is_delta_crl/1, combines/2, match_one/2]).
      +	 is_delta_crl/1, combines/2, match_one/2, dp_crlissuer_to_issuer/1]).
       
       -record(userstate, {dpcrls,
       		    idp
      @@ -333,6 +333,7 @@ verify_issuer_and_scope(#'OTPCertificate'{tbsCertificate = TBSCert}= Cert,
           end.
       
       dp_crlissuer_to_issuer(DPCRLIssuer) ->
      +     %% Assume the cRLIssuer SEQUENCE is of length exactly 1
            [{directoryName, Issuer}] = pubkey_cert_records:transform(DPCRLIssuer, decode),
            Issuer.
       
      diff --git a/lib/ssl/src/ssl_crl_hash_dir.erl b/lib/ssl/src/ssl_crl_hash_dir.erl
      index bb62737..71f689d 100644
      --- a/lib/ssl/src/ssl_crl_hash_dir.erl
      +++ b/lib/ssl/src/ssl_crl_hash_dir.erl
      @@ -33,7 +33,7 @@ lookup(#'DistributionPoint'{cRLIssuer = CRLIssuer} = DP, CertIssuer, CRLDbInfo)
       		%% indicate a CRL issuer, use the certificate issuer.
       		CertIssuer;
       	    _ ->
      -		CRLIssuer
      +		pubkey_crl:dp_crlissuer_to_issuer(CRLIssuer)
       	end,
           %% Find all CRLs for this issuer, and return those that match the
           %% given distribution point.
      

      However, this does make the assumption that cRLIssuer is a SEQUENCE of
      length exactly 1, or asn1_NOVALUE. But, for better or worse, the
      current usage of pubkey_crl:dp_crlissuer_to_issuer() already makes
      that assumption.

      I'm not steeped well enough in RFC5280 to guess what the implied
      semantics of a list of len > 1 would be; the openssl implementation
      does seem to handle a list of general length and the semantics appear
      to be 'if any dp_issuer matches crl_issuer'.

      Repro and test
      --------------
      I've looked through the OTP SSL tests for somewhere suitable to drop
      in a case for this, but it's all rather new territory for me. I'm sure
      the following is the wrong place, but it does seem to provoke the
      issue.

      diff --git a/release/tests/ssl_test/make_certs.erl b/release/tests/ssl_test/make_certs.erl
      index 7f27f70..254108d 100644
      --- a/release/tests/ssl_test/make_certs.erl
      +++ b/release/tests/ssl_test/make_certs.erl
      @@ -437,14 +436,20 @@ ca_cnf(Root, C = #config{issuing_distribution_point = true}) ->
            "authorityKeyIdentifier = keyid,issuer:always\n"
            "subjectAltName	= email:copy\n"
            "issuerAltName	= issuer:copy\n"
      -     "crlDistributionPoints=@crl_section\n"
      +     "crlDistributionPoints=crl_section\n"
       
            "[crl_section]\n"
            %% intentionally invalid
      -     "URI.1=http://localhost/",C#config.commonName,"/crl.pem\n"
      -     "URI.2=http://localhost:",integer_to_list(C#config.crl_port),"/",C#config.commonName,"/crl.pem\n"
      +     "fullname=URI:http://localhost/",C#config.commonName,"/crl.pem\n"
      +     "fullname=URI:http://localhost:",integer_to_list(C#config.crl_port),"/",C#config.commonName,"/crl.pem\n"
      +     "CRLissuer=dirName:issuer_sect\n"
            "\n"
       
      +     "[issuer_sect]\n"
      +     "C=UK\n"
      +     "O=Organisation\n"
      +     "CN=DEF123\n"
      +
            "[user_cert_digital_signature_only]\n"
            "basicConstraints	= CA:false\n"
            "keyUsage 		= digitalSignature\n"
      
      

      I get:

        Testing tests.ssl_test.ssl_crl_SUITE.crl_hash_dir: TEST COMPLETE, 12 ok, 3 failed of 15 test cases
      

      And the backtrace I get (I need to instrument
      ssl_handshake.erl:certify catch to see it) is the same as shown at the
      top.

      Adding my investigatory means I don't get the indicative traceback,
      suggesting my patch may be effective. But it still leaves two other
      errors. I think I may be turning an 'intentionally invalid'
      crl_section into something valid. I think maybe the testing should go
      somewhere else, but I'm not sure where would be good.

        Attachments

          Activity

            People

            Assignee:
            otp_team_ps Team PS
            Reporter:
            Tim Gleeson Tim Gleeson
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: