dream icon indicating copy to clipboard operation
dream copied to clipboard

More clearly log details about SSL errors

Open aantron opened this issue 2 years ago • 7 comments

At the moment, SSL errors and warnings server-side look like this in the log:

20.04.23 13:03:35.555      dream.http  WARN TLS (127.0.0.1:41428): SSL accept() error: error:0A000416:SSL routines::sslv3 alert certificate unknown
20.04.23 13:03:35.819      dream.http  WARN TLS (127.0.0.1:41436): SSL accept() error: error:0A000416:SSL routines::sslv3 alert certificate unknown

The number 0A000416 can probably be decoded using the SSL bindings to give the human readers some more-useful information.

EDIT: the information is already in the message, just at the very end -- see https://github.com/aantron/dream/issues/259#issuecomment-1516386879. It just needs to be moved forward so that it's actually likely to be seen by a developer or devops user.

aantron avatar Apr 20 '23 13:04 aantron

For the repro, I was able to just try the HTTPS example in a 5.0.0 switch with all the latest opam packages and Dream master.

aantron avatar Apr 20 '23 13:04 aantron

It looks like the issue here isn't that messages aren't logged -- the above messages end with

alert certificate unknown

the issue is that this appears at the very end of the log message, where it is clipped by Dream's own default terminal settings. It should be moved to the front of the message, and the numeric details can be displayed later. So, depending on the API of the SSL bindings, this might actually be an easy issue.

aantron avatar Apr 20 '23 14:04 aantron

For context, the current format comes from the printer registered in ocaml-ssl. This calls Ssl.get_error_string which binds openssl's ERR_error_string_n(ERR_get_error(), ...). This function is the one providing error:[error code]:[library name]::[reason string] as a blob.

ocaml-ssl doesn't seem have bindings to obtain the error code and call ERR_lib_error_string and ERR_reason_error_string separately, though. I'd say it's either that or parsing the error string we have.

amongonz avatar Apr 20 '23 15:04 amongonz

If these things are available in SSL in C, it seems better (as I'm sure you agree!) to provide bindings to them, rather than parse these strings. So maybe this isn't a good first issue after this point, but it is an upstream issue. Thank you!

aantron avatar Apr 20 '23 15:04 aantron

@devvydeebug would you be willing to open an issue at savonet/ocaml-ssl about this? Maybe they would like to add bindings, or can provide other info.

aantron avatar Apr 20 '23 15:04 aantron

An alternative would be to just rearrange the error output in the printer, though I'm afraid that could cause pain to someone that is parsing the messages 😆

aantron avatar Apr 20 '23 15:04 aantron

@aantron Agreed, parsing is always messy. I'll open an issue there.

Absolute worst case, catching the exception before Printexc.to_string and rearranging the string there is a solution.

amongonz avatar Apr 20 '23 15:04 amongonz