Added ability to access devices with unclaimed interfaces.
BUG=chromium:243798
TEST=existing unit tests
Change-Id: Iac36f6429fb66047ee5abc73472d0bbe0d7fa303
diff --git a/allow_usb_device_rule_unittest.cc b/allow_usb_device_rule_unittest.cc
index 80d4bd5..ebec24f 100644
--- a/allow_usb_device_rule_unittest.cc
+++ b/allow_usb_device_rule_unittest.cc
@@ -21,11 +21,13 @@
};
TEST_F(AllowUsbDeviceRuleTest, IgnoreNonUsbDevice) {
- ASSERT_EQ(Rule::IGNORE, rule_.Process("/dev/loop0"));
+ ASSERT_EQ(Rule::IGNORE, rule_.Process("/dev/loop0",
+ Rule::ANY_INTERFACE));
}
TEST_F(AllowUsbDeviceRuleTest, AllowUsbDevice) {
- ASSERT_EQ(Rule::ALLOW, rule_.Process("/dev/bus/usb/001/001"));
+ ASSERT_EQ(Rule::ALLOW, rule_.Process("/dev/bus/usb/001/001",
+ Rule::ANY_INTERFACE));
}
} // namespace permission_broker
diff --git a/deny_claimed_usb_device_rule.cc b/deny_claimed_usb_device_rule.cc
index 1d1cd57..f3cddfd 100644
--- a/deny_claimed_usb_device_rule.cc
+++ b/deny_claimed_usb_device_rule.cc
@@ -7,6 +7,7 @@
#include <libudev.h>
#include "base/logging.h"
+#include "base/string_number_conversions.h"
namespace permission_broker {
@@ -17,7 +18,8 @@
udev_unref(udev_);
}
-Rule::Result DenyClaimedUsbDeviceRule::Process(const std::string &path) {
+Rule::Result DenyClaimedUsbDeviceRule::Process(const std::string &path,
+ int interface_id) {
struct udev_enumerate *enumerate = udev_enumerate_new(udev_);
udev_enumerate_scan_devices(enumerate);
@@ -31,15 +33,34 @@
// usb_interface entries--direct descendants of usb_device entires--are not,
// for the purposes of this rule, considered as having claimed the device.
const char *device_type = udev_device_get_devtype(device);
- if (device_type && !strcmp(device_type, "usb_interface")) {
+ if (device_type &&
+ !strcmp(device_type, "usb_interface") &&
+ interface_id == ANY_INTERFACE) {
udev_device_unref(device);
continue;
}
struct udev_device *parent = udev_device_get_parent_with_subsystem_devtype(
device, "usb", "usb_device");
- if (parent && (path == udev_device_get_devnode(parent)))
+ if (parent && (path == udev_device_get_devnode(parent))) {
deny = true;
+ // Check if the specific interface is already claimed (has driver).
+ if (interface_id != ANY_INTERFACE) {
+ const char* driver = udev_device_get_driver(device);
+ if (!driver) {
+ const char* interface_num = udev_device_get_sysattr_value(
+ device, "bInterfaceNumber");
+ int interface_value;
+ if (interface_num &&
+ base::HexStringToInt(interface_num, &interface_value) &&
+ interface_value == interface_id) {
+ deny = false;
+ udev_device_unref(device);
+ break;
+ }
+ }
+ }
+ }
udev_device_unref(device);
}
diff --git a/deny_claimed_usb_device_rule.h b/deny_claimed_usb_device_rule.h
index 1e2ac15..51dd91f 100644
--- a/deny_claimed_usb_device_rule.h
+++ b/deny_claimed_usb_device_rule.h
@@ -26,7 +26,8 @@
DenyClaimedUsbDeviceRule();
virtual ~DenyClaimedUsbDeviceRule();
- virtual Result Process(const std::string &path);
+ virtual Result Process(const std::string &path,
+ const int interface_id);
private:
struct udev *const udev_;
diff --git a/deny_claimed_usb_device_rule_unittest.cc b/deny_claimed_usb_device_rule_unittest.cc
index c75ed99..0898dab 100644
--- a/deny_claimed_usb_device_rule_unittest.cc
+++ b/deny_claimed_usb_device_rule_unittest.cc
@@ -59,7 +59,7 @@
};
TEST_F(DenyClaimedUsbDeviceRuleTest, IgnoreNonUsbDevice) {
- ASSERT_EQ(Rule::IGNORE, rule_.Process("/dev/tty0"));
+ ASSERT_EQ(Rule::IGNORE, rule_.Process("/dev/tty0", Rule::ANY_INTERFACE));
}
TEST_F(DenyClaimedUsbDeviceRuleTest, DenyClaimedUsbDevice) {
@@ -74,7 +74,7 @@
for (set<string>::const_iterator i = claimed_usb_devices.begin(); i !=
claimed_usb_devices.end(); ++i)
- ASSERT_EQ(Rule::DENY, rule_.Process(*i)) << *i;
+ ASSERT_EQ(Rule::DENY, rule_.Process(*i, Rule::ANY_INTERFACE)) << *i;
}
} // namespace permission_broker
diff --git a/deny_usb_device_class_rule_unittest.cc b/deny_usb_device_class_rule_unittest.cc
index 4161ebe..3fe2984 100644
--- a/deny_usb_device_class_rule_unittest.cc
+++ b/deny_usb_device_class_rule_unittest.cc
@@ -22,11 +22,13 @@
};
TEST_F(DenyUsbDeviceClassRuleTest, IgnoreNonUsbDevice) {
- ASSERT_EQ(Rule::IGNORE, rule_.Process("/dev/loop0"));
+ ASSERT_EQ(Rule::IGNORE, rule_.Process("/dev/loop0",
+ Rule::ANY_INTERFACE));
}
TEST_F(DenyUsbDeviceClassRuleTest, DenyMatchingUsbDevice) {
- ASSERT_EQ(Rule::DENY, rule_.Process("/dev/bus/usb/001/001"));
+ ASSERT_EQ(Rule::DENY, rule_.Process("/dev/bus/usb/001/001",
+ Rule::ANY_INTERFACE));
}
} // namespace permission_broker
diff --git a/deny_usb_vendor_id_rule_unittest.cc b/deny_usb_vendor_id_rule_unittest.cc
index 80b71d6..908f1d8 100644
--- a/deny_usb_vendor_id_rule_unittest.cc
+++ b/deny_usb_vendor_id_rule_unittest.cc
@@ -23,11 +23,13 @@
};
TEST_F(DenyUsbVendorIdRuleTest, IgnoreNonUsbDevice) {
- ASSERT_EQ(Rule::IGNORE, rule_.Process("/dev/loop0"));
+ ASSERT_EQ(Rule::IGNORE, rule_.Process("/dev/loop0",
+ Rule::ANY_INTERFACE));
}
TEST_F(DenyUsbVendorIdRuleTest, DenyMatchingUsbDevice) {
- ASSERT_EQ(Rule::DENY, rule_.Process("/dev/bus/usb/001/001"));
+ ASSERT_EQ(Rule::DENY, rule_.Process("/dev/bus/usb/001/001",
+ Rule::ANY_INTERFACE));
}
} // namespace permission_broker
diff --git a/permission_broker.cc b/permission_broker.cc
index 35a3c9e..be0edc7 100644
--- a/permission_broker.cc
+++ b/permission_broker.cc
@@ -11,6 +11,7 @@
#include <grp.h>
#include <libudev.h>
#include <poll.h>
+#include <stdint.h>
#include <sys/inotify.h>
#include <sys/types.h>
#include <unistd.h>
@@ -93,13 +94,15 @@
rules_.push_back(rule);
}
-bool PermissionBroker::ProcessPath(const string &path) {
+bool PermissionBroker::ProcessPath(const string &path,
+ int interface_id) {
WaitForEmptyUdevQueue();
LOG(INFO) << "ProcessPath(" << path << ")";
Rule::Result result = Rule::IGNORE;
for (unsigned int i = 0; i < rules_.size(); ++i) {
- const Rule::Result rule_result = rules_[i]->Process(path);
+ const Rule::Result rule_result = rules_[i]->Process(path,
+ interface_id);
LOG(INFO) << " " << rules_[i]->name() << ": "
<< Rule::ResultToString(rule_result);
if (rule_result == Rule::DENY)
@@ -122,9 +125,10 @@
return true;
}
-bool PermissionBroker::ExpandUsbIdentifiersToPaths(const uint16_t vendor_id,
- const uint16_t product_id,
- vector<string> *paths) {
+bool PermissionBroker::ExpandUsbIdentifiersToPaths(
+ const uint16_t vendor_id,
+ const uint16_t product_id,
+ vector<string> *paths) {
CHECK(paths) << "Cannot invoke ExpandUsbIdentifiersToPaths with NULL paths.";
paths->clear();
@@ -213,22 +217,29 @@
dbus_bool_t success = false;
char *path = NULL;
+ int interface_id = Rule::ANY_INTERFACE;
DBusError error;
dbus_error_init(&error);
if (!dbus_message_get_args(message, &error,
DBUS_TYPE_STRING, &path,
+ DBUS_TYPE_INT32, &interface_id,
DBUS_TYPE_INVALID)) {
- LOG(WARNING) << "Error parsing arguments: " << error.message;
- dbus_error_free(&error);
+ interface_id = Rule::ANY_INTERFACE;
+ if (!dbus_message_get_args(message, &error,
+ DBUS_TYPE_STRING, &path,
+ DBUS_TYPE_INVALID)) {
+ LOG(WARNING) << "Error parsing arguments: " << error.message;
+ dbus_error_free(&error);
- dbus_message_append_args(reply,
- DBUS_TYPE_BOOLEAN, &success,
- DBUS_TYPE_INVALID);
- return reply;
+ dbus_message_append_args(reply,
+ DBUS_TYPE_BOOLEAN, &success,
+ DBUS_TYPE_INVALID);
+ return reply;
+ }
}
- success = ProcessPath(path);
+ success = ProcessPath(path, interface_id);
dbus_message_append_args(reply,
DBUS_TYPE_BOOLEAN, &success,
DBUS_TYPE_INVALID);
@@ -243,30 +254,40 @@
dbus_bool_t success = false;
uint16_t vendor_id;
uint16_t product_id;
+ int interface_id = Rule::ANY_INTERFACE;
DBusError error;
dbus_error_init(&error);
if (!dbus_message_get_args(message, &error,
DBUS_TYPE_UINT16, &vendor_id,
DBUS_TYPE_UINT16, &product_id,
+ DBUS_TYPE_INT32, &interface_id,
DBUS_TYPE_INVALID)) {
- LOG(WARNING) << "Error parsing arguments: " << error.message;
- dbus_error_free(&error);
+ interface_id = Rule::ANY_INTERFACE;
+ if (!dbus_message_get_args(message, &error,
+ DBUS_TYPE_UINT16, &vendor_id,
+ DBUS_TYPE_UINT16, &product_id,
+ DBUS_TYPE_INVALID)) {
+ LOG(WARNING) << "Error parsing arguments: " << error.message;
+ dbus_error_free(&error);
- dbus_message_append_args(reply,
- DBUS_TYPE_BOOLEAN, &success,
- DBUS_TYPE_INVALID);
- return reply;
+ dbus_message_append_args(reply,
+ DBUS_TYPE_BOOLEAN, &success,
+ DBUS_TYPE_INVALID);
+ return reply;
+ }
}
if (ContainsKey(usb_exceptions_, std::make_pair(vendor_id, product_id))) {
success = true;
} else {
vector<string> paths;
- if (ExpandUsbIdentifiersToPaths(vendor_id, product_id, &paths)) {
+ if (ExpandUsbIdentifiersToPaths(vendor_id,
+ product_id,
+ &paths)) {
success = true;
for (unsigned int i = 0; i < paths.size(); ++i)
- success &= ProcessPath(paths[i]);
+ success &= ProcessPath(paths[i], interface_id);
} else {
LOG(INFO) << "Could not expand (" << vendor_id << ", " << product_id
<< ") to a list of device nodes.";
diff --git a/permission_broker.h b/permission_broker.h
index 9296e87..9326733 100644
--- a/permission_broker.h
+++ b/permission_broker.h
@@ -64,7 +64,8 @@
// executing all of the stored rules, no rule has explicitly allowed access to
// the path then access is denied. If _any_ rule denies access to |path| then
// processing the rules is aborted early and access is denied.
- bool ProcessPath(const std::string &path);
+ bool ProcessPath(const std::string &path,
+ int interface_id);
// Grants access to |path|, which is accomplished by changing the owning group
// on the path to the one specified numerically by the 'access_group' flag.
diff --git a/permission_broker_unittest.cc b/permission_broker_unittest.cc
index 15018d7..dc54bc2 100644
--- a/permission_broker_unittest.cc
+++ b/permission_broker_unittest.cc
@@ -22,7 +22,7 @@
MockRule() : Rule("MockRule") {}
virtual ~MockRule() {}
- MOCK_METHOD1(Process, Result(const string &path));
+ MOCK_METHOD2(Process, Result(const string &path, int interface_id));
private:
DISALLOW_COPY_AND_ASSIGN(MockRule);
@@ -49,14 +49,14 @@
PermissionBrokerTest() {}
virtual ~PermissionBrokerTest() {}
- bool ProcessPath(const string &path) {
- return broker_.ProcessPath(path);
+ bool ProcessPath(const string &path, int interface_id) {
+ return broker_.ProcessPath(path, interface_id);
}
protected:
Rule *CreateMockRule(const Rule::Result result) const {
MockRule *rule = new MockRule();
- EXPECT_CALL(*rule, Process(_))
+ EXPECT_CALL(*rule, Process(_, _))
.WillOnce(Return(result));
return rule;
}
@@ -70,21 +70,21 @@
TEST_F(PermissionBrokerTest, EmptyRuleChain) {
EXPECT_CALL(broker_, MockGrantAccess(_))
.Times(0);
- ASSERT_FALSE(ProcessPath("/dev/foo"));
+ ASSERT_FALSE(ProcessPath("/dev/foo", Rule::ANY_INTERFACE));
}
TEST_F(PermissionBrokerTest, AllowAccess) {
EXPECT_CALL(broker_, MockGrantAccess("/dev/foo"))
.WillOnce(Return(true));
broker_.AddRule(CreateMockRule(Rule::ALLOW));
- ASSERT_TRUE(ProcessPath("/dev/foo"));
+ ASSERT_TRUE(ProcessPath("/dev/foo", Rule::ANY_INTERFACE));
}
TEST_F(PermissionBrokerTest, DenyAccess) {
EXPECT_CALL(broker_, MockGrantAccess(_))
.Times(0);
broker_.AddRule(CreateMockRule(Rule::DENY));
- ASSERT_FALSE(ProcessPath("/dev/foo"));
+ ASSERT_FALSE(ProcessPath("/dev/foo", Rule::ANY_INTERFACE));
}
TEST_F(PermissionBrokerTest, DenyPrecedence) {
@@ -93,7 +93,7 @@
broker_.AddRule(CreateMockRule(Rule::ALLOW));
broker_.AddRule(CreateMockRule(Rule::IGNORE));
broker_.AddRule(CreateMockRule(Rule::DENY));
- ASSERT_FALSE(ProcessPath("/dev/foo"));
+ ASSERT_FALSE(ProcessPath("/dev/foo", Rule::ANY_INTERFACE));
}
TEST_F(PermissionBrokerTest, AllowPrecedence) {
@@ -102,7 +102,7 @@
broker_.AddRule(CreateMockRule(Rule::IGNORE));
broker_.AddRule(CreateMockRule(Rule::ALLOW));
broker_.AddRule(CreateMockRule(Rule::IGNORE));
- ASSERT_TRUE(ProcessPath("/dev/foo"));
+ ASSERT_TRUE(ProcessPath("/dev/foo", Rule::ANY_INTERFACE));
}
} // namespace permission_broker
diff --git a/rule.h b/rule.h
index 739edeb..e49f27f 100644
--- a/rule.h
+++ b/rule.h
@@ -20,13 +20,15 @@
class Rule {
public:
enum Result { ALLOW, DENY, IGNORE };
+ enum SpecialInterfaces { ANY_INTERFACE = -1 };
static const char *ResultToString(const Result &result);
virtual ~Rule();
const std::string &name();
- virtual Result Process(const std::string &path) = 0;
+ virtual Result Process(const std::string &path,
+ const int interface_id) = 0;
protected:
Rule(const std::string &name);
diff --git a/udev_rule.cc b/udev_rule.cc
index 0845530..e5b420f 100644
--- a/udev_rule.cc
+++ b/udev_rule.cc
@@ -20,7 +20,7 @@
udev_unref(udev_);
}
-Rule::Result UdevRule::Process(const string &path) {
+Rule::Result UdevRule::Process(const string &path, int interface_id) {
udev_enumerate_scan_devices(enumerate_);
struct udev_list_entry *entry = NULL;
diff --git a/udev_rule.h b/udev_rule.h
index a2149ff..9e3a4d2 100644
--- a/udev_rule.h
+++ b/udev_rule.h
@@ -22,7 +22,7 @@
virtual Result ProcessDevice(struct udev_device *device) = 0;
- virtual Result Process(const std::string& path);
+ virtual Result Process(const std::string& path, int interface_id);
private:
struct udev *const udev_;