Ignoring access control file because of malformed name

[brphilly]

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]

Hi @brphilly,

a quick look at the code reveals that the error message is produced at…

usbguard/src/Daemon/Daemon.cpp

Lines 466 to 472 in 87e5c2d

	try {
	parseIPCAccessControlFilename(basename, &user, &group);
	}
	catch (...) {
	USBGUARD_LOG(Warning) << "Ignoring access control file because of malformed name: " << basename;
	return false;
	}

… which means that function parseIPCAccessControlFilename threw an exception. The way I read function parseIPCAccessControlFilename at…

usbguard/src/Daemon/Daemon.cpp

Lines 446 to 457 in 87e5c2d

	void Daemon::parseIPCAccessControlFilename(const std::string& basename, std::string* const ptr_user,
	std::string* const ptr_group)
	{
	const auto ug_separator = basename.find_first_of(":");
	const bool has_group = ug_separator != std::string::npos;
	const std::string user = basename.substr(0, ug_separator);
	const std::string group = has_group ? basename.substr(ug_separator + 1) : std::string();
	checkIPCAccessControlName(user);
	checkIPCAccessControlName(group);
	*ptr_user = user;
	*ptr_group = group;
	}

… 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…

usbguard/src/Daemon/Daemon.cpp

Lines 441 to 444 in 87e5c2d

	void Daemon::checkIPCAccessControlName(const std::string& name)
	{
	IPCServer::checkAccessControlName(name);
	}

…, which in turn forwards to isValidName at…

usbguard/src/Library/public/usbguard/IPCServer.cpp

Lines 33 to 42 in 64f7169

	void IPCServer::checkAccessControlName(const std::string& name)
	{
	if (name.size() > 32) {
	throw Exception("IPC access control", "name too long", name);
	}
	if (!isValidName(name)) {
	throw Exception("IPC access control", "invalid name format", name);
	}
	}

…, which rejects empty strings at…

usbguard/src/Common/Utility.cpp

Lines 546 to 554 in b15ef71

	bool isValidName(const std::string& name)
	{
	const char* s = name.data();
	if (('\0' == *s) ||
	!((('a' <= *s) && ('z' >= *s)) ||
	(('A' <= *s) && ('Z' >= *s)) ||
	('_' == *s))) {
	return false;

… (since commit b15ef71 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]

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

[brphilly]

@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]

@brphilly thanks for testing and reporting back! 👍

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

[hartwork]

@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]

@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]

I'll do it later today.

@radosroka awesome, thank you! 👍