ubuntu / authd

Authentication daemon for external Brokers
GNU Lesser General Public License v3.0
112 stars 9 forks source link

Issue: EntraID groups that are renamed result in auth failures because GID conflicts #620

Open arion-ch opened 1 day ago

arion-ch commented 1 day ago

Is there an existing issue for this?

Describe the issue

Authd appears to cache the name of each group a user is a member of, as supplied by the OIDC claim, with a randomly generated GID. However, group names can be changed at any time within EntraID. I'm not sure what is actually included in the claim, but if I explicitly rename a group, after having previously logged in to a server, the new group name immediately starts causing a conflict. This only impacts password authentication -- I can still SSH using a key -- but there is no way a user can resolve this without admin intervention.

User experience looks like this:

$ ssh first.last@domain.com@192.168.1.1
Insert 'r' to cancel the request and go back
(first.last@domain.com@192.168.1.1) Enter your local password:
authentication status failure: can't check authentication: failed to update user "first.last@domain.com": GID for group "NEW_GROUP" already in use by a different group
Insert 'r' to cancel the request and go back
(first.last@domain.com@192.168.1.1) Enter your local password:

The log files show the following entries:

2024-11-06T23:22:37.528555+00:00 analysis authd[14833]: 2024/11/06 23:22:37 ERROR GID 1499969210 for group "NEW_GROUP" already in use by group "OLD_GROUP"
2024-11-06T23:22:37.533334+00:00 analysis authd[14833]: 2024/11/06 23:22:37 WARN can't check authentication: failed to update user "first.last@domain.com": GID for group "NEW_GROUP" already in use by a different group

This work-around seems to work:

Steps to reproduce

System information and logs

authd version

authd   0.3.6

authd-msentraid broker version

name:      authd-msentraid
summary:   MSEntra ID broker for authd
publisher: Canonical**
store-url: https://snapcraft.io/authd-msentraid
license:   GPL-3.0
description: |
  This is the MS Entra ID broker snap for authd  to provide MS Entra ID OIDC
  based authentication on Ubuntu with authd.
services:
  authd-msentraid: simple, enabled, active
snap-id:      vS3oJLMss6lgWwoFcPqYDUA2HB20I1Dc
tracking:     0.x/edge
refresh-date: today at 01:15 UTC
channels:
  0.x/stable:    0.1+4fe9826.0f76acc 2024-10-02 (51) 18MB -
  0.x/candidate: ^
  0.x/beta:      ^
  0.x/edge:      0.1+fef0c13.918f3f8 2024-11-05 (61) 18MB -
installed:       0.1+fef0c13.918f3f8            (61) 18MB -

gnome-shell version

gnome-shell:
  Installed: 46.3.1-1ubuntu1~24.04.1authd2
  Candidate: 46.3.1-1ubuntu1~24.04.1authd2
  Version table:
 *** 46.3.1-1ubuntu1~24.04.1authd2 500
        500 https://ppa.launchpadcontent.net/ubuntu-enterprise-desktop/authd/ubuntu noble/main amd64 Packages
        100 /var/lib/dpkg/status
     46.0-0ubuntu6~24.04.5 500
        500 http://us.archive.ubuntu.com/ubuntu noble-updates/main amd64 Packages
     46.0-0ubuntu6~24.04.3 500
        500 http://security.ubuntu.com/ubuntu noble-security/main amd64 Packages
     46.0-0ubuntu5 500
        500 http://us.archive.ubuntu.com/ubuntu noble/main amd64 Packages

Distribution

Distributor ID: Ubuntu
Description:    Ubuntu 24.04.1 LTS
Release:        24.04
Codename:       noble

Logs

[180724.694978] analysis authd[14833]: 2024/11/06 23:22:37 ERROR GID 1499969210 for group "NEW_GROUP" already in use by group "OLD_GROUP"
[180724.694978] analysis authd[14833]: 2024/11/06 23:22:37 WARN can't check authentication: failed to update user "first.last@domain.com": GID for group "NEW_GROUP" already in use by a different group

authd broker configuration

/etc/authd/brokers.d/msentraid.conf

# This section is used by authd to identify and communicate with the broker.
# It should not be edited.
[authd]
name = Microsoft Entra ID
brand_icon = /snap/authd-msentraid/current/broker_icon.png
dbus_name = com.ubuntu.authd.MSEntraID
dbus_object = /com/ubuntu/authd/MSEntraID

authd-msentraid configuration

[oidc]
issuer = https://login.microsoftonline.com/<UUID redacted>/v2.0
client_id = <UUID redacted>

[users]
# The directory where the home directory will be created for new users.
# Existing users will keep their current directory.
# The user home directory will be created in the format of {home_base_dir}/{username}
home_base_dir = /home

# The username suffixes that are allowed to login via ssh without existing previously in the system.
# The suffixes must be separated by commas.
# ssh_allowed_suffixes = @example.com,@anotherexample.com
ssh_allowed_suffixes = @<redacted>

Double check your logs

junebugin commented 1 day ago

Experiencing the same issue via GDM.

In my case, the issue is easily reproducible when 2 conditions are met: -A group the user is a member of has had a name change. -Password change for the user takes place

EDIT: I see the author is using edge - just for awareness, I am using stable.

adombeck commented 1 day ago

Thanks a lot for the report. We're able to reproduce this and will look into solutions.

adombeck commented 1 day ago

@didrocks @denisonbarbosa:This bug is caused by this check.

If we merge https://github.com/canonical/authd-private/pull/9 as is (we should move that to the public repo btw), there won't be a failure anymore but instead the renamed group will receive a new GID. That's also incorrect behavior, because files created by the old group won't be accessible to users of the renamed group.

For groups which are managed in Microsoft Entra, I think we actually want to rename the group in the authd database if we notice that it has been renamed in Microsoft Entra. That would require that we store a "remoteID" field in the database, which contains the ID of the group in Microsoft Entra. Then when we add the user to groups during login, we check if there is another group with the same remoteID, and if so, rename that group to the new name. What do you think?

didrocks commented 1 day ago

That makes sense to me. Also, we can probably derive the GID from this remoteID? (there is still the issue with backward compatibility and potential collisions), but at least, we won’t have to explicitly track it ourself that way in an extra metadata column, wdyt?

adombeck commented 1 day ago

The collision issue is a problem which I still think we should fix (either via https://github.com/canonical/authd-private/pull/9 or by storing the GIDs and UIDs in Microsoft Entra), so for that reason I don't think it would be good to derive the GID from the remote ID.

didrocks commented 1 day ago

Please, refresh my memory: we do already pass the ugid (what you call remote id) in the userinfo, right?