1188957 - Separate mozilla::pkix::Input/Reader from rest of DER parsing

Status: RESOLVED WONTFIX

(NSS :: Libraries, enhancement)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

1. Change the result of Input and Result functions to a new type that is different from mozilla::pkix::Result, so that they can be more easily used outside of DER parsing.

2. Change the result type of Input and Result functions so that it is more convenient (fewer lines of code) to string multiple calls together.

3. Make it so that Input.h does not include any header files other than standard header files.

4. Refactor tests to take these changes into account.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Separate mozilla::pkix::Input/Reader from rest of DER parsing

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7666549cc057

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8640594 [details] [diff] [review]
Separate mozilla::pkix::Input/Reader from rest of DER parsing

Review of attachment 8640594 [details] [diff] [review]:
-----------------------------------------------------------------

Ok - LGTM. Just the one comment.

::: security/pkix/test/gtest/pkixcert_signature_algorithm_tests.cpp
@@ +83,5 @@
>        return Success;
>      }
>      Input issuerCert;
> +    if (issuerCert.Init(issuerDER->data(), issuerDER->length()) != Input::OK) {
> +      return Result::FATAL_ERROR_INVALID_STATE;

In pkixbuild_tests.cpp, similar failures return Result::ERROR_BAD_DER (unless I'm misunderstanding the context). We should probably be consistent one way or another.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

This is a bug worth fixing but I don't have the time to finish this, so unassigning myself.

Comment 5

[J.C. Jones [:jcj] (he/they)]

Since we moved pkix/input.h into NSS, it has effectively become only an ASN.1 parser anyway. If we end up wanting to use it for other purposes in the future we can reopen this bug.