Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More clearly log details about SSL errors #259

Open
aantron opened this issue Apr 20, 2023 · 7 comments
Open

More clearly log details about SSL errors #259

aantron opened this issue Apr 20, 2023 · 7 comments

Comments

@aantron
Copy link
Owner

aantron commented Apr 20, 2023

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 #259 (comment). It just needs to be moved forward so that it's actually likely to be seen by a developer or devops user.

@aantron
Copy link
Owner Author

aantron commented Apr 20, 2023

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
Copy link
Owner Author

aantron commented Apr 20, 2023

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.

@amongonz
Copy link

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.

@aantron
Copy link
Owner Author

aantron commented Apr 20, 2023

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 aantron removed this from the alpha6 milestone Apr 20, 2023
@aantron
Copy link
Owner Author

aantron commented Apr 20, 2023

@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
Copy link
Owner Author

aantron commented Apr 20, 2023

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 😆

@amongonz
Copy link

@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.

@aantron aantron changed the title Log details about SSL errors More clearly log details about SSL errors Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants