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

Errors Contain More Information #183

Closed
Mythra opened this issue Feb 15, 2021 · 3 comments
Closed

Errors Contain More Information #183

Mythra opened this issue Feb 15, 2021 · 3 comments

Comments

@Mythra
Copy link

Mythra commented Feb 15, 2021

Hey! Thanks for all your work on this crate.

tl;dr I'd like error enum to have field variants (some would need to be behind #[cfg(feature = "alloc")]) that report more detailed information.

I'm consuming webpki for a command line tool, and I was looking to provide some high quality error messages to the user, unfortunately I ran into a bit of a snag when dealing with TLS because I simply just don't have enough information to report to the user.

For a concrete example of what I'd like to see I'll show two example scenarios I've personally ran into:

  1. Reporting to a user when a certificate is expired. Right now the enum variant just says CertExpired, but there's no way to derive when the certificate did start expiring. Is it a few minutes ago, and I just need to let someone know who runs the site, or is it an incredibly defunct site that hasn't been renewed in years, and I need to think of some other info as I probably won't get a response?
  2. Reporting hostnames not covered by a certificate. When a certificate isn't covering the host you're trying to reach it at, what hosts does the certificate cover? Is it perhaps an old domain that you missed messages about going out of bounds, or is it a setup error?

There are several other cases I'd love to have extra info in too, such as BadDER (which field is invalid? What can I report to whoever runs that server to let them know this is how your certificate is invalid), but I haven't run into them.


I'd be happy to do some of this work myself, but I just wanna make sure that this work is:

  • Worth doing from your point of view, and not just wasting time.
  • Have some sort of general plan, or help (I started taking a look into this, and while some things like cert expirey where easy (just return a u64 of the unix timestamp, in the enum), things like BadDER made me very confused very quickly).
@briansmith
Copy link
Owner

tl;dr I'd like error enum to have field variants (some would need to be behind #[cfg(feature = "alloc")]) that report more detailed information.

I'm skeptical about ones that would need to be behind feature = "alloc".

I'm consuming webpki for a command line tool, and I was looking to provide some high quality error messages to the user, unfortunately I ran into a bit of a snag when dealing with TLS because I simply just don't have enough information to report to the user.

Are you aware of the certificate linting projects that are open-source and maintained by people in the CA/browser community? These are the same libraries/tools that many commercial CAs are using to check their certificates and they put a lot of effort into diagnosing certificate problems. You might try integrating with them, though I think they're in Go and/or Ruby.

As far as this webpki crate is concerned, simplicity and portability (to small embedded systems) is valued over the quality of the contents of errors returned. My expectation is that webpki would be used in the core trusted computing base of the systems that use it, but that detailed certificate diagnostics would live outside the TCB, in a highly restricted sandbox.

My guideline--not a hard-and-fast rule--is that I'm likely to accept a PR for extending the error reporting that meets these requirements:

  • Does not add extra checks to the parser in the non-error case
  • is net 1 or less lines of code (often it will be net 0 lines of code)
  • works in no_std environments*
  • has a test
  1. Reporting to a user when a certificate is expired. Right now the enum variant just says CertExpired, but there's no way to derive when the certificate did start expiring. Is it a few minutes ago, and I just need to let someone know who runs the site, or is it an incredibly defunct site that hasn't been renewed in years, and I need to think of some other info as I probably won't get a response?

I think we should probably fix issue #67. Then when your wrapper around webpki sees CertExpired, it can pull the notAfter and notBefore from EndEntityCert and then map that error to whatever error structure you want.

Maybe we could include the times in the CertExpired error itself too, though I'm a little weary of increasing the size of the Error structure. I think we should fix #67 first because the validity period is useful even in non-error cases.

  1. Reporting hostnames not covered by a certificate. When a certificate isn't covering the host you're trying to reach it at, what hosts does the certificate cover? Is it perhaps an old domain that you missed messages about going out of bounds, or is it a setup error?

PR #103 will add an API for collecting the names in a certificate. Again, when you get a name mismatch error, you could then map that error to another error that includes the names in the certificate. I don't think I would add the names in the certificate to the Error structure; the list of names can be very large.

There are several other cases I'd love to have extra info in too, such as BadDER (which field is invalid? What can I report to whoever runs that server to let them know this is how your certificate is invalid), but I haven't run into them.

PR #184 and PR #185 make improvements to this area. If you submit similar PRs (considering the feedback I left on them) then I would probably accept them.

  • Have some sort of general plan, or help (I started taking a look into this, and while some things like cert expirey where easy (just return a u64 of the unix timestamp,

instead we'd probably expose it as webpki::Time

@Mythra
Copy link
Author

Mythra commented Feb 16, 2021

Thanks, it sounds like my use cases are a bit different than what are wanted for this crate. Something I was concerned about. I understand wanting to keep a small trusted codebase though, especially for something as important as TLS.

I'd definitely very much dislike just sending users to another tool just to try and troubleshoot why my particular tool is failing though. "Hey my tool barfed, go check with this other unrelated tool I don't maintain to troubleshoot" isn't the best user experience.

So I'll wait for some of those PRs to merge in you mentioned, and pulling those through hyper, and then see where I stand, and what I can. Thanks again for all the information!

@briansmith
Copy link
Owner

"Hey my tool barfed, go check with this other unrelated tool I don't maintain to troubleshoot"

I agree that's a bad experience. I think the Go tool may be embeddable as a library. Anyway, the last time I looked at those tools, they were very comprehensive in some areas that would be difficult to replicate, so I would recommend trying to "buy" before building if it makes sense at all for you.

I'm going to close this issue. I hope the guidelines above are useful for if/when you contribute improvements in this area. I've come around to agreeing with others that BadDER isn't such a great error. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants