Skip to content

Commit

Permalink
Send alert messages on handshake failures
Browse files Browse the repository at this point in the history
When SSL_do_handshake() fails we should send data even when
SSL_ERROR_WANT_WRITE is not reported (because a final error
such as "unsupported protocol" is reported).

If we don't do it, we don't send handshake alerts toward the peer and
as a consequence:

- We formally violate RFC 5246, appendix E.1:
  > If server supports (or is willing to use) only versions greater
  > than client_version, it MUST send a "protocol_version" alert message
  > and close the connection.
- We are complicating the debugging of the protocol.
- We don't give a hint to a peer on how to proceed with the error.

This commit fixes it.
  • Loading branch information
Evgeny Khramtsov committed Jan 9, 2024
1 parent 4ae9adf commit f83322c
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 12 deletions.
22 changes: 13 additions & 9 deletions c_src/fast_tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -969,7 +969,7 @@ get_decrypted_data(ErlNifEnv *env, state_t* state, int bytes_to_read, ERL_NIF_TE
}

static ERL_NIF_TERM
return_read_write(ErlNifEnv *env, state_t* state, int bytes_to_read) {
return_read_write(ErlNifEnv *env, state_t* state, int bytes_to_read, ERL_NIF_TERM tag) {
ERL_NIF_TERM read;
if (get_decrypted_data(env, state, bytes_to_read, &read) == 2) {
enif_mutex_unlock(state->mtx);
Expand All @@ -979,7 +979,7 @@ return_read_write(ErlNifEnv *env, state_t* state, int bytes_to_read) {

enif_mutex_unlock(state->mtx);

return enif_make_tuple2(env, write, read);
return enif_make_tuple3(env, tag, write, read);
}

static int
Expand Down Expand Up @@ -1095,9 +1095,12 @@ loop_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) {
if (res == 2) {
return err_term;
}
return return_read_write(env, state, bytes_to_read);
return return_read_write(env, state, bytes_to_read, enif_make_atom(env, "ok"));
} else {
enif_mutex_unlock(state->mtx);
res = do_send_queue(env, state, &err_term, &to_send);
if (res == 2) {
return err_term;
}
int reason = ERR_GET_REASON(ERR_peek_error());
if (reason == SSL_R_DATA_LENGTH_TOO_LONG ||
reason == SSL_R_PACKET_LENGTH_TOO_LONG ||
Expand All @@ -1107,19 +1110,20 @@ loop_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) {
reason == SSL_R_HTTP_REQUEST ||
reason == SSL_R_HTTPS_PROXY_REQUEST)
/* Do not report badly formed Client Hello */
return ERR_T(enif_make_atom(env, "closed"));
err_term = ERR_T(enif_make_atom(env, "closed"));
else if (state->sni_error)
return ssl_error(env, state->sni_error);
err_term = ssl_error(env, state->sni_error);
else
return ssl_error(env, "SSL_do_handshake failed");
err_term = ssl_error(env, "SSL_do_handshake failed");
return return_read_write(env, state, bytes_to_read, err_term);
}
}
if (!SSL_is_init_finished(state->ssl)) {
res = do_send_queue(env, state, &err_term, &to_send);
if (res == 2) {
return err_term;
}
return return_read_write(env, state, bytes_to_read);
return return_read_write(env, state, bytes_to_read, enif_make_atom(env, "ok"));
}
}

Expand All @@ -1130,7 +1134,7 @@ loop_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) {

if (res <= 0)
res = SSL_get_error(state->ssl, res);
return return_read_write(env, state, bytes_to_read);
return return_read_write(env, state, bytes_to_read, enif_make_atom(env, "ok"));
}

static ERL_NIF_TERM get_verify_result_nif(ErlNifEnv *env, int argc,
Expand Down
9 changes: 6 additions & 3 deletions src/fast_tls.erl
Original file line number Diff line number Diff line change
Expand Up @@ -232,16 +232,19 @@ loop(#tlssock{tcpsock = TCPSocket,
try loop_nif(Port, ToSend, Received, Length) of
{error, _} = Err ->
Err;
{<<>>, Decrypted} ->
{ok, <<>>, Decrypted} ->
{ok, <<DecBuf/binary, Decrypted/binary>>};
{ToWrite, Decrypted} ->
{ok, ToWrite, Decrypted} ->
case gen_tcp:send(TCPSocket, ToWrite) of
ok ->
loop(Socket, <<>>, <<>>, <<DecBuf/binary, Decrypted/binary>>,
Length - byte_size(Decrypted));
{error, _} = Err ->
Err
end
end;
{{error, _} = Err, ToWrite, _} ->
_ = gen_tcp:send(TCPSocket, ToWrite),
Err
catch error:badarg ->
{error, einval}
end.
Expand Down

0 comments on commit f83322c

Please sign in to comment.