Provide more information in certificate validation failures and other exceptions

[lf-]

Hi! We have been debugging an issue in production where a certificate validation fails on a connection. It would be really helpful to be able to have the contents of the offending certificate in the logs as PEM or so, so that we don't have to reproduce the problem to be able to get any more ideas as to why the validation is failing.

Currently what the library does is this:

InternalException (HandshakeFailed (Error_Protocol “certificate has unknown CA” UnknownCa))

This is generated by this code, which is called by other code that does have the certificate in question. However, the error types don't have more than the TLS RFC's error code number in them. This is reasonable enough and ensures errors are nice and consistent with other libraries, but it does mean that it's hard to add contextual metadata for specific error types:

hs-tls/tls/Network/TLS/Handshake/Certificate.hs

Line 24 in 4cdbf9a

throwCore $ Error_Protocol "certificate has unknown CA" UnknownCa

I would like to fix this issue so that future issues with TLS connections can return richer diagnostics when they are reported. It seems like it might be reasonable to implement a way of adding flexible contextual information to exceptions where available. Currently it seems that TLSException is intended to store some idea of what state the connection was in when the exception occurred, but the state of the connection is not the only meaningful information that could be reported; the contents of failing certificates are just one example of contextual information that could be meaningfully available. Also, changing TLSException to add more context is an API break, and it seems prudent to use some kind of solution here that allows for improving errors without breaking API for each improvement.

One possible approach API-wise is to do something similar to annotated-exception where exceptions have a list of Show e, Typeable e => e existentials which can then be either printed or pulled out in a structured manner, while not having strict constraints of what must be present or not.

In particular, my idea is to extend TLSException with such an annotations list on variants where there is a TLSError, though it could also be reasonable to do it inside of TLSError itself. This could, I think, be done without breaking API by using PatternSynonyms.

Alternatively, it would be possible to just depend on annotated-exception and change things to throw AnnotatedException containing TLSException (which also would not be an API break due to the compatible fromException of AnnotatedException).

Would this approach be acceptable? I could schedule implementing this at work in the next few weeks if it's workable.

cc @parsonsmatt

[parsonsmatt]

I think this would be an API breaking change: the compatibility baked into the AnnotatedException type is that fromException @(AnnotatedException e) can catch a thrown e or a thrown AnnotatedException e. annotated-exception provides a special catch that allows it to catch an e when an AnnotatedException e is thrown.

With GHC 9.10, SomeException is getting exception contexts, so this can be implemented with base starting with recent GHC. But this doesn't help GHC prior to 9.10.

[kazu-yamamoto]

The direction seems good to me.
Real users should change this library as they like.

I don't want to increase dependencies.
But if it is not avoidable, I can accept it.

If breaking changes are not avoidable, let's bump the major version.

[kazu-yamamoto]

Additional note:

• If the features of GHC 9.x are required, it's OK to introduce CPP + #ifdef
• If you are satisfied with concatenating an informative string to the first argument of Error_Protocol, that's also OK.