853776 - Remove packaged app signature verification code from mozilla-aurora due to incompatibility with system NSS

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

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

Attachment: Backout the patch that adds support for the Firefox Marketplace root cert, mozilla-aurora only

We do not need packaged app signature verification support in mozilla-aurora. We only need it for mozilla-central and the mozilla-b2g18 trees. That packaged app signature verification code uses a function that did not get added to the NSS release that mozilla-aurora will eventually ship with. This means that the Firefox/Gecko build is broken when the --use-system-nss option is used. We try to make sure that the --use-system-nss build option works for mozilla-beta and definition for releases. The way I've chosen to resolve this is to simply remove all the JAR signature verification code from mozilla-aurora, and leave just a stub for nsIX509CertDB.openSignedJARFileAsync that always fails.

In particular, I removed all the signed packaged app tests and I removed the support for the Firefox Marketplace root cert.

Comment 1

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

Attachment: Remove the SEC_PKCS7VerifyDetachedSignatureAtTime patch to NSS and replace the code that called it with a stub tha always fails, mozilla-aurora only

Note that the security/nss portion of the patch was created by doing these steps:

  python client.py update_nss NSS_3_14_3_RTM
  cd security/nss
  patch -p 1 < ../patches/bug-832942.patch

That is, NSS was reset to NSS 3.14.3, and then the patch for bug 832942 was applied on top of the stock NSS release. Note that the patch for bug 832942 does not affect compatibility with --use-system-nss.

Comment 2

[Kai Engert [:KaiE:]]

Is this the same code that verifies signatures of XPI addons?

Comment 3

[Alex Keybl [:akeybl]]

Will this need to happen after every m-c->m-a merge?

Comment 4

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

(In reply to Kai Engert (:kaie) from comment #2)
> Is this the same code that verifies signatures of XPI addons?

No. There is an open bug to migrate the old verification logic to this new function (to get the certificate validation off of the main thread), but that hasn't happened yet.

(In reply to Alex Keybl [:akeybl] from comment #3)
> Will this need to happen after every m-c->m-a merge?

No. We're going to add the function to NSS upstream and/or we're going to change Gecko to use a different function so that this won't be an issue.

Comment 5

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

Comment on attachment 728102 [details] [diff] [review]
Backout the patch that adds support for the Firefox Marketplace root cert, mozilla-aurora only

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Linux distros cannot distribute Firefox that links to their system NSS libraries
Testing completed (on m-c, etc.): n/a
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None

Comment 6

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

Comment on attachment 728104 [details] [diff] [review]
Remove the SEC_PKCS7VerifyDetachedSignatureAtTime patch to NSS and replace the code that called it with a stub tha always fails, mozilla-aurora only

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Linux distros cannot distribute Firefox that links against their system NSS
Testing completed (on m-c, etc.): none (mozilla-aurora only patch)
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none

Comment 7

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/7ba9d5f19980
https://hg.mozilla.org/releases/mozilla-aurora/rev/e39e3947febb

Thanks for the review, Honza.

NOTE that this will NOT land on any branches other than aurora, so marking it fixed.

Comment 8

[Mike Hommey [:glandium]]

Shouldn't that land on the now aurora 22?

Comment 9

[Hussam Al-Tayeb]

still broken for me on mozilla-central.

Comment 10

[Hussam Al-Tayeb]

clang++ -o nsCertOverrideService.o -c -I../../../../dist/stl_wrappers -I../../../../dist/system_wrappers -include /home/hussam/packages/firefox/src/firefox/config/gcc_hidden.h -DNSS_ENABLE_ECC -DDLL_PREFIX=\"lib\" -DDLL_SUFFIX=\".so\"  -DMOZ_GLUE_IN_PROGRAM -DXPCOM_TRANSLATE_NSGM_ENTRY_POINT=1 -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DSTATIC_EXPORTABLE_JS_API -DNO_NSPR_10_SUPPORT  -I/home/hussam/packages/firefox/src/firefox/security/manager/ssl/src -I. -I../../../../dist/include  -I/usr/include/nspr -I/usr/include/nss      -fPIC -Qunused-arguments  -Qunused-arguments -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wsign-compare -Wno-invalid-offsetof -Wno-c++0x-extensions -Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-mismatched-tags -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector --param=ssp-buffer-size=4 -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -I/home/hussam/packages/firefox/src/firefox/build/unix/headers -pthread -pipe  -DNDEBUG -DTRIMMED -g -Os -freorder-blocks  -fomit-frame-pointer  -Qunused-arguments  -DMOZILLA_CLIENT -include ../../../../mozilla-config.h -MD -MF .deps/nsCertOverrideService.o.pp  /home/hussam/packages/firefox/src/firefox/security/manager/ssl/src/nsCertOverrideService.cpp
nsRecentBadCerts.cpp
/home/hussam/packages/firefox/src/firefox/security/manager/ssl/src/JARSignatureVerification.cpp:589:8: error: use of
      undeclared identifier 'SEC_PKCS7VerifyDetachedSignatureAtTime'
  if (!SEC_PKCS7VerifyDetachedSignatureAtTime(p7_info, certUsageObjectSigner,
       ^

Comment 11

[Hussam Al-Tayeb]

it stops building there. (sorry for the message spam).

Comment 12

[Kai Engert [:KaiE:]]

Dear Hussam, I'm sorry, but at this time this is expected.

I just wrote an explanation of the situation and sent it to the dev-tech-crypto list, see
http://mozilla.6506.n7.nabble.com/Clarification-regarding-SEC-PKCS7VerifyDetachedSignatureAtTime-td276689.html

Comment 13

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/52a2c598a879
https://hg.mozilla.org/releases/mozilla-aurora/rev/23ea2714ce30

OK, this bug is done now. The NSS that is in mozilla-central has the new function.