1449060 - remove some OIDs/processing from the certificate viewer that don't affect certificate verification

Status: RESOLVED FIXED

(Core :: Security: PSM, enhancement, P1)

Description

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

We have some dynamically-added OIDs in the certificate viewer code that don't serve any useful purpose. They correspond to some extensions that have no effect on certificate verification (unless they're marked critical, in which case the certificate would be rejected like any other unknown extension). We should just remove them and simplify this code.

Comment 1

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

Attachment: bug 1449060 - remove some dynamic OIDs from the certificate viewer that don't serve any purpose

Review commit: https://reviewboard.mozilla.org/r/231354/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231354/

Comment 2

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8962552 [details]
bug 1449060 - remove some dynamic OIDs from the certificate viewer that don't serve any purpose

https://reviewboard.mozilla.org/r/231354/#review236960

+1 for removing code

::: security/manager/ssl/nsNSSCertHelper.cpp
(Diff revision 1)
>  #include "prerror.h"
>  #include "secder.h"
>  
>  using namespace mozilla;
>  
> -/* Object Identifier constants */

Looking at the bug that added these the code is here so we can display the extensions. I presume we won't be able to display them after this change? But the extensions don't look widespread so that this shouldn't be an issue.

Comment 3

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

Comment on attachment 8962552 [details]
bug 1449060 - remove some dynamic OIDs from the certificate viewer that don't serve any purpose

https://reviewboard.mozilla.org/r/231354/#review236960

> Looking at the bug that added these the code is here so we can display the extensions. I presume we won't be able to display them after this change? But the extensions don't look widespread so that this shouldn't be an issue.

More or less. We won't be able to display them in the specially-formatted way they are now - it'll just be a hexdump, basically. Since they don't affect verification, I don't see a point in supporting them in the certificate viewer.

Comment 4

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

Thanks for the review!
Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=deb11cee8b845e0bfb84be22501773d4e30a16cf

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a39db181e6a
remove some dynamic OIDs from the certificate viewer that don't serve any purpose r=fkiefer

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/5a39db181e6a