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

RESOLVED FIXED in seamonkey2.12

Status

P3
normal
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

({sec-moderate})

Trunk
seamonkey2.12
sec-moderate
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

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

Details

(Whiteboard: [sg:moderate])

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

7 years ago
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?)
(Assignee)

Comment 2

7 years ago
Created attachment 600754 [details] [diff] [review]
(Av1) Fill and package update-settings.ini
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.

Comment 5

7 years ago
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
(Assignee)

Comment 7

7 years ago
(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.
(Assignee)

Comment 9

7 years ago
Created attachment 600767 [details] [diff] [review]
(Av2) Fill and package update-settings.ini

Av1, with comment 6 suggestion(s).
Attachment #600754 - Attachment is obsolete: true
Attachment #600767 - Flags: review?(bugspam.Callek)
Attachment #600754 - Flags: review?(bugspam.Callek)
(Assignee)

Updated

7 years ago
Depends on: 721758
Whiteboard: [sg:moderate]
(Assignee)

Updated

7 years ago
status-seamonkey2.9: --- → affected
tracking-seamonkey2.9: --- → ?
(Assignee)

Updated

7 years ago
Attachment #600767 - Flags: review?(kairo)
(Assignee)

Comment 10

7 years ago
Ping for review.

Comment 11

7 years ago
(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 12

7 years ago
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.
(Assignee)

Updated

7 years ago
status-seamonkey2.10: --- → affected
Target Milestone: seamonkey2.10 → seamonkey2.11
(Assignee)

Updated

7 years ago
Depends on: 732480
(Assignee)

Comment 14

7 years ago
Created attachment 609125 [details] [diff] [review]
(Av3) Fill and package update-settings.ini
[Checked in: See comment 17]

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 15

7 years ago
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+
(Assignee)

Updated

7 years ago
status-seamonkey2.10: affected → wontfix
status-seamonkey2.11: --- → wontfix
status-seamonkey2.9: affected → wontfix
tracking-seamonkey2.9: ? → ---
Target Milestone: seamonkey2.11 → Future
(Assignee)

Comment 17

7 years ago
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]
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: Future → seamonkey2.12
status-firefox-esr10: --- → unaffected

Updated

3 years ago
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.