Addition of signon-apparmor-extension causes token lookup problems

Bug #1376445 reported by dobey
268
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Canonical System Image
Fix Released
High
David Barth
Online Accounts setup for Ubuntu Touch
Fix Released
High
Alberto Mardegan
The Savilerow project
Invalid
Undecided
Unassigned
go-onlineaccounts
Incomplete
High
James Henstridge
signon-apparmor-extension
Fix Released
High
Alberto Mardegan
signon-apparmor-extension (Ubuntu RTM)
Triaged
High
Unassigned
ubuntu-system-settings-online-accounts (Ubuntu)
Fix Released
Undecided
Alberto Mardegan
ubuntu-system-settings-online-accounts (Ubuntu RTM)
New
Undecided
Alberto Mardegan
ubuntuone-credentials (Ubuntu)
Fix Released
High
dobey
ubuntuone-credentials (Ubuntu RTM)
Fix Released
High
dobey

Bug Description

As of image ~264 of ubuntu-touch, the signon-apparmor-extension package is included. As a result, apps like pay-ui cannot find the token any longer, and are not being notified that they are not allowed to access the token. The following error appears in the payui log file:

2014-10-01 19:15:51,550 - DEBUG - ../../../../lib/SignOn/authsessionimpl.cpp 184 errorSlot QDBusError("com.google.code.AccountsSSO.SingleSignOn.Error.PermissionDenied", "Client has insuficient permissions to access the service.Method:getAuthSessionObjectPath")

Related branches

dobey (dobey)
Changed in ubuntuone-credentials (Ubuntu RTM):
status: New → Triaged
importance: Undecided → Critical
Revision history for this message
Chris Wayne (cwayne) wrote :

This is affecting some of our custom scopes that use online accounts as well.

Revision history for this message
Alberto Mardegan (mardy) wrote :

The problem here is with the U1 account plugin, which doesn't add "unconfined" to the initial list of ACL.
After a quick look at the source code, I believe that the code which needs to be changed is here:
http://bazaar.launchpad.net/~ubuntuone-control-tower/ubuntuone-credentials/trunk/view/head:/libubuntuoneauth/keyring.cpp#L161

The change is about adding

    info.setAccessControlList(QStringList() << "unconfined");

before storing the "info" object.

Revision history for this message
Chris Wayne (cwayne) wrote :

@mardy
I'm also seeing this in other account plugins (namely fitbit and flickr). Is there any way to add unconfined to the ACL there?

Revision history for this message
Alberto Mardegan (mardy) wrote :

@cwayne18: all plugins based on our OAuth code already add "unconfined" to the ACL. So the problem is probably different, in that case.

As @jdstrand suggested in the ML, your scopes are not running under the "unconfined" profile (even though they are practically unconfined), so we need to figure out why they are not in the ACL, and how to add them.

As suggested by Rodney, I'll also hack "unconfined" into the signon-apparmor plugin to let processes carrying that profile to access any account. But please remember that this is a temporary hack which I'd eventually like to remove, so please update the U1 plugin anyway.

Changed in signon-apparmor-extension:
assignee: nobody → Alberto Mardegan (mardy)
importance: Undecided → High
status: New → In Progress
Revision history for this message
dobey (dobey) wrote :

Yes, the plug-in will be fixed (that's why I filed the bug here), but according to what Chris is stating, I'm not sure that will be enough to resolve the issue even for people who create new accounts; given the problem is happening with OAuth-based plug-ins that have "unconfined" in the ACL already.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package signon-apparmor-extension - 0.1+14.10.20141002-0ubuntu1

---------------
signon-apparmor-extension (0.1+14.10.20141002-0ubuntu1) 14.09; urgency=low

  [ CI bot ]
  * Resync trunk

  [ Alberto Mardegan ]
  * Let the unconfined profile access any resource (LP: #1376445)
 -- Ubuntu daily release <email address hidden> Thu, 02 Oct 2014 15:48:27 +0000

Changed in signon-apparmor-extension (Ubuntu RTM):
status: New → Fix Released
Alberto Mardegan (mardy)
Changed in signon-apparmor-extension:
status: In Progress → Fix Released
Changed in ubuntuone-credentials (Ubuntu):
status: Triaged → Invalid
Changed in ubuntuone-credentials (Ubuntu RTM):
status: Triaged → Invalid
Changed in savilerow:
status: New → Invalid
Revision history for this message
dobey (dobey) wrote :

Pat, why did you mark this as Invalid in ubuntuone-credentials? The temporary fix in signon-apparmor-extension and temporary removal of that from the image might have lowered the priority of this bug, but I think we still need to make adjustments in ubuntuone-credentials to deal with the ACL appropriately, no?

Revision history for this message
Alberto Mardegan (mardy) wrote :

That was probably my mistake, I misunderstood the scope of the bug and told Pat it could be closed. I'm reopening it for ubuntuone-credentials, and lowering the proprity to "high".

Changed in ubuntuone-credentials (Ubuntu):
status: Invalid → Confirmed
Changed in ubuntuone-credentials (Ubuntu RTM):
status: Invalid → Confirmed
Revision history for this message
Alberto Mardegan (mardy) wrote :

Well, I actually cannot lower the priority.

dobey (dobey)
Changed in ubuntuone-credentials (Ubuntu):
importance: Critical → High
Changed in ubuntuone-credentials (Ubuntu RTM):
importance: Critical → High
dobey (dobey)
tags: added: touch-2014-10-30
removed: touch-2014-10-09
David Barth (dbarth)
Changed in signon-apparmor-extension (Ubuntu RTM):
status: Fix Released → Triaged
importance: Undecided → High
Changed in go-onlineaccounts:
status: New → Triaged
importance: Undecided → High
assignee: nobody → James Henstridge (jamesh)
tags: added: touch-2014-11-27
removed: touch-2014-10-30
tags: added: ota-1
Alberto Mardegan (mardy)
Changed in ubuntu-system-settings-online-accounts (Ubuntu):
status: New → In Progress
assignee: nobody → Alberto Mardegan (mardy)
Changed in ubuntu-system-settings-online-accounts:
assignee: nobody → Alberto Mardegan (mardy)
status: New → In Progress
importance: Undecided → High
Revision history for this message
James Henstridge (jamesh) wrote :

Since the go-onlineaccounts bug task was added, I guess we are also tracking the problems with scopes here too.

In my investigations last week, I came to the conclusion that the problems were not limited to the Go scopes, but instead affected all scopes running under confinement.

After digging in a bit, it isn't clear how it can be fixed without changes to the online accounts API. The way the online accounts integration for scopes works is:

1. the scope starts a signin session for the account service with the "no interaction" flag set. If no token can be retrieved (either because no account is available, or because the token isn't available), it will push a result with a special login button.

2. When the special result is clicked by the user, the dash will initiate the account creation process with the OnlineAccountsClient::Setup class.

3. If the account is successfully created, the dash refreshes the scope's results. The scope starts another signin session and finds the new token and displays personalised results.

From my understanding, this breaks down because it is the dash's AppArmor label (which I guess would be unconfined) that gets added to the ACL for the account service. What would be needed here would be some kind of API the dash could use to ask for a second AppArmor label to be added to the ACL.

Changed in go-onlineaccounts:
status: Triaged → Incomplete
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

FYI, the fix for bug 1392380 should accompany when signon-apparmor-extension hits rtm. Both bugs are currently identically tagged. This must not slip past OTA-1 otherwise there is a gaping security hole for shipping devices.

information type: Public → Public Security
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Since this bug is blocking the landing of signon-apparmor-extension, and the lack of signon-apparmor-extension is a security issue for devices, marking the bug as Public Security.

Alberto Mardegan (mardy)
Changed in ubuntu-system-settings-online-accounts (Ubuntu RTM):
assignee: nobody → Alberto Mardegan (mardy)
Revision history for this message
Pat McGowan (pat-mcgowan) wrote :

Confirming as this security issue was agreed to be fixed in first update

Changed in canonical-devices-system-image:
importance: Undecided → High
milestone: none → ww51-2014
status: New → Confirmed
Revision history for this message
Alberto Mardegan (mardy) wrote :

We really need the U1 plugin to add "unconfined" to the ACL of all newly created accounts.

We recently landed some changes in signond which mitigate this problem, but they only solve the issue for accounts which are created after this fix landed.
I guess I could add a session migration script to signond, to add "unconfined" to the ACL of the old U1 accounts, or maybe a hack to signond to treat "unconfined" specially and let it access any resource.

Alberto Mardegan (mardy)
Changed in ubuntu-system-settings-online-accounts:
status: In Progress → Fix Released
Changed in canonical-devices-system-image:
milestone: ww51-2014 → ww03-2015
Revision history for this message
dobey (dobey) wrote :

I think we need to push this back, as there seems to be a bigger problem here in vivid, than we thought. Even on a newly flashed vivid image, with a newly created u1 account, I am seeing the permissions denied error message with the signon-apparmor-extension installed. I'll have to talk to Alberto more to figure out why this happens there, as the account has the 'unconfined' ACL on it, but unconfined apps seem to have problems reading the token.

Changed in canonical-devices-system-image:
milestone: ww03-2015 → ww05-2015
dobey (dobey)
Changed in ubuntuone-credentials (Ubuntu):
assignee: nobody → Rodney Dawes (dobey)
status: Confirmed → In Progress
Changed in ubuntuone-credentials (Ubuntu RTM):
assignee: nobody → Rodney Dawes (dobey)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntuone-credentials - 14.04+15.04.20150120

---------------
ubuntuone-credentials (14.04+15.04.20150120) vivid; urgency=low

  [ Rodney Dawes ]
  * Set the default ACL as ["unconfined"] for the account. (LP:
    #1376445)
  * Handle keyringError from the keyring, by deleting token. (LP:
    #1282392)
 -- Ubuntu daily release <email address hidden> Tue, 20 Jan 2015 18:09:09 +0000

Changed in ubuntuone-credentials (Ubuntu):
status: In Progress → Fix Released
dobey (dobey)
Changed in pay-ui:
importance: Undecided → Critical
status: New → Triaged
Changed in canonical-devices-system-image:
milestone: ww05-2015 → ww07-2015
status: Confirmed → In Progress
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntuone-credentials - 14.04+15.04.20150122~rtm

---------------
ubuntuone-credentials (14.04+15.04.20150122~rtm) 14.09; urgency=low

  [ Ubuntu daily release ]
  * New rebuild forced

  [ Rodney Dawes ]
  * Handle keyringError from the keyring, by deleting token. (LP:
    #1282392)
  * Set the default ACL as ["unconfined"] for the account. (LP:
    #1376445)

ubuntuone-credentials (14.04+14.10.20140910) utopic; urgency=low

  [ CI bot ]
  * Resync trunk

  [ Rodney Dawes ]
  * Add new ctor for Token to accept created/updated date strings. Use
    the new ctor when creating the token from the REST response. Turn
    the date string returned from the server into an ISO string for
    parsing. Add more tests. (LP: #1366998)

  [ Sebastien Bacher ]
  * Set wrapmode to avoid having a label cut (LP: #1366294)
 -- Ubuntu daily release <email address hidden> Thu, 22 Jan 2015 16:16:46 +0000

Changed in ubuntuone-credentials (Ubuntu RTM):
status: Confirmed → Fix Released
David Barth (dbarth)
Changed in ubuntu-system-settings-online-accounts (Ubuntu):
status: In Progress → Fix Released
Changed in canonical-devices-system-image:
assignee: nobody → David Barth (dbarth)
milestone: ww07-2015 → ww09-2015
Changed in canonical-devices-system-image:
status: In Progress → Fix Released
dobey (dobey)
Changed in pay-ui:
importance: Critical → Undecided
status: Triaged → Invalid
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.