Closed Bug 1441894 Opened 6 years ago Closed 3 years ago

Snap apparmor denial binding to dbus

Categories

(Release Engineering :: Release Automation: Snap, defect)

Product:
Release Engineering
Component:
Release Automation: Snap
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

(firefox59 fix-optional, firefox60 affected)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox59 --- fix-optional
firefox60 --- affected

People

(Reporter: ken.vandine, Assigned: ken.vandine)

References

(Blocks 1 open bug)

Details

Attachments

(1 obsolete file) 

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/65.0.3325.88 Chrome/65.0.3325.88 Safari/537.36

Steps to reproduce:

Running the snap generates these errors:

apparmor="DENIED" operation="dbus_bind"  bus="session" name="org.mozilla.firefox.V29yaw__" 
mask="bind" pid=25893 label="snap.firefox.firefox". It should use the dbus interface slot to bind to this path
Assignee: nobody → ken.vandine
Blocks: snappy
See Also: → 1441822, 1441884
See Also: → 1441900
We can use the dbus interface to allow the snap to bind to a well known dbus name.  Firefox is appending a seemingly random string to the name.  In this example it's "org.mozilla.firefox.V29yaw__".  Can anyone help answer why this needed?
I couldn't find who's may be in charge of GTK's DBus. Would you have an idea, Mike?
Flags: needinfo?(mozilla)
Martin Stransky. And it appears it's this bug?

https://bugzilla.mozilla.org/show_bug.cgi?id=1418770
Flags: needinfo?(mozilla) → needinfo?(stransky)
Blocks: 1418770
(In reply to Mike Kaply [:mkaply] from comment #3)
> Martin Stransky. And it appears it's this bug?
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1418770

That shouldn't be related.  It fails to bind to the interface silently right now.  We can tell snap it's allowed to bind to the name, but I think it needs a well-known name, like org.mozilla.firefox.  I'm asking our security team if there's a way to allow a wildcard like "org.mozilla.firefox.*".
I'm confused. Is this a new problem? I'm wondering why it wasn't found before.

There were multiple changes related to dbus:

https://hg.mozilla.org/mozilla-central/log/tip/widget/xremoteclient/DBusRemoteClient.cpp

This particular change (appending the profile directory) is about allowing multiple copies of Firefox to run at the same time.
I'm trying to figure out what exactly isn't working.  Currently apparmor is denying the request to bind to the dbus name.  So effectively the DBusRemoteClient stuff is just failing silently.  Everything seems to behave just fine, even when launching a second instance with a second profile.  They both seem to work fine, but I could be missing something.  I guess it's just gracefully falling back as if there is no dbus session available.  

Can you describe what functionality we lose by not having the dbus binding?
(In reply to Ken VanDine from comment #1)
> We can use the dbus interface to allow the snap to bind to a well known dbus
> name.  Firefox is appending a seemingly random string to the name.  In this
> example it's "org.mozilla.firefox.V29yaw__".  Can anyone help answer why
> this needed?

The appendix "V29yaw__" is encoded (due to DBus limitations) Firefox profile - you can have more than one active Firefox instance running.

The DBus Remote service is actively utilized by Wayland backend only - we may add an option to disable it when running under snap.
Flags: needinfo?(stransky)
(In reply to Martin Stránský [:stransky] from comment #7)
> (In reply to Ken VanDine from comment #1)
> > We can use the dbus interface to allow the snap to bind to a well known dbus
> > name.  Firefox is appending a seemingly random string to the name.  In this
> > example it's "org.mozilla.firefox.V29yaw__".  Can anyone help answer why
> > this needed?
> 
> The appendix "V29yaw__" is encoded (due to DBus limitations) Firefox profile
> - you can have more than one active Firefox instance running.
> 
> The DBus Remote service is actively utilized by Wayland backend only - we
> may add an option to disable it when running under snap.

Our security team suggested perhaps you could use the same well-known dbus name and differentiate the profiles with different paths.  That way we could still safely confine the snap and still allowing access to bind to the dbus name.

Thoughts?
(In reply to Ken VanDine from comment #8)
> 
> Our security team suggested perhaps you could use the same well-known dbus
> name and differentiate the profiles with different paths.  That way we could
> still safely confine the snap and still allowing access to bind to the dbus
> name.
> 
> Thoughts?

Well, I can review such patch if you write one.
(In reply to Martin Stránský [:stransky] from comment #9)
> (In reply to Ken VanDine from comment #8)
> > 
> > Our security team suggested perhaps you could use the same well-known dbus
> > name and differentiate the profiles with different paths.  That way we could
> > still safely confine the snap and still allowing access to bind to the dbus
> > name.
> > 
> > Thoughts?
> 
> Well, I can review such patch if you write one.

Great, we'll work on that.
See Also: → 1442419
See Also: → 1441920
I just confirmed with Canonical this bug is minor and has no user impact. They are okay to release 59.0 without it fixed.
Severity: normal → minor
status-firefox59: affected → fix-optional
Component: Release Automation: Other → Release Automation: Snap
See Also: → 1623696

(In reply to Ken VanDine from comment #8)

Our security team suggested perhaps you could use the same well-known dbus
name and differentiate the profiles with different paths. That way we could
still safely confine the snap and still allowing access to bind to the dbus
name.

I'm not sure how that would work: each firefox instance is tied to a given profile, and we can't have two instances claim the same well-known bus name, can we?

Related discussion on the snapcraft forum.

Will that change make it into snapd in time for 89 or should we make a local change?

According to their roadmap, snapd 2.50 is scheduled for a stable release on May 12th, so it seems very unlikely that the change that I'm proposing would be accepted into the release branch before then.

What sort of local change are you thinking about?

Your comment here:

"A temporary band-aid would be to have the firefox snap claim the org.mozilla.firefox.ZGVmYXVsdA__ slot, because this corresponds to the profile named “default”. This would at least make the most common use case (one single profile with the default name) work:"

Yes, I suppose we can do that until this is properly addressed in snapd.

Good news, my snapd patch was merged and cherry-picked into the 2.50 branch, and I was informed that it will be part of snapd 2.50.1, scheduled for release in ~2weeks.

Good news, my snapd patch was merged and cherry-picked into the 2.50 branch, and I was informed that it will be part of snapd 2.50.1, scheduled for release in ~2weeks.

Awesome! So should we wait to close this until we can test with the new snapd?

Awesome! So should we wait to close this until we can test with the new snapd?

Yes. Actually, this can be tested now, with snapd from the edge channel (snap refresh snapd --edge).
Just tested here, and this appears to work correctly. So I'm going to close D114774.

Attachment #9221186 - Attachment is obsolete: true

This bug was fixed in snapd and doesn't require further work in firefox, so it can now be closed.

(In reply to Olivier Tilloy from comment #23)

This bug was fixed in snapd and doesn't require further work in firefox, so it can now be closed.

Thanks for the heads-up, closing now.

Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.