Improve error message when trying to build_chain

[davidMcneil]

Previously this function always returned the UnknownIssuer error.
This was confussing when the error was the result of another problem
(eg unsupported signature algorithm). build_chain now returns the
last error.

Signed-off-by: David McNeil [email protected]

[briansmith]

This was confusing when the error was the result of another problem
(eg unsupported signature algorithm).

In the specific case where the signature algorithm isn't supported, I do think it might make sense to report a more specific error. I would be interested in seeing a PR that adds some complete tests for that scenerio and better error handling for the "unsupported signature algorithm" case. If the change to the non-test code is very small then I think it would be acceptable.

In all cases, the certificate chain should be valid if the used signature algorithms are acceptable.

Test case 1: End-entity certificate's signature uses a signature algorithm that webpki supports, but which isn't passed in as an input. All other certificates in the chain should use supported signature algorithms that are passed as input. The result should be the "unsupported signature algorithm" error you suggest.

Test case 2: End-entity certificate's signature uses a signature algorithm that webpki doesn't even support, perhaps one that it can't even parse. All other certificates in the chain should use supported signature algorithms that are passed as input. I'm open to what this should return.

Test case 3: The end-entity certificate's signature uses a signature algorithm that webpki supports. The intermediate certificate uses a signature algorithm that webpki supports but which isn't enabled. This should return "unknown issuer."

Test case 4: The end-entity certificate's signature uses a signature algorithm that webpki supports. The intermediate certificate uses a signature algorithm that webpki doesn't even support at all. This should return "unknown issuer."

I understand why people want cases #3 and #4 to return something more specific than "unknown issuer" but in general any more specific error would be misleading.

More generally, webpki is not a certificate chain linting tool. It is designed to give a binary answer "Yes" or "No." The "No" happens to have some more detailed information in it but that's only done when it wouldn't be misleading to include the more detailed information, and only if being more detailed doesn't require any extra error-handling logic.

A change that makes the error handling any more complicated just to report a more detailed answer has to be a huge win in order to be accepted.

[briansmith]

See the comment above.

For my part, when I find some time I will document more clearly the design of webpki in this respect, because I know it surprises a lot of people.

[briansmith]

A change that makes the error handling any more complicated just to report a more detailed answer has to be a huge win in order to be accepted.

To expand on this a bit, until I can expand on it elsewhere: IIRC, in mozilla::pkix we did try to implement slightly fancier logic, like "If every path we tried to build resulted in the same error then return that error." It's really intentional that webpki avoids that kind of logic.

A big difference between webpki and most other certificate chain validation libraries is that webpki tries to do as little as possible to get the right "yes or no" answer, to minimize its negative impact on the size of the TCB. The idea is that webpki should accept every valid chain and reject every invalid chain consistent with any similarly-configured well-implemented library. If somebody wants to get into the details of what's wrong with a certificate chain then a much more elaborate tool should be used, e.g. certlint or something else. That more elaborate tool should live outside the TCB and should probably be heavily sandboxed. (This was also true for mozilla::pkix.)

[briansmith]

I'm going to close this for the reasons I mentioned above. If you disagree, PLMK and I'll reopen it.

[briansmith]

I've reconsidered this. I filed an issue about trying to do something like this: #206.