1466683 - autograph: debug "this addon appears to be corrupt" errors for addons with COSE signatures

Status: RESOLVED FIXED

(Cloud Services :: Security, enhancement)

Comment 1

[Greg G]

Attachment: autograph-30a60cb4-stage-tomato-clock-PKCS7-ES256-ES384.zip

Comment 2

[Greg G]

Attachment: autograph-30a60cb4-prod-tomato-clock-PKCS7-ES256-PS256.zip

Comment 3

[Greg G]

Attachment: autograph-30a60cb4-prod-tomato-clock-PKCS7-ES256.zip

Comment 4

[Greg G]

Attachment: autograph-30a60cb4-stage-tomato-clock-PKCS7.zip

Comment 5

[Greg G]

Attachment: autograph-30a60cb4-prod-tomato-clock-PKCS7.zip

Comment 6

[Greg G]

Attachment: autograph-30a60cb4-stage-tomato-clock-PKCS7-ES256.zip

Comment 7

[Greg G]

Attachment: autograph-30a60cb4-prod-tomato-clock-PKCS7-ES256-ES384.zip

Comment 8

[Greg G]

Attachment: autograph-30a60cb4-stage-tomato-clock-PKCS7-ES256-PS256.zip

Comment 9

[Greg G]

Attachments generated with https://github.com/mozilla-services/autograph/blob/30a60cb46504fbea2f588f07d93e18223c8924a7/tools/autograph-client/build_test_xpis.sh on 30a60cb4 of https://github.com/mozilla-services/autograph/pull/76 using the stage and prod autograph configs from an unsigned zip built from the tomato-clock source.

XPIs with only the PKCS7 signature install properly on new profiles (Fx Nightly 62.0a1 (2018-06-04) (64-bit) osx with xpinstall.signatures.dev-root=true (bool) for stage), but the XPIs with COSE signatures raise "This add-on could no be installed because it appears to be corrupt".

I think this is related to autograph not providing the correct cert chain.

franziskus: can you reproduce the corruption error? Also, we're reusing the same cert spec for PKCS7 and COSE EE certs, does the COSE verification run additional checks (e.g. on timestamps) on the certs?

Comment 10

[Greg G]

franziskus tracked this down:

> the only difference in the bytes Fx puts into the signature verification and autograph puts in is 0xf6 vs 0x40
> so it should work if you use `nil` here .... https://github.com/mozilla-services/go-cose/blob/master/sign_verify.go#L115

so autograph was signing an empty bstr for the external_aad data instead of nil

Comment 11

[Greg G]

Attachment: tomato-clock-PKCS7-ES256.zip

working stage webextension with cose and pkcs7 signatures

Comment 12

[Franziskus Kiefer [:franziskus]]

The pk7 signature uses SHA1 in the attached zip. Is that intentional? Can we have one with SHA256 for unit tests in Firefox as well?

Comment 13

[Greg G]

Attachment: tomato-clock-PKCS7-SHA256.zip

pk7 with sha256 digest and no COSE signatures
 
from https://github.com/mozilla-services/autograph/pull/76/commits/20772dcd013303014eb32b014879be0b43bc7e20

signed with the autograph stage config

Produces an "addon appears to be corrupt" error in Nightly 63.0a1 (2018-06-29) (64-bit) with the dev-root enabled.

Comment 14

[Greg G]

Attachment: tomato-clock-PKCS7-SHA256-ES256.zip

pk7 with sha256 digest and an ES256 COSE signature
 
from https://github.com/mozilla-services/autograph/pull/76/commits/20772dcd013303014eb32b014879be0b43bc7e20

signed with the autograph stage config

Produces an "addon appears to be corrupt" error in Nightly 63.0a1 (2018-06-29) (64-bit) with the dev-root enabled.

Comment 15

[Greg G]

(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #12)
> The pk7 signature uses SHA1 in the attached zip. Is that intentional? 

It is hardcoded here: https://github.com/mozilla-services/autograph/blob/06ac681623466de13809f08351d15428d9cabe96/signer/xpi/xpi.go#L198-L199

> Can we have one with SHA256 for unit tests in Firefox as well?

Yep, attached one without and one with a COSE signature.

I changed the digest in this commit: https://github.com/mozilla-services/autograph/pull/76/commits/b277d036d0e7bc35bccb042b8cb806c1fbbab1e0

It's entirely possible that the other refactoring in that PR is causing the errors though.


:franziskus Is changing the digest something autograph should support as an option? Or should we do something like only use SHA256 for the pk7 digest when a COSE signature is requested too?

Comment 16

[Franziskus Kiefer [:franziskus]]

I think we shold always use SHA256 (or better ;) for PK7 when doing COSE. There shouldn't be a reason to use SHA1.

Comment 17

[Julien Vehent]

I don't think that's right. If we force SHA256 for PKCS7 then we'll break older versions of Firefox that don't support COSE and only support SHA1.

Comment 18

[Franziskus Kiefer [:franziskus]]

SHA256 support for PK7 landed in 58 so if we only care about supported versions of Firefox that shouldn't be an issue. Not sure where you want to make the cut there.
For testing it would be great to have both.

Comment 19

[Julien Vehent]

> if we only care about supported versions of Firefox

This is a question for add-on product mgmt, but I'd suggest addressing it in a separate bug.

That said, adding a signing request option to sign with SHA256 instead of SHA1 is a good idea.