Closed Bug 730663 Opened 8 years ago Closed 8 years ago

Port |Bug 708690 - Signed MAR files do not protect against applying an update for the wrong product| to SeaMonkey

Categories

(SeaMonkey :: Build Config, defect, P3)

defect

Tracking

(firefox-esr10 unaffected, seamonkey2.9 wontfix, seamonkey2.10 wontfix, seamonkey2.11 wontfix)

RESOLVED FIXED
seamonkey2.12
Tracking Status
firefox-esr10 --- unaffected
seamonkey2.9 --- wontfix
seamonkey2.10 --- wontfix
seamonkey2.11 --- wontfix

People

(Reporter: sgautherie, Assigned: sgautherie)

References

Details

(Keywords: sec-moderate, Whiteboard: [sg:moderate])

Attachments

(1 file, 2 obsolete files)

No description provided.
Ftr,
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1330242947.1330245869.25819.gz&fulltext=1
OS X 10.6 comm-central-trunk debug test xpcshell on 2012/02/25 23:55:47
{
TEST-PASS | /builds/slave/test/build/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0200_app_launch_apply_update.js | test passed (time: 4201.133ms)
}
still passes.
(Brian, that is expected, isn't it?)
Attachment #600754 - Flags: review?(bugspam.Callek)
Attachment #600754 - Flags: feedback?(netzen)
Comment on attachment 600754 [details] [diff] [review]
(Av1) Fill and package update-settings.ini

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

::: suite/confvars.sh
@@ +49,5 @@
>  MOZ_BRANDING_DIRECTORY=suite/branding/nightly
>  MOZ_OFFICIAL_BRANDING_DIRECTORY=suite/branding/nightly
>  MOZ_EXTENSIONS_DEFAULT=" venkman inspector irc gnomevfs"
>  MOZ_UPDATER=1
> +MAR_CHANNEL_ID=seamonkey-comm-central

This will encode your MAR files with that product/channel name.

So that the updater will accept that particular channel you'll have to also add this into confvars.sh:
ACCEPTED_MAR_CHANNEL_IDS=seamonkey-comm-central

also in toolkit/mozapps/update/updater/Makefile.in there is:


> ifneq (,$(filter beta release esr,$(MOZ_UPDATE_CHANNEL)))
> RCFLAGS += -DMAR_SIGNING_RELEASE_BETA=1
> else
> ifneq (,$(filter nightly aurora nightly-elm nightly-profiling nightly-oak,$(MOZ_UPDATE_CHANNEL)))
> RCFLAGS += -DMAR_SIGNING_AURORA_NIGHTLY=1
> endif
> endif

This determines what key will be used to verify your product signed MARs.  I checked with bhearsum and he said that looked good but just wanted to mention it here in case you want changes.
Attachment #600754 - Flags: feedback?(netzen)
About the last bit relating to the makefile, that is only for signed MAR files. I'm not sure if you want to work on that in the same bug.  If so in confvars.sh you also need MOZ_VERIFY_MAR_SIGNATURE=1 to enable MAR verifying inside updater code.
Not that SeaMonkey even _can_ sign MARs right now, as the project has neither signing infrastructure nor a key for doing the signing.
OK so basically these are the only important parts from my above messages:

::: suite/confvars.sh
@@ +49,5 @@
>  MOZ_BRANDING_DIRECTORY=suite/branding/nightly
>  MOZ_OFFICIAL_BRANDING_DIRECTORY=suite/branding/nightly
>  MOZ_EXTENSIONS_DEFAULT=" venkman inspector irc gnomevfs"
>  MOZ_UPDATER=1
> +MAR_CHANNEL_ID=seamonkey-comm-central

This will encode your MAR files with that product/channel name.

So that the updater will accept that particular channel you'll have to also add this into confvars.sh:
ACCEPTED_MAR_CHANNEL_IDS=seamonkey-comm-central
(In reply to Brian R. Bondy [:bbondy] from comment #6)
> add this into confvars.sh:
> ACCEPTED_MAR_CHANNEL_IDS=seamonkey-comm-central

"You are not authorized to access bug #721758."
Can you CC me to it too?
Yup, done.
Av1, with comment 6 suggestion(s).
Attachment #600754 - Attachment is obsolete: true
Attachment #600767 - Flags: review?(bugspam.Callek)
Attachment #600754 - Flags: review?(bugspam.Callek)
Depends on: 721758
Whiteboard: [sg:moderate]
Attachment #600767 - Flags: review?(kairo)
Ping for review.
(In reply to Serge Gautherie (:sgautherie) from comment #10)
> Ping for review.

At least in my case, this is a waste of time, I'm marking such things TODO in my mailbox, but I can only work on things like this in my free time, which is pretty sparse since I started working full-time for something other than SeaMonkey. I will get to it, no worries. Also, people are reminded weekly by Bugzilla nowadays about their outstanding review requests anyhow, no need to send bugmail in addition.
Comment on attachment 600767 [details] [diff] [review]
(Av2) Fill and package update-settings.ini

OK, this looks good to me by code inspection. I'm not removing the request for Callek though as once this is deployed we need to make sure that updates still work, and he should be in the loop for that.
Attachment #600767 - Flags: review?(kairo) → review+
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #12)
> for Callek though as once this is deployed we need to make sure that updates
> still work, and he should be in the loop for that.

Yes, please don't land this right now, I want to wait until after uplift and devote at least a few hours to researching what all these values specifically mean for us. Then once deployed I want to test!

Once I test and its baked for a few days, we can probably (without pain) deploy it to aurora.
Target Milestone: seamonkey2.10 → seamonkey2.11
Depends on: 732480
Av2, plus bug 732480.
Attachment #600767 - Attachment is obsolete: true
Attachment #609125 - Flags: review?(kairo)
Attachment #609125 - Flags: review?(bugspam.Callek)
Attachment #600767 - Flags: review?(bugspam.Callek)
Comment on attachment 609125 [details] [diff] [review]
(Av3) Fill and package update-settings.ini
[Checked in: See comment 17]

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

r=me but please let Callek OK the landing of this and the used "channel ID" as he and ewong will need to deal with any of this on the SeaMonkey releng side.
Attachment #609125 - Flags: review?(kairo) → review+
Comment on attachment 609125 [details] [diff] [review]
(Av3) Fill and package update-settings.ini
[Checked in: See comment 17]

Ok, so I just had a brief meeting with bbondy to figure out this and what it would do/affect for us.

I'd love to do this, but *currently* it would be a noop for us, and it would be additional complexity for the train-day changes. So for the *time being* consider this r+ for Gecko 15 NOT Gecko 14 (as in, please land after tuesday)

This is currently no-op because its checked against the "is the mar signed" define. http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#1577 We can probably change the define for the part of the code that uses this, but I would say we need to do that in a separate bug.

If that change is accepted for any branches *or* if we start signing mars on our branches before Gecko 15 is out, we can uplift this change at that point.

Sorry for the delay in reviewing, but I am glad I could review correctly.
Attachment #609125 - Flags: review?(bugspam.Callek) → review+
Target Milestone: seamonkey2.11 → Future
Comment on attachment 609125 [details] [diff] [review]
(Av3) Fill and package update-settings.ini
[Checked in: See comment 17]

http://hg.mozilla.org/comm-central/rev/5030dc66c861
Av3, unbitrotted.
Attachment #609125 - Attachment description: (Av3) Fill and package update-settings.ini → (Av3) Fill and package update-settings.ini [Checked in: See comment 17]
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: Future → seamonkey2.12
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.