From c5046c508e6a8e58b37fb980d87f70a594aeb9f7 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 13 Feb 2014 13:07:32 -0600 Subject: [PATCH v3 10/13] Mediation of processes sending and receiving messages When an AppArmor confined process wants to send or receive a message, a check is performed to see if the action should be allowed. When a message is going through dbus-daemon, there are two checks performed at once. One for the sending process and one for the receiving process. The checks are based on the process's label, the bus type, destination, path, interface, and member, as well as the peer's label and/or destination name. This allows for the traditional connection-based enforcement, as well as any fine-grained filtering desired by the system administrator. It is important to note that error and method_return messages are allowed to cut down on the amount of rules needed. If a process was allowed to send a message, it can receive error and method_return messages. An example AppArmor rule that would be needed to allow a process to call the UpdateActivationEnvironment method of the session bus itself would be: dbus send bus=session path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=UpdateActivationEnvironment peer=(name=org.freedesktop.DBus), To receive any message on the system bus from a process confined by the "confined-client" AppArmor profile: dbus receive bus=system peer=(label=confined-client), Bug: https://bugs.freedesktop.org/show_bug.cgi?id=75113 Signed-off-by: John Johansen [tyhicks: Use BusAppArmorConfinement, bug fixes, cleanup, commit msg] Signed-off-by: Tyler Hicks --- * Changes in v2: - Fix up minor conflicts from changes in earlier patches - Fix coding style issue of bus_apparmor_allows_send() declaration - Call the newly named modestr_is_complain() function - Adjust build_query() call sites to use DBusString for the query - Create build_message_query() to get rid of variable length build_query() args - Fix bad (bustype && print (bustype ? bustype : "unknown")) logic - Use the renamed bus_connection_dup_apparmor_confinement() function - Always set the DBusError from inside bus_apparmor_allows_send() when returning FALSE - Rename variables having 's' and 't' prefixes to 'src_' and 'dst_' prefixes - Rename string_alloced variable to free_auxdata - Fixed coding style nit regarding braces being included or excluded equally on all connection conditional blocks - Remove "do we want to include the msgtype in the encoding ..." comment that wasn't supposed to make it into the submitted patches - Changed the example in the commit message from using the Hello method to using the UpdateActivationEnvironment method - Added a bool argument to bus_apparmor_allows_send() to indicate if the reply was requested - Adjusted the logic for letting method_return and error messages through without querying the AppArmor policy. They're now only allowed through if requested_reply is true. Unrequested replies are subjected to the AppArmor policy checks. * Changes in v3: - Added Bug link in commit message - Rename 'method' argument to 'member' in bus_apparmor_allows_send() - Document all bus_apparmor_allows_send() parameters bus/apparmor.c | 328 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ bus/apparmor.h | 12 +++ bus/bus.c | 19 +++- 3 files changed, 358 insertions(+), 1 deletion(-) diff --git a/bus/apparmor.c b/bus/apparmor.c index 5ffce3d..05c9704 100644 --- a/bus/apparmor.c +++ b/bus/apparmor.c @@ -280,6 +280,24 @@ build_service_query (DBusString *query, query_append (query, name); } +static dbus_bool_t +build_message_query (DBusString *query, + const char *src_con, + const char *bustype, + const char *name, + const char *dst_con, + const char *path, + const char *interface, + const char *member) +{ + return build_common_query (query, src_con, bustype) && + query_append (query, name) && + query_append (query, dst_con) && + query_append (query, path) && + query_append (query, interface) && + query_append (query, member); +} + static void set_error_from_query_errno (DBusError *error, int error_number) { @@ -287,6 +305,41 @@ set_error_from_query_errno (DBusError *error, int error_number) "Failed to query AppArmor policy: %s", _dbus_strerror (error_number)); } + +static void +set_error_from_denied_message (DBusError *error, + DBusConnection *sender, + DBusConnection *proposed_recipient, + dbus_bool_t requested_reply, + const char *msgtype, + const char *path, + const char *interface, + const char *member, + const char *error_name, + const char *destination) +{ + const char *proposed_recipient_loginfo; + const char *unset = "(unset)"; + + proposed_recipient_loginfo = proposed_recipient ? + bus_connection_get_loginfo (proposed_recipient) : + "bus"; + + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, + "An AppArmor policy prevents this sender from sending this " + "message to this recipient, %d matched rules; type=\"%s\", " + "sender=\"%s\" (%s) interface=\"%s\" member=\"%s\" " + "error name=\"%s\" destination=\"%s\" (%s)", + requested_reply, + msgtype, + bus_connection_get_name (sender), + bus_connection_get_loginfo (sender), + interface ? interface : unset, + member ? member : unset, + error_name ? error_name : unset, + destination, + proposed_recipient_loginfo); +} #endif /* HAVE_APPARMOR */ /** @@ -622,3 +675,278 @@ bus_apparmor_allows_acquire_service (DBusConnection *connection, return TRUE; #endif /* HAVE_APPARMOR */ } + +/** + * Check if Apparmor security controls allow the message to be sent to a + * particular connection based on the security context of the sender and + * that of the receiver. The destination connection need not be the + * addressed recipient, it could be an "eavesdropper" + * + * @param sender the sender of the message. + * @param proposed_recipient the connection the message is to be sent to. + * @param requested_reply TRUE if the message is a reply requested by + * proposed_recipient + * @param bustype name of the bus + * @param msgtype message type (method_call, error, etc.) + * @param path object path the message should be sent to + * @param interface the type of the object instance + * @param member the member of the object + * @param error_name the name of the error if the message type is error + * @param destination name that the message should be sent to + * @param source name that the message should be sent from + * @param error the reason for failure when FALSE is returned + * @returns TRUE if the message is permitted + */ +dbus_bool_t +bus_apparmor_allows_send (DBusConnection *sender, + DBusConnection *proposed_recipient, + dbus_bool_t requested_reply, + const char *bustype, + const char *msgtype, + const char *path, + const char *interface, + const char *member, + const char *error_name, + const char *destination, + const char *source, + DBusError *error) +{ +#ifdef HAVE_APPARMOR + BusAppArmorConfinement *src_con = NULL, *dst_con = NULL; + DBusString qstr, auxdata; + dbus_bool_t src_allow = FALSE, dst_allow = FALSE; + dbus_bool_t src_audit = TRUE, dst_audit = TRUE; + dbus_bool_t free_auxdata = FALSE; + unsigned long pid; + int len, res, src_errno = 0, dst_errno = 0; + uint32_t src_perm = AA_DBUS_SEND, dst_perm = AA_DBUS_RECEIVE; + + if (!apparmor_enabled) + return TRUE; + + _dbus_assert (sender != NULL); + + src_con = bus_connection_dup_apparmor_confinement (sender); + + if (proposed_recipient) + { + dst_con = bus_connection_dup_apparmor_confinement (proposed_recipient); + } + else + { + dst_con = bus_con; + bus_apparmor_confinement_ref (dst_con); + } + + /* map reply messages: "error" and "method_return" to initial send and + * receive. That is permission to receive a message from X grants + * permission to reply to X. And permission to send a message to Y + * grants permission to receive a reply from Y. Note that this only applies + * to requested replies. Unrequested replies still require a policy query. + */ + if (requested_reply && + (strcmp (msgtype, "error") == 0 || strcmp (msgtype, "method_return") == 0)) + { + /* ignore requested reply messages and let dbus reply mapping handle them + * as the send was already allowed + */ + src_allow = TRUE; + dst_allow = TRUE; + goto out; + } + + if (is_unconfined (src_con->context, src_con->mode)) + { + src_allow = TRUE; + src_audit = FALSE; + } + else + { + if (!_dbus_string_init (&qstr)) + goto oom; + + if (!build_message_query (&qstr, src_con->context, bustype, destination, + dst_con->context, path, interface, member)) + { + _dbus_string_free (&qstr); + goto oom; + } + + res = aa_query_label (src_perm, + _dbus_string_get_data (&qstr), + _dbus_string_get_length (&qstr), + &src_allow, &src_audit); + _dbus_string_free (&qstr); + if (res == -1) + { + src_errno = errno; + set_error_from_query_errno (error, src_errno); + goto audit; + } + } + + if (is_unconfined (dst_con->context, dst_con->mode)) + { + dst_allow = TRUE; + dst_audit = FALSE; + } + else + { + if (!_dbus_string_init (&qstr)) + goto oom; + + if (!build_message_query (&qstr, dst_con->context, bustype, source, + src_con->context, path, interface, member)) + { + _dbus_string_free (&qstr); + goto oom; + } + + res = aa_query_label (dst_perm, + _dbus_string_get_data (&qstr), + _dbus_string_get_length (&qstr), + &dst_allow, &dst_audit); + _dbus_string_free (&qstr); + if (res == -1) + { + dst_errno = errno; + set_error_from_query_errno (error, dst_errno); + goto audit; + } + } + + /* Don't fail operations on profiles in complain mode */ + if (modestr_is_complain (src_con->mode)) + src_allow = TRUE; + if (modestr_is_complain (dst_con->mode)) + dst_allow = TRUE; + + if (!src_allow || !dst_allow) + set_error_from_denied_message (error, + sender, + proposed_recipient, + requested_reply, + msgtype, + path, + interface, + member, + error_name, + destination); + + if (!src_audit && !dst_audit) + goto out; + + audit: + if (!_dbus_string_init (&auxdata)) + goto oom; + free_auxdata = TRUE; + + if (!_dbus_append_pair_str (&auxdata, "bus", bustype ? bustype : "unknown")) + goto oom; + + if (path && !_dbus_append_pair_str (&auxdata, "path", path)) + goto oom; + + if (interface && !_dbus_append_pair_str (&auxdata, "interface", interface)) + goto oom; + + if (member && !_dbus_append_pair_str (&auxdata, "member", member)) + goto oom; + + if (error_name && !_dbus_append_pair_str (&auxdata, "error_name", error_name)) + goto oom; + + len = _dbus_string_get_length (&auxdata); + + if (src_audit) + { + if (!_dbus_append_mask (&auxdata, src_perm)) + goto oom; + + if (destination && !_dbus_append_pair_str (&auxdata, "name", destination)) + goto oom; + + if (sender && dbus_connection_get_unix_process_id (sender, &pid) && + !_dbus_append_pair_uint (&auxdata, "pid", pid)) + goto oom; + + if (src_con->context && + !_dbus_append_pair_str (&auxdata, "profile", src_con->context)) + goto oom; + + if (proposed_recipient && + dbus_connection_get_unix_process_id (proposed_recipient, &pid) && + !_dbus_append_pair_uint (&auxdata, "peer_pid", pid)) + goto oom; + + if (dst_con->context && + !_dbus_append_pair_str (&auxdata, "peer_profile", dst_con->context)) + goto oom; + + if (src_errno && !_dbus_append_pair_str (&auxdata, "info", strerror (src_errno))) + goto oom; + + if (dst_errno && + !_dbus_append_pair_str (&auxdata, "peer_info", strerror (dst_errno))) + goto oom; + + log_message (src_allow, msgtype, &auxdata); + } + if (dst_audit) + { + _dbus_string_set_length (&auxdata, len); + + if (source && !_dbus_append_pair_str (&auxdata, "name", source)) + goto oom; + + if (!_dbus_append_mask (&auxdata, dst_perm)) + goto oom; + + if (proposed_recipient && + dbus_connection_get_unix_process_id (proposed_recipient, &pid) && + !_dbus_append_pair_uint (&auxdata, "pid", pid)) + goto oom; + + if (dst_con->context && + !_dbus_append_pair_str (&auxdata, "profile", dst_con->context)) + goto oom; + + if (sender && dbus_connection_get_unix_process_id (sender, &pid) && + !_dbus_append_pair_uint (&auxdata, "peer_pid", pid)) + goto oom; + + if (src_con->context && + !_dbus_append_pair_str (&auxdata, "peer_profile", src_con->context)) + goto oom; + + if (dst_errno && !_dbus_append_pair_str (&auxdata, "info", strerror (dst_errno))) + goto oom; + + if (src_errno && + !_dbus_append_pair_str (&auxdata, "peer_info", strerror (src_errno))) + goto oom; + + log_message (dst_allow, msgtype, &auxdata); + } + + out: + if (src_con != NULL) + bus_apparmor_confinement_unref (src_con); + if (dst_con != NULL) + bus_apparmor_confinement_unref (dst_con); + if (free_auxdata) + _dbus_string_free (&auxdata); + + return src_allow && dst_allow; + + oom: + if (error != NULL && !dbus_error_is_set (error)) + BUS_SET_OOM (error); + src_allow = FALSE; + dst_allow = FALSE; + goto out; + +#else + return TRUE; +#endif /* HAVE_APPARMOR */ +} diff --git a/bus/apparmor.h b/bus/apparmor.h index 2010cbe..bdc601d 100644 --- a/bus/apparmor.h +++ b/bus/apparmor.h @@ -46,5 +46,17 @@ dbus_bool_t bus_apparmor_allows_acquire_service (DBusConnection *connection, const char *bustype, const char *service_name, DBusError *error); +dbus_bool_t bus_apparmor_allows_send (DBusConnection *sender, + DBusConnection *proposed_recipient, + dbus_bool_t requested_reply, + const char *bustype, + const char *msgtype, + const char *path, + const char *interface, + const char *member, + const char *error_name, + const char *destination, + const char *source, + DBusError *error); #endif /* BUS_APPARMOR_H */ diff --git a/bus/bus.c b/bus/bus.c index 7d76017..263ee36 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -1438,7 +1438,7 @@ bus_context_check_security_policy (BusContext *context, DBusMessage *message, DBusError *error) { - const char *dest; + const char *src, *dest; BusClientPolicy *sender_policy; BusClientPolicy *recipient_policy; dbus_int32_t toggles; @@ -1447,6 +1447,7 @@ bus_context_check_security_policy (BusContext *context, dbus_bool_t requested_reply; type = dbus_message_get_type (message); + src = dbus_message_get_sender (message); dest = dbus_message_get_destination (message); /* dispatch.c was supposed to ensure these invariants */ @@ -1536,6 +1537,22 @@ bus_context_check_security_policy (BusContext *context, return FALSE; } + /* next verify AppArmor access controls. If allowed then + * go on with the standard checks. + */ + if (!bus_apparmor_allows_send (sender, proposed_recipient, + requested_reply, + bus_context_get_type (context), + dbus_message_type_to_string (dbus_message_get_type (message)), + dbus_message_get_path (message), + dbus_message_get_interface (message), + dbus_message_get_member (message), + dbus_message_get_error_name (message), + dest ? dest : DBUS_SERVICE_DBUS, + src ? src : DBUS_SERVICE_DBUS, + error)) + return FALSE; + if (!bus_connection_is_active (sender)) { /* Policy for inactive connections is that they can only send -- 1.9.1