Bug in ssl_certificate.erl in R13B04

[Matthew Sackman]

ssl_certificate.erl:

find_issuer(OtpCert, PrevCandidateKey) ->
    case ssl_certificate_db:issuer_candidate(PrevCandidateKey) of
 	no_more_candidates -> ...
 	{Key, {_Cert, ErlCertCandidate}} -> ...
    end.

The problem is that ssl_certificate_db:issuer_candidate returns
no_more_candidates | {Key, [Candidate]}, which is not what its own
comments suggest:

%% Function: issuer_candidate() -> {Key, Candidate} | no_more_candidates   

issuer_candidate(no_candidate) ->
    Db = certificate_db_name(),
    case ets:first(Db) of
 	'$end_of_table' ->
 	    no_more_candidates;
 	Key ->
	    [Cert] = lookup(Key, Db),
 	    {Key, Cert}
    end;

So we assume that lookup returns a 1 element list. Fine:

lookup(Key, Db) ->
    case ets:lookup(Db, Key) of
	[] ->
	    undefined;
	Contents  ->
	    Pick = fun({_, Data}) -> Data;
		      ({_,_,Data}) -> Data
		   end,
	    [Pick(Data) || Data <- Contents]
    end.

Still looking fine, but what happens if the inner Data (in the Pick
function) itself is a list? And can this happen? Well yes, it seems it
can. If we look at cache_pem_file, we see:

cache_pem_file(Pid, File, [CertsDb, _FileToRefDb, PidToFileDb]) ->
    Res = {ok, Content} = public_key:pem_to_der(File),
    insert({file, File}, Content, CertsDb),
    insert(Pid, File, PidToFileDb),
    Res.

The CertsDB here is, I believe the certificate_db_name() mentioned
above. The Content pem_to_der returns a list:

%% Function: pem_to_der(CertSource) ->
%%           pem_to_der(CertSource, Password) -> {ok, [Entry]} |
%%                                               {error, Reason}

And then we do the insert:

insert(Key, Data, Db) ->
    true = ets:insert(Db, {Key, Data}).

Which takes the list Data, and puts it in a tuple. Hence we end up with
a list as the Data in the db, which then blows up the case in
find_issuer right at the top.

This didn't happen in R13B03 because cache_pem_file is totally
different:

cache_pem_file(Pid, File, [_CertsDb, FileToRefDb, PidToFileDb]) ->
    try ref_count(File, FileToRefDb,1)
    catch _:_ -> 
	    {ok, Content} = public_key:pem_to_der(File),
	    insert(File,Content,1,FileToRefDb)
    end,
    insert(Pid, File, PidToFileDb),
    {ok, FileToRefDb}.

For one thing, it never gets added to the CertsDB. Anyway, just to
demonstrate this is really causing a problem, this bug has broken SSL
support in Rabbit under R13B04:

CRASH REPORT==== 4-Apr-2010::15:15:58 ===
  crasher:
    initial call: ssl_connection:init/1
    pid: <0.309.0>
    registered_name: []
    exception exit: {{case_clause,
                         {{file,
                             
"/home/matthew/rabbitmq-umbrella/rabbitmq-test/certs/server/cert.pem"},
                          [{cert,
                               <<48,130,2,237,48,130,1,213,160,3,2,1,2,2,1,2,
                                 48,13,6,9,42,134,72,134,247,13,1,1,5,5,0,48,
                                 ....lots more....
                                 102,134,32,110,107,45,24,26>>,
                               not_encrypted}]}},
                     [{ssl_certificate,find_issuer,2},
                      {ssl_certificate,certificate_chain,4},
                      {ssl_handshake,certificate,3},
                      {ssl_connection,certify_server,1},
                      {ssl_connection,server_certify_and_key_exchange,1},
                      {ssl_connection,do_server_hello,2},
                      {lists,foldl,3},
                      {ssl_connection,handle_event,3}]}
      in function  gen_fsm:terminate/7
    ancestors: [ssl_connection_sup,ssl_sup,<0.232.0>]
    messages: []
    links: [<0.236.0>]
    dictionary: []
    trap_exit: false
    status: running
    heap_size: 2584
    stack_size: 24
    reductions: 1905
  neighbours:

It's possible we're doing something wrong, but my reading of the code
above does seem to suggest there's a bug in the ssl code.

Matthew