Bug 466786

Unfortunately it seems like we're hitting this error condition:

                    QString helpfulActionErrorText = job->errorString();
                    if (helpfulActionErrorText.isEmpty()) {
                        // unknown error :(
                        helpfulActionErrorText = xi18nc("@info", "Failed to make user <resource>%1</resource> a member of group <resource>%2</resource>", user, group);
                    }
                    Q_EMIT helpfulActionError(helpfulActionErrorText);

So the backend process didn't give us any indication of what actually wrong wrong. :/

Is this 100% reproducible for you? If you remove your user from the appropriate group, reboot, and use the setup wizard again, does it happen again?

Pretty sure the group name must contain 'samba' for hardening reasons.

Where's that restriction enforced?

https://invent.kde.org/network/kdenetwork-filesharing/-/blob/master/samba/filepropertiesplugin/authhelper.cpp#L87

Aha.

Are you sure that's a good idea to keep? In principle the distro can set the group name to anything. It doesn't seem ideal to declare otherwise valid distro configs to be unauthorized, especially without bubbling up a notification of this to the user. We should do that at a minimum.

We also have differing definitions of valid group names in different places in the code. In authhelper.cpp, we want the group to contain "samba" but in groupmanager.cpp, we only check for whether the group name is "root".

Not sure I understand the question. Yes, we need some hardening ^^

Mind that there is a difference between an invalid group name (e.g. the directory is group owned by root) indicative of the setup being incorrect and an unauthorized group name.

A possibly relevant merge request was started @ https://invent.kde.org/network/kdenetwork-filesharing/-/merge_requests/40

A possibly relevant merge request was started @ https://invent.kde.org/network/kdenetwork-filesharing/-/merge_requests/41

Git commit 64998586b683d1268a260ae228eb35c3bdb1e55a by Nate Graham.
Committed on 10/03/2023 at 13:49.
Pushed by ngraham into branch 'master'.

Fix incorrect conditions and instructions in group setup

Right now the setup wizard checks for whether the group contains the
text "root", and if it does, it errors out and recommends that you
change the group owner to "usershares." But this isn't valid either; in
authhelper.cpp, it will only accept a group with the word "samba" in it,
plus a few other conditions.

This commit makes the frontend code for the wizard match the backend
conditions, so never recommends doing something invalid, which will then
fail.
FIXED-IN: 23.04

M  +1    -0    samba/filepropertiesplugin/authhelper.cpp
M  +7    -3    samba/filepropertiesplugin/groupmanager.cpp

https://invent.kde.org/network/kdenetwork-filesharing/commit/64998586b683d1268a260ae228eb35c3bdb1e55a

> Is this 100% reproducible for you? If you remove your user from the
> appropriate group, reboot, and use the setup wizard again, does it happen
> again?

do you still need clarification here?

No, we found and fixed the issue. Thanks for reporting it!

Also worth reporting to Fedora that the group name should be called "sambashares", not "usershares".

(In reply to Nate Graham from comment #14)
> Also worth reporting to Fedora that the group name should be called
> "sambashares", not "usershares".

hopefully the correct spot: https://pagure.io/fedora-kde/SIG/issue/109

(In reply to Harald Sitter from comment #7)
> Not sure I understand the question. Yes, we need some hardening ^^

Why does kde need to do some hardening via group name? I am not sure I understand.

The hardening would be done by the samba administrators and deciding whether to allow the usershares functionality and under what group (via write permissions I think).

Or am I missing something?

One can ask the auth helper to make the user a member of any group

(In reply to Harald Sitter from comment #17)
> One can ask the auth helper to make the user a member of any group

That is very nice but the question remains: Why does kde need to do some hardening via group name?

Yeah, I don't understand your question.

Because you can pass any old group in we need to limit the amount of garbage groups you can put in to abuse the system.

(In reply to Harald Sitter from comment #19)
> Yeah, I don't understand your question.
> 
> Because you can pass any old group in we need to limit the amount of garbage
> groups you can put in to abuse the system.
Who is "you"? Where can the user change the group they're asking to be made a member of?

you = any program, including malware.

How exactly would that do that? Woudn't they need local root access to modify the kauth files to change the group name?

(In reply to Harald Sitter from comment #19)
> Yeah, I don't understand your question.
> 
> Because you can pass any old group in we need to limit the amount of garbage
> groups you can put in to abuse the system.

Let me rephrase the question then:

What makes a group name `sambausers` safer than a group name `thisismyfavouritegroupname` ?

Either would be set by our system (in our Fedora's usecase by the "samba-usershares" package).

(In reply to Nate Graham from comment #22)
> How exactly would that do that? Woudn't they need local root access to
> modify the kauth files to change the group name?

The group name is not encoded in any kauth files?

Then where does the group get set? Can you help explain what exactly the vulnerability is here?

It's clear you understand it, but Marc and I don't, and we're not domain experts, so it would be helpful if you could be a bit more verbose and imagine that you're explaining the vulnerability in question and how restricting certain group names plugs it.

This call here https://invent.kde.org/network/kdenetwork-filesharing/-/blob/master/samba/filepropertiesplugin/groupmanager.cpp#L120 may be made by any application that has access to the bus. They may request becoming member of any group because of how the function works. The only thing standing between a malicious application making this request to push the user into the wheel group or root group or admin group and then exploit the access that comes with that is the group filtering.

I see, so KAuth files are public and any apps can try to use them. Is there no way to restrict them to only specific apps, where we can ensure more security.

Hi,

sorry to kind of abuse your report for this but:

(In reply to kinghat from comment #0)
> SUMMARY
> trying to share the default "Music" dir in home on fedora and the "make me a
> group member" button didn't function properly. manually adding myself to the
> group and restarting did.

could you please explain how exactly you were able to share the directory after manually adding your user to the group "usershares"?
I'm asking because of my report I did create a few months ago. https://bugs.kde.org/show_bug.cgi?id=463067

I ran into a dead end because kdenetwork-filesharing plugin asks for creating a Samba user when trying to share something.
It asks this although a Samba user (the name is the same like my system user) is already present. I did hop that it was fixed as the plugin was updated several times since I made that report. Sadly, I have still that issue. Sharing something is not possible. :/

Git commit d3ead0674aff6714b67ba9d3a9dfd38a57a66eb3 by Nate Graham.
Committed on 15/03/2023 at 17:33.
Pushed by ngraham into branch 'master'.

Show appropriate error messages when KAuth actions fail

Let's provide the user with the context needed to understand what went
wrong so they can figure out who's fault it is or how they might be able
to fix it.

M  +37   -9    samba/filepropertiesplugin/authhelper.cpp

https://invent.kde.org/network/kdenetwork-filesharing/commit/d3ead0674aff6714b67ba9d3a9dfd38a57a66eb3