[apparmor] [patch] parser - add support for variable expansion in dbus rules

[Seth Arnold]

On Thu, Aug 29, 2013 at 09:17:24AM -0700, Steve Beattie wrote:
> Subject: parser - add support for variable expansion in dbus rules
> Bug: https://bugs.launchpad.net/bugs/1218099
> 
> This patch adds support for expanding variables with dbus rules.
> Specifically, they can expanded within the bus, name, path, member,
> interface, and peer label fields.
> 
> Parser test cases and regression test cases are added as well.
> 
> Signed-off-by: Steve Beattie <sbeattie at ubuntu.com>

Acked-by: Seth Arnold <seth.arnold at canonical.com>

I've got some comments inline -- feel free to commit with or without
any further changes. (The small 'ret' change isn't worth much effort
one way or another, and the testing suggestion is large enough that it
shouldn't block anything.)

> +static int process_dbus_variables(struct dbus_entry *entry_list)
> +{
> +	int ret = TRUE, rc;
> +	struct dbus_entry *entry;
> +
> +	list_for_each(entry_list, entry) {
> +		rc = expand_entry_variables(&entry->bus, entry,
> +					    clone_and_chain_dbus);
> +		if (!rc)
> +			return FALSE;
> +		rc = expand_entry_variables(&entry->name, entry,
> +					    clone_and_chain_dbus);
> +		if (!rc)
> +			return FALSE;
> +		rc = expand_entry_variables(&entry->peer_label, entry,
> +					    clone_and_chain_dbus);
> +		if (!rc)
> +			return FALSE;
> +		rc = expand_entry_variables(&entry->path, entry,
> +					    clone_and_chain_dbus);
> +		if (!rc)
> +			return FALSE;
> +		rc = expand_entry_variables(&entry->interface, entry,
> +					    clone_and_chain_dbus);
> +		if (!rc)
> +			return FALSE;
> +		rc = expand_entry_variables(&entry->member, entry,
> +					    clone_and_chain_dbus);
> +		if (!rc)
> +			return FALSE;
> +
> +	}
> +
> +	return ret;
> +}

The final 'return ret' is the only use of 'ret' aside from
initialization. You might as well drop 'ret' and just return TRUE at
the end directly.

> +/does/not/exist {
> +  dbus (send)
> +       bus=session
> +       path="/com/canonical/hud/applications/@{BAR}",
> +}

> +/does/not/exist {
> +  dbus (receive)
> +       bus=accessibility
> +       interface=org.@{ORGS}.DBus.Properties
> +       path="/com/canonical/hud/applications/bar",
> +}

> +/does/not/exist {
> +  dbus (send)
> +       bus=@{BUSES}
> +       path="/com/canonical/hud/applications/baz",
> +}

> +/does/not/exist {
> +  dbus (send)
> +       bus=session
> +       member=@{MEMBERS}
> +       path="/com/canonical/hud/applications/biff",
> +}

> +/does/not/exist {
> +  dbus (send, receive)
> +       bus=session
> +       path="/foo/bar" member="bar"
> +       peer=(name="com.@{FOO}" label="/usr/bin/app.@{FOO}"),
> +}

> +/does/not/exist {
> +  dbus (bind)
> +       bus=session
> +       name="com.@{BAR}.@{FOO}",
> +}

> +/does/not/exist {
> +  dbus (bind)
> +       bus=session
> +       name="com.@{BAR}.@{FOO}",
> +}

These tests are a great start, but only one of them uses variables
in two fields. I'd love to see one with all the fields -- though it
would probably only tell us something new if we could compare the
after-expansion with a known-good expected expansion.

Thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130829/67325a5b/attachment.pgp>