From 4d24ae9a4d4b96a3791eb3def2c672d7ec644c8c Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Tue, 19 Oct 2010 09:29:02 +0200 Subject: Correct handling of client certificate verify message When checking the client certificate verify message the server used the wrong algorithm identifier to determine the signing algorithm, causing a function clause error in the public_key application when the key-exchange algorithm and the public key algorithm of the client certificate happen to differ. --- lib/ssl/src/ssl_connection.erl | 21 ++++-------- lib/ssl/src/ssl_handshake.erl | 74 +++++++++++++++++++----------------------- lib/ssl/src/ssl_ssl3.erl | 7 ++-- lib/ssl/src/ssl_tls1.erl | 7 ++-- 4 files changed, 46 insertions(+), 63 deletions(-) (limited to 'lib') diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index c94199c336..c3f16699a1 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -653,12 +653,10 @@ cipher(#certificate_verify{signature = Signature}, public_key_info = PublicKeyInfo, negotiated_version = Version, session = #session{master_secret = MasterSecret}, - key_algorithm = Algorithm, tls_handshake_hashes = Hashes } = State0) -> case ssl_handshake:certificate_verify(Signature, PublicKeyInfo, - Version, MasterSecret, - Algorithm, Hashes) of + Version, MasterSecret, Hashes) of valid -> {Record, State} = next_record(State0), next_state(cipher, Record, State); @@ -1182,16 +1180,15 @@ verify_client_cert(#state{client_certificate_requested = true, role = client, negotiated_version = Version, own_cert = OwnCert, socket = Socket, - key_algorithm = KeyAlg, private_key = PrivateKey, session = #session{master_secret = MasterSecret}, tls_handshake_hashes = Hashes0} = State) -> + case ssl_handshake:client_certificate_verify(OwnCert, MasterSecret, - Version, KeyAlg, - PrivateKey, Hashes0) of + Version, PrivateKey, Hashes0) of #certificate_verify{} = Verified -> {BinVerified, ConnectionStates1, Hashes1} = - encode_handshake(Verified, KeyAlg, Version, + encode_handshake(Verified, Version, ConnectionStates0, Hashes0), Transport:send(Socket, BinVerified), State#state{connection_states = ConnectionStates1, @@ -1585,13 +1582,9 @@ encode_change_cipher(#change_cipher_spec{}, Version, ConnectionStates) -> ?DBG_TERM(#change_cipher_spec{}), ssl_record:encode_change_cipher_spec(Version, ConnectionStates). -encode_handshake(HandshakeRec, Version, ConnectionStates, Hashes) -> - encode_handshake(HandshakeRec, null, Version, - ConnectionStates, Hashes). - -encode_handshake(HandshakeRec, SigAlg, Version, ConnectionStates0, Hashes0) -> +encode_handshake(HandshakeRec, Version, ConnectionStates0, Hashes0) -> ?DBG_TERM(HandshakeRec), - Frag = ssl_handshake:encode_handshake(HandshakeRec, Version, SigAlg), + Frag = ssl_handshake:encode_handshake(HandshakeRec, Version), Hashes1 = ssl_handshake:update_hashes(Hashes0, Frag), {E, ConnectionStates1} = ssl_record:encode_handshake(Frag, Version, ConnectionStates0), @@ -2179,7 +2172,7 @@ renegotiate(#state{role = server, negotiated_version = Version, connection_states = ConnectionStates0} = State0) -> HelloRequest = ssl_handshake:hello_request(), - Frag = ssl_handshake:encode_handshake(HelloRequest, Version, null), + Frag = ssl_handshake:encode_handshake(HelloRequest, Version), Hs0 = ssl_handshake:init_hashes(), {BinMsg, ConnectionStates} = ssl_record:encode_handshake(Frag, Version, ConnectionStates0), diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index b9b1ccb134..a6d39f0af1 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -33,11 +33,11 @@ -export([master_secret/4, client_hello/5, server_hello/4, hello/4, hello_request/0, certify/6, certificate/3, - client_certificate_verify/6, certificate_verify/6, + client_certificate_verify/5, certificate_verify/5, certificate_request/2, key_exchange/2, server_key_exchange_hash/2, finished/4, verify_connection/5, get_tls_handshake/2, - decode_client_key/3, server_hello_done/0, sig_alg/1, - encode_handshake/3, init_hashes/0, update_hashes/2, + decode_client_key/3, server_hello_done/0, + encode_handshake/2, init_hashes/0, update_hashes/2, decrypt_premaster_secret/2]). -type tls_handshake() :: #client_hello{} | #server_hello{} | @@ -252,17 +252,17 @@ certificate(OwnCert, CertDbRef, server) -> %%-------------------------------------------------------------------- -spec client_certificate_verify(undefined | der_cert(), binary(), - tls_version(), key_algo(), private_key(), + tls_version(), private_key(), {{binary(), binary()},{binary(), binary()}}) -> #certificate_verify{} | ignore | #alert{}. %% %% Description: Creates a certificate_verify message, called by the client. %%-------------------------------------------------------------------- -client_certificate_verify(undefined, _, _, _, _, _) -> +client_certificate_verify(undefined, _, _, _, _) -> ignore; -client_certificate_verify(_, _, _, _, undefined, _) -> +client_certificate_verify(_, _, _, undefined, _) -> ignore; -client_certificate_verify(OwnCert, MasterSecret, Version, Algorithm, +client_certificate_verify(OwnCert, MasterSecret, Version, PrivateKey, {Hashes0, _}) -> case public_key:pkix_is_fixed_dh_cert(OwnCert) of true -> @@ -270,33 +270,30 @@ client_certificate_verify(OwnCert, MasterSecret, Version, Algorithm, false -> Hashes = calc_certificate_verify(Version, MasterSecret, - Algorithm, Hashes0), + alg_oid(PrivateKey), Hashes0), Signed = digitally_signed(Hashes, PrivateKey), #certificate_verify{signature = Signed} end. %%-------------------------------------------------------------------- -%% -spec certificate_verify(binary(), public_key_info(), tls_version(), -%% binary(), key_algo(), -%% {_, {binary(), binary()}}) -> valid | #alert{}. +-spec certificate_verify(binary(), public_key_info(), tls_version(), + binary(), {_, {binary(), binary()}}) -> valid | #alert{}. %% %% Description: Checks that the certificate_verify message is valid. %%-------------------------------------------------------------------- -certificate_verify(Signature, {_, PublicKey, _}, Version, - MasterSecret, Algorithm, {_, Hashes0}) - when Algorithm == rsa; - Algorithm == dhe_rsa -> +certificate_verify(Signature, {?'rsaEncryption'= Algorithm, PublicKey, _}, Version, + MasterSecret, {_, Hashes0}) -> Hashes = calc_certificate_verify(Version, MasterSecret, Algorithm, Hashes0), - case public_key:decrypt_public(Signature, PublicKey, + case public_key:decrypt_public(Signature, PublicKey, [{rsa_pad, rsa_pkcs1_padding}]) of Hashes -> valid; _ -> ?ALERT_REC(?FATAL, ?BAD_CERTIFICATE) end; -certificate_verify(Signature, {_, PublicKey, PublicKeyParams}, Version, - MasterSecret, dhe_dss = Algorithm, {_, Hashes0}) -> +certificate_verify(Signature, {?'id-dsa' = Algorithm, PublicKey, PublicKeyParams}, Version, + MasterSecret, {_, Hashes0}) -> Hashes = calc_certificate_verify(Version, MasterSecret, Algorithm, Hashes0), case public_key:verify(Hashes, none, Signature, {PublicKey, PublicKeyParams}) of @@ -441,13 +438,12 @@ server_hello_done() -> #server_hello_done{}. %%-------------------------------------------------------------------- --spec encode_handshake(tls_handshake(), tls_version(), key_algo()) -> iolist(). +-spec encode_handshake(tls_handshake(), tls_version()) -> iolist(). %% %% Description: Encode a handshake packet to binary %%-------------------------------------------------------------------- -encode_handshake(Package, Version, KeyAlg) -> - SigAlg = sig_alg(KeyAlg), - {MsgType, Bin} = enc_hs(Package, Version, SigAlg), +encode_handshake(Package, Version) -> + {MsgType, Bin} = enc_hs(Package, Version), Len = byte_size(Bin), [MsgType, ?uint24(Len), Bin]. @@ -884,14 +880,14 @@ certs_from_list(ACList) -> <> end || Cert <- ACList]). -enc_hs(#hello_request{}, _Version, _) -> +enc_hs(#hello_request{}, _Version) -> {?HELLO_REQUEST, <<>>}; enc_hs(#client_hello{client_version = {Major, Minor}, random = Random, session_id = SessionID, cipher_suites = CipherSuites, compression_methods = CompMethods, - renegotiation_info = RenegotiationInfo}, _Version, _) -> + renegotiation_info = RenegotiationInfo}, _Version) -> SIDLength = byte_size(SessionID), BinCompMethods = list_to_binary(CompMethods), CmLength = byte_size(BinCompMethods), @@ -909,20 +905,20 @@ enc_hs(#server_hello{server_version = {Major, Minor}, session_id = Session_ID, cipher_suite = Cipher_suite, compression_method = Comp_method, - renegotiation_info = RenegotiationInfo}, _Version, _) -> + renegotiation_info = RenegotiationInfo}, _Version) -> SID_length = byte_size(Session_ID), Extensions = hello_extensions(RenegotiationInfo), ExtensionsBin = enc_hello_extensions(Extensions), {?SERVER_HELLO, <>}; -enc_hs(#certificate{asn1_certificates = ASN1CertList}, _Version, _) -> +enc_hs(#certificate{asn1_certificates = ASN1CertList}, _Version) -> ASN1Certs = certs_from_list(ASN1CertList), ACLen = erlang:iolist_size(ASN1Certs), {?CERTIFICATE, <>}; enc_hs(#server_key_exchange{params = #server_dh_params{ dh_p = P, dh_g = G, dh_y = Y}, - signed_params = SignedParams}, _Version, _) -> + signed_params = SignedParams}, _Version) -> PLen = byte_size(P), GLen = byte_size(G), YLen = byte_size(Y), @@ -934,21 +930,21 @@ enc_hs(#server_key_exchange{params = #server_dh_params{ }; enc_hs(#certificate_request{certificate_types = CertTypes, certificate_authorities = CertAuths}, - _Version, _) -> + _Version) -> CertTypesLen = byte_size(CertTypes), CertAuthsLen = byte_size(CertAuths), {?CERTIFICATE_REQUEST, <> }; -enc_hs(#server_hello_done{}, _Version, _) -> +enc_hs(#server_hello_done{}, _Version) -> {?SERVER_HELLO_DONE, <<>>}; -enc_hs(#client_key_exchange{exchange_keys = ExchangeKeys}, Version, _) -> +enc_hs(#client_key_exchange{exchange_keys = ExchangeKeys}, Version) -> {?CLIENT_KEY_EXCHANGE, enc_cke(ExchangeKeys, Version)}; -enc_hs(#certificate_verify{signature = BinSig}, _, _) -> +enc_hs(#certificate_verify{signature = BinSig}, _) -> EncSig = enc_bin_sig(BinSig), {?CERTIFICATE_VERIFY, EncSig}; -enc_hs(#finished{verify_data = VerifyData}, _Version, _) -> +enc_hs(#finished{verify_data = VerifyData}, _Version) -> {?FINISHED, VerifyData}. enc_cke(#encrypted_premaster_secret{premaster_secret = PKEPMS},{3, 0}) -> @@ -1107,15 +1103,6 @@ server_key_exchange_hash(Algorithm, Value) when Algorithm == rsa; server_key_exchange_hash(dhe_dss, Value) -> crypto:sha(Value). -sig_alg(dh_anon) -> - ?SIGNATURE_ANONYMOUS; -sig_alg(Alg) when Alg == dhe_rsa; Alg == rsa -> - ?SIGNATURE_RSA; -sig_alg(dhe_dss) -> - ?SIGNATURE_DSA; -sig_alg(_) -> - ?NULL. - key_exchange_alg(rsa) -> ?KEY_EXCHANGE_RSA; key_exchange_alg(Alg) when Alg == dhe_rsa; Alg == dhe_dss; @@ -1133,3 +1120,8 @@ apply_user_fun(Fun, OtpCert, ExtensionOrError, UserState0, SslState) -> {unknown, UserState} -> {unknown, {SslState, UserState}} end. + +alg_oid(#'RSAPrivateKey'{}) -> + ?'rsaEncryption'; +alg_oid(#'DSAPrivateKey'{}) -> + ?'id-dsa'. diff --git a/lib/ssl/src/ssl_ssl3.erl b/lib/ssl/src/ssl_ssl3.erl index 1add203fb0..f3cb6ad66e 100644 --- a/lib/ssl/src/ssl_ssl3.erl +++ b/lib/ssl/src/ssl_ssl3.erl @@ -79,10 +79,9 @@ finished(Role, MasterSecret, {MD5Hash, SHAHash}) -> SHA = handshake_hash(?SHA, MasterSecret, Sender, SHAHash), <>. --spec certificate_verify(key_algo(), binary(), {binary(), binary()}) -> binary(). +-spec certificate_verify(OID::tuple(), binary(), {binary(), binary()}) -> binary(). -certificate_verify(Algorithm, MasterSecret, {MD5Hash, SHAHash}) - when Algorithm == rsa; Algorithm == dhe_rsa -> +certificate_verify(?'rsaEncryption', MasterSecret, {MD5Hash, SHAHash}) -> %% md5_hash %% MD5(master_secret + pad_2 + %% MD5(handshake_messages + master_secret + pad_1)); @@ -94,7 +93,7 @@ certificate_verify(Algorithm, MasterSecret, {MD5Hash, SHAHash}) SHA = handshake_hash(?SHA, MasterSecret, undefined, SHAHash), <>; -certificate_verify(dhe_dss, MasterSecret, {_, SHAHash}) -> +certificate_verify(?'id-dsa', MasterSecret, {_, SHAHash}) -> %% sha_hash %% SHA(master_secret + pad_2 + %% SHA(handshake_messages + master_secret + pad_1)); diff --git a/lib/ssl/src/ssl_tls1.erl b/lib/ssl/src/ssl_tls1.erl index d1bc0730ba..dd66418dd8 100644 --- a/lib/ssl/src/ssl_tls1.erl +++ b/lib/ssl/src/ssl_tls1.erl @@ -60,15 +60,14 @@ finished(Role, MasterSecret, {MD5Hash, SHAHash}) -> SHA = hash_final(?SHA, SHAHash), prf(MasterSecret, finished_label(Role), [MD5, SHA], 12). --spec certificate_verify(key_algo(), {binary(), binary()}) -> binary(). +-spec certificate_verify(OID::tuple(), {binary(), binary()}) -> binary(). -certificate_verify(Algorithm, {MD5Hash, SHAHash}) when Algorithm == rsa; - Algorithm == dhe_rsa -> +certificate_verify(?'rsaEncryption', {MD5Hash, SHAHash}) -> MD5 = hash_final(?MD5, MD5Hash), SHA = hash_final(?SHA, SHAHash), <>; -certificate_verify(dhe_dss, {_, SHAHash}) -> +certificate_verify(?'id-dsa', {_, SHAHash}) -> hash_final(?SHA, SHAHash). -spec setup_keys(binary(), binary(), binary(), integer(), -- cgit v1.2.3