Prompt for credential when it shouldn't

[Sebastien Bacher]

* Impact

Devices should be approved without password prompting for logged in admin users but they are not today because the code looks a 'wheel' as being the admin group.

* Test case
- log into a Ubuntu/GNOME session with an admin user
- connect an USB-C device

the device should be accepted without prompting (the result can be checked with the boltctl command or in the thunderbolt g-c-c section)

- do the same test with a non admin user, it should be prompted for a password

* Regression potentiel

Check that device auth works for admin and non admin users

[Sebastien Bacher (seb128)]

The issue has been fixed in cosmic, the fix is being SRUed to bionic now

[Robie Basak (racb)]

Hello Sebastien, or anyone else affected,

Accepted bolt into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/bolt/0.5-0ubuntu0.18.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Yuan-Chen Cheng (ycheng-twn)]

It's new to me that it's designed in a way that a user in admin/wheel group doesn't need to enter the password.

The user in admin group needs to enter the password when they are doing something serious or potentially harmful to the system. If there could be a potential security risk, why we shouldn't?

No offence, just 2 cents.

[Sebastien Bacher (seb128)]

@Yuan-Chen, you wrote "The user in admin group needs to enter the password when they are doing something serious or potentially harmful to the system" ... we are speaking about an action which happens when the user has physical access to the machine (so he can connect the usb-c device) with an unlocked admin session, what do you think that are the extra safety risk created by authentificating the device?

Note that authentificating an USB device is not a destructive action (not like formatting a disk or removing packages) and can be revoked at any time.

[Mario Limonciello (superm1)]

My personal opinion aligns with YC actually.

It's specifically in the handling of a Thunderbolt device not just any USB device. If a thunderbolt device is automatically authenticated it does improve the usability at the expense of security.

A nefarious Thunderbolt device can trivially perform a DMA attack if automatically authorized in a situation that DMA mitigation such as IOMMU (VT-d) is not used.

Until there is a guarantee of DMA mitigation presence (which is going to be coming in 4.21 and a newer version of bolt) it's much safer to adjust
 to prompt for authorization or open a notification to do such.

I feel if this change is included Canonical's security team should review as well.

[Will Cooke (willcooke)]

If I understand correctly, the attack would have to be on a logged in session. So the attacker would have local access to the machine and the session already.
It is something for careful consideration though. I will loop in the security team for their thoughts.

[Sebastien Bacher (seb128)]

@Mario, I'm a bit confused on why you didn't raise those concerns earlier, you knew about the upstream behaviour and even tested things on 18.10 for https://gitlab.gnome.org/GNOME/gnome-shell/issues/709
It would have been nice to have that discussion upfront and not have it lock the fwupd and bolt SRUs now...

Anyway, Ccing ubuntu-security team to have their input. Note that the "right" admin group use change landed before 18.10 so the behaviour you consider problematic is the default one is 18.10 today.

I still think that if someone has physical access to your machine and an unlocked admin session in front of them then you are already in trouble and DMA attach isn't or not is not going to do much of a difference.

[Sebastien Bacher (seb128)]

(one potential problem is https://gitlab.freedesktop.org/bolt/bolt/issues/95 which would involve a way to crash gnome-shell which wouldn't be uncommon)

[Sebastien Bacher (seb128)]

ignore that previous common, would need to crash bolt itself, which is less likely

[Sebastien Bacher (seb128)]

Also note that the upstream behaviour is described on https://wiki.gnome.org/Design/Whiteboards/ThunderboltAccess , they decided to not show the dialog because they believe that most users would not read/understand the question or the implication and would click "yes" anyway so it wouldn't change the situation much

[Alex Murray (alexmurray)]

The security team consider the existing behaviour is fine - ie. automatically connect without authentication when an admin session is logged in and is an active seat (ie. the screen / session is not switched to some other users sessions / VT), and the screen is unlocked.

If someone has direct physical access to your machine they can achieve a lot already (say for instance they could connect an-inline USB keylogger or similar http://www.keelog.com/) - so I don't see this as any higher risk for TB3. Also agree with @seb128's comments in this regard too.

Finally, I also agree with upstream's rationale that it is not helpful or useful to ask the user to authorize - training users to just click Yes to get things done is not an effective security strategy.

[Betty Lin (bettyl)]

Image: pc-timbuktu-bionic-amd64-X00-20181114-312.iso
Test machine: XPS 13
Thunderbolt 3 device: Dell dock TB16 BME

Test steps:
1. Enable "Security level - User Authentication" of Thunderbolt Adapter Configuration in BIOS
2. Create a user "test-admin" and add in sudo group
3. Reboot and login with test-admin
4. Plug in Dell dock (usb-type c)

Test result:
Authentication required with password still prompting for logged in admin users.

Related information:

test-admin@u-XPS-13-9370:~$ groups
test-admin adm sudo
test-admin@u-XPS-13-9370:~$ dpkg -l | grep bolt
ii bolt 0.5-0ubuntu0.18.04.1 amd64 system daemon to manage thunderbolt 3 devices

[Sebastien Bacher (seb128)]

@Alex, thanks for the input. Mario are you happy with those replies?

@Betty, thanks for testing, weird that it's not working though :/ That said, that's not a regression over the current bionic version and while it would be good to figure out why it's not working it's maybe not worth blocking the SRU.

[Mario Limonciello (superm1)]

@seb128, regarding comment #7:
That is exactly why I raised that bug upstream. The way that makes sense to me is for popping up a GNotification when new devices are plugged in rather than automatically authorizing or automatically "trying" to authorize unless you know the device is "safe".

@alex-murray, regarding comment #11:
There are mitigations that will be included in a future kernel version (Probably 4.21 right now) around turning on the IOMMU and turning off ATS for Thunderbolt devices in safer scenarios on newer machines. That's the right time to automatically authorize.

Anyway, I do agree that the surface area for attack is extremely low. Upstream is going to be implementing this policy change around the kernel 4.21 behavior in the future and I expect that we'll eventually SRU that version too at that time.

Regarding comment #12,
I think there is some other bug with the automatic authorization not working in some situation, I saw this on 18.10 too (see the confusion about whether I was testing 18.10 or 18.04 in https://gitlab.gnome.org/GNOME/gnome-shell/issues/709).

It doesn't make sense to block this SRU right now for that reason especially since it's intertwined with the fwupd and gnome-software one. I think we should just all agree that there will be a future bolt release that we'll SRU at that time and this policy will improve in the future.

[Sebastien Bacher (seb128)]

@Mario, thanks, I agree with you wrote and marking it as verification-done. The current system is not perfect but the issues are not regression/worth blocked the SRUs which are pending and we can improve thing in the next iteration.

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for bolt has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package bolt - 0.5-0ubuntu0.18.04.1

---------------
bolt (0.5-0ubuntu0.18.04.1) bionic; urgency=medium

  * New upstream version (lp: #1798014)
  * debian/control:
    - update the meson requirement according to the upstream changes
  * debian/rules:
    - drop use of the -Ddb-path option which is deprecated now
    - use -Dprivileged-group=sudo, the admin group is not 'wheel' for us
      (lp: #1800715)

 -- Sebastien Bacher <email address hidden> Tue, 30 Oct 2018 20:31:16 +0100

[Alex Tu (alextu)]

it would be better to remove item which not really be verified passed(#12) from change log to avoid confusing.

ex. Mario is confused on https://bugs.launchpad.net/somerville/+bug/1809721/comments/5