search

USBGuard / usbguard

USBGuard is a software framework for implementing USB device authorization policies (what kind of USB devices are authorized) as well as method of use policies (how a USB device may interact with the system)
https://usbguard.github.io/
GNU General Public License v2.0
1.1k stars 133 forks source link

Ignoring access control file because of malformed name #540

Closed brphilly closed 2 years ago

brphilly commented 2 years ago

Ever since upgrading to usbguard 1.1.0, I'm getting this error ERROR: IPC connect: service=usbguard: Operation not permitted when trying to use usbguard as my user. I ran usbguard-daemon -d and the logs showed

[1646452999.870] (D) Daemon.cpp@262/loadConfiguration: Setting IPCAllowedUsers to { root }
[1646452999.870] (T) Daemon.cpp@1086/addIPCAllowedUser: user=root
[1646452999.870] (D) Daemon.cpp@274/loadConfiguration: Setting IPCAllowedGroups to {  }
[1646452999.870] (D) Daemon.cpp@284/loadConfiguration: Setting DeviceRulesWithPort to false
[1646452999.870] (i) File has correct permissions.
[1646452999.870] (i) Loading IPC access control files at /etc/usbguard/IPCAccessControl.d
[1646452999.870] (T) Utility.cpp@361/loadFiles: L: brady : /etc/usbguard/IPCAccessControl.d/brady
[1646452999.870] (i) Loading IPC access control file /etc/usbguard/IPCAccessControl.d/brady
[1646452999.870] (W) Ignoring access control file because of malformed name: brady

The file /etc/usbguard/IPCAccessControl.d/brady was created by

sudo usbguard add-user brady --policy list --devices list,listen --exceptions listen --parameters list,listen

and it has the correct permissions.

I saw #479 and thought maybe that is causing the issue. I'm using Arch Linux in case it matters.

hartwork commented 2 years ago

Hi @brphilly,

a quick look at the code reveals that the error message is produced at… https://github.com/USBGuard/usbguard/blob/87e5c2dac79ca6edbe500391436aa565b59304a1/src/Daemon/Daemon.cpp#L466-L472 … which means that function parseIPCAccessControlFilename threw an exception. The way I read function parseIPCAccessControlFilename at… https://github.com/USBGuard/usbguard/blob/87e5c2dac79ca6edbe500391436aa565b59304a1/src/Daemon/Daemon.cpp#L446-L457 … it seems to be okay with both filename formats user:group and user but it then calls out to checkIPCAccessControlName for an empty group name, which then checkIPCAccessControlName forwards to checkAccessControlName at… https://github.com/USBGuard/usbguard/blob/87e5c2dac79ca6edbe500391436aa565b59304a1/src/Daemon/Daemon.cpp#L441-L444 …, which in turn forwards to isValidName at… https://github.com/USBGuard/usbguard/blob/64f7169ed346ed61fa5f25dafd897db4fa746ea0/src/Library/public/usbguard/IPCServer.cpp#L33-L42 …, which rejects empty strings at… https://github.com/USBGuard/usbguard/blob/b15ef713a9ac47e84525bbf829c7f444b84c3c81/src/Common/Utility.cpp#L546-L554 … (since commit b15ef713a9ac47e84525bbf829c7f444b84c3c81 introduce with release 1.1.0) which explains this problem.

So one possible fix would be to adjust function parseIPCAccessControlFilename to only call out to checkIPCAccessControlName(group) when group is non-empty. Does that make sense?

Best, Sebastian

hartwork commented 2 years ago

@brphilly any chance you could try if pull request #541 fully fixes this problem for you?

brphilly commented 2 years ago

@brphilly any chance you could try if pull request #541 fully fixes this problem for you?

Just compiled this PR and can confirm it fixed the problem. The logs now show

[1646502018.264] (i) Loading IPC access control files at /etc/usbguard/IPCAccessControl.d
[1646502018.264] (T) Utility.cpp@361/loadFiles: L: brady : /etc/usbguard/IPCAccessControl.d/brady
[1646502018.264] (i) Loading IPC access control file /etc/usbguard/IPCAccessControl.d/brady
[1646502018.264] (T) Daemon.cpp@1090/addIPCAllowedUser: user=brady

and everything seems to work. Thanks so much for the quick fix!

hartwork commented 2 years ago

@brphilly thanks for testing and reporting back! :+1:

@radosroka any chance for a release 1.1.1 with the regression fix from PR #541 ?

hartwork commented 2 years ago

@radosroka any chance for a release 1.1.1 with the regression fix from PR #541 ?

@radosroka I'm currently considering to backport PR #541 in Gentoo packaging of USBGuard but since it's a regression fix from 1.0.0, all distros should ideally have this patch and not need to accumulate distro-agnostic patches. Any chance you could cut release 1.1.1 from Git master?

radosroka commented 2 years ago

@radosroka any chance for a release 1.1.1 with the regression fix from PR #541 ?

@radosroka I'm currently considering to backport PR #541 in Gentoo packaging of USBGuard but since it's a regression fix from 1.0.0, all distros should ideally have this patch and not need to accumulate distro-agnostic patches. Any chance you could cut release 1.1.1 from Git master?

I'll do it later today.

hartwork commented 2 years ago

I'll do it later today.

@radosroka awesome, thank you! :+1: