From 949fb07e6e3e24f2a91a5ef940ee13894b0ecef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 5 Feb 2020 15:55:43 +0100 Subject: [PATCH 1/4] =?UTF-8?q?network:=20also=20change=20fair=5Fqueue=5Ft?= =?UTF-8?q?raffic=5Fpolicing=E2=86=92fair=5Fqueueing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up for ca58d00c68bce8ac33439fccd81491bf04205fc5. I messed up the "ing" suffix in the regex when doing the initial patch. --- src/network/networkd-network-gperf.gperf | 18 +++++++++--------- src/network/tc/fq.c | 18 +++++++++--------- src/network/tc/fq.h | 10 +++++----- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf index 6797153418..369631d9ee 100644 --- a/src/network/networkd-network-gperf.gperf +++ b/src/network/networkd-network-gperf.gperf @@ -258,15 +258,15 @@ ControlledDelay.IntervalSec, config_parse_controlled_delay_usec, ControlledDelay.CEThresholdSec, config_parse_controlled_delay_usec, QDISC_KIND_CODEL, 0 ControlledDelay.ECN, config_parse_controlled_delay_bool, QDISC_KIND_CODEL, 0 FairQueueing.Parent, config_parse_qdisc_parent, QDISC_KIND_FQ, 0 -FairQueueing.PacketLimit, config_parse_fair_queue_traffic_policing_u32, QDISC_KIND_FQ, 0 -FairQueueing.FlowLimit, config_parse_fair_queue_traffic_policing_u32, QDISC_KIND_FQ, 0 -FairQueueing.Quantum, config_parse_fair_queue_traffic_policing_size, QDISC_KIND_FQ, 0 -FairQueueing.InitialQuantum, config_parse_fair_queue_traffic_policing_size, QDISC_KIND_FQ, 0 -FairQueueing.MaximumRate, config_parse_fair_queue_traffic_policing_max_rate, QDISC_KIND_FQ, 0 -FairQueueing.Buckets, config_parse_fair_queue_traffic_policing_u32, QDISC_KIND_FQ, 0 -FairQueueing.OrphanMask, config_parse_fair_queue_traffic_policing_u32, QDISC_KIND_FQ, 0 -FairQueueing.Pacing, config_parse_fair_queue_traffic_policing_bool, QDISC_KIND_FQ, 0 -FairQueueing.CEThresholdSec, config_parse_fair_queue_traffic_policing_usec, QDISC_KIND_FQ, 0 +FairQueueing.PacketLimit, config_parse_fair_queueing_u32, QDISC_KIND_FQ, 0 +FairQueueing.FlowLimit, config_parse_fair_queueing_u32, QDISC_KIND_FQ, 0 +FairQueueing.Quantum, config_parse_fair_queueing_size, QDISC_KIND_FQ, 0 +FairQueueing.InitialQuantum, config_parse_fair_queueing_size, QDISC_KIND_FQ, 0 +FairQueueing.MaximumRate, config_parse_fair_queueing_max_rate, QDISC_KIND_FQ, 0 +FairQueueing.Buckets, config_parse_fair_queueing_u32, QDISC_KIND_FQ, 0 +FairQueueing.OrphanMask, config_parse_fair_queueing_u32, QDISC_KIND_FQ, 0 +FairQueueing.Pacing, config_parse_fair_queueing_bool, QDISC_KIND_FQ, 0 +FairQueueing.CEThresholdSec, config_parse_fair_queueing_usec, QDISC_KIND_FQ, 0 FairQueueingControlledDelay.Parent, config_parse_qdisc_parent, QDISC_KIND_FQ_CODEL, 0 FairQueueingControlledDelay.PacketLimit, config_parse_fair_queueing_controlled_delay_u32, QDISC_KIND_FQ_CODEL, 0 FairQueueingControlledDelay.MemoryLimit, config_parse_fair_queueing_controlled_delay_size, QDISC_KIND_FQ_CODEL, 0 diff --git a/src/network/tc/fq.c b/src/network/tc/fq.c index 28ee64fa69..c7eeec2307 100644 --- a/src/network/tc/fq.c +++ b/src/network/tc/fq.c @@ -11,7 +11,7 @@ #include "string-util.h" #include "util.h" -static int fair_queue_traffic_policing_init(QDisc *qdisc) { +static int fair_queueing_init(QDisc *qdisc) { FairQueueing *fq; assert(qdisc); @@ -24,7 +24,7 @@ static int fair_queue_traffic_policing_init(QDisc *qdisc) { return 0; } -static int fair_queue_traffic_policing_fill_message(Link *link, QDisc *qdisc, sd_netlink_message *req) { +static int fair_queueing_fill_message(Link *link, QDisc *qdisc, sd_netlink_message *req) { FairQueueing *fq; int r; @@ -102,7 +102,7 @@ static int fair_queue_traffic_policing_fill_message(Link *link, QDisc *qdisc, sd return 0; } -int config_parse_fair_queue_traffic_policing_u32( +int config_parse_fair_queueing_u32( const char *unit, const char *filename, unsigned line, @@ -165,7 +165,7 @@ int config_parse_fair_queue_traffic_policing_u32( return 0; } -int config_parse_fair_queue_traffic_policing_size( +int config_parse_fair_queueing_size( const char *unit, const char *filename, unsigned line, @@ -232,7 +232,7 @@ int config_parse_fair_queue_traffic_policing_size( return 0; } -int config_parse_fair_queue_traffic_policing_bool( +int config_parse_fair_queueing_bool( const char *unit, const char *filename, unsigned line, @@ -284,7 +284,7 @@ int config_parse_fair_queue_traffic_policing_bool( return 0; } -int config_parse_fair_queue_traffic_policing_usec( +int config_parse_fair_queueing_usec( const char *unit, const char *filename, unsigned line, @@ -343,7 +343,7 @@ int config_parse_fair_queue_traffic_policing_usec( return 0; } -int config_parse_fair_queue_traffic_policing_max_rate( +int config_parse_fair_queueing_max_rate( const char *unit, const char *filename, unsigned line, @@ -403,8 +403,8 @@ int config_parse_fair_queue_traffic_policing_max_rate( } const QDiscVTable fq_vtable = { - .init = fair_queue_traffic_policing_init, + .init = fair_queueing_init, .object_size = sizeof(FairQueueing), .tca_kind = "fq", - .fill_message = fair_queue_traffic_policing_fill_message, + .fill_message = fair_queueing_fill_message, }; diff --git a/src/network/tc/fq.h b/src/network/tc/fq.h index 0e8ced03d0..5a8ed6d651 100644 --- a/src/network/tc/fq.h +++ b/src/network/tc/fq.h @@ -22,8 +22,8 @@ typedef struct FairQueueing { DEFINE_QDISC_CAST(FQ, FairQueueing); extern const QDiscVTable fq_vtable; -CONFIG_PARSER_PROTOTYPE(config_parse_fair_queue_traffic_policing_u32); -CONFIG_PARSER_PROTOTYPE(config_parse_fair_queue_traffic_policing_size); -CONFIG_PARSER_PROTOTYPE(config_parse_fair_queue_traffic_policing_bool); -CONFIG_PARSER_PROTOTYPE(config_parse_fair_queue_traffic_policing_usec); -CONFIG_PARSER_PROTOTYPE(config_parse_fair_queue_traffic_policing_max_rate); +CONFIG_PARSER_PROTOTYPE(config_parse_fair_queueing_u32); +CONFIG_PARSER_PROTOTYPE(config_parse_fair_queueing_size); +CONFIG_PARSER_PROTOTYPE(config_parse_fair_queueing_bool); +CONFIG_PARSER_PROTOTYPE(config_parse_fair_queueing_usec); +CONFIG_PARSER_PROTOTYPE(config_parse_fair_queueing_max_rate); From 22cd7aabecd81fca14026749e48eadddff98a4bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 6 Feb 2020 21:16:08 +0100 Subject: [PATCH 2/4] core/selinux-access: do not use NULL for %s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ../src/core/selinux-access.c: In function ‘mac_selinux_generic_access_check’: ../src/basic/log.h:223:27: error: ‘%s’ directive argument is null [-Werror=format-overflow=] ../src/core/selinux-access.c:235:85: note: format string is defined here 235 | log_warning_errno(errno, "SELinux getcon_raw failed (tclass=%s perm=%s): %m", tclass, permission); | ^~ I wonder why nobody ever noticed this. Fixes #14691 (other issues listed in that ticket have already been fixed). --- src/core/selinux-access.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c index fb38231702..2ad500d539 100644 --- a/src/core/selinux-access.c +++ b/src/core/selinux-access.c @@ -181,7 +181,7 @@ int mac_selinux_generic_access_check( sd_bus_error *error) { _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; - const char *tclass = NULL, *scon = NULL; + const char *tclass, *scon; struct audit_info audit_info = {}; _cleanup_free_ char *cl = NULL; char *fcon = NULL; @@ -223,7 +223,7 @@ int mac_selinux_generic_access_check( r = getfilecon_raw(path, &fcon); if (r < 0) { - log_warning_errno(errno, "SELinux getfilecon_raw on '%s' failed (tclass=%s perm=%s): %m", path, tclass, permission); + log_warning_errno(errno, "SELinux getfilecon_raw on '%s' failed (perm=%s): %m", path, permission); r = sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get file context on %s.", path); goto finish; } @@ -232,7 +232,7 @@ int mac_selinux_generic_access_check( } else { r = getcon_raw(&fcon); if (r < 0) { - log_warning_errno(errno, "SELinux getcon_raw failed (tclass=%s perm=%s): %m", tclass, permission); + log_warning_errno(errno, "SELinux getcon_raw failed (perm=%s): %m", permission); r = sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get current context."); goto finish; } From d7887449e7c972770b6dd896f7d8c09f0d26c88f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 6 Feb 2020 21:37:49 +0100 Subject: [PATCH 3/4] basic/selinux-util: expose _cleanup_freecon_ --- src/basic/selinux-util.c | 3 --- src/basic/selinux-util.h | 7 +++++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/basic/selinux-util.c b/src/basic/selinux-util.c index 1d209d03d5..90bb93ed0b 100644 --- a/src/basic/selinux-util.c +++ b/src/basic/selinux-util.c @@ -28,10 +28,7 @@ #include "time-util.h" #if HAVE_SELINUX -DEFINE_TRIVIAL_CLEANUP_FUNC(char*, freecon); DEFINE_TRIVIAL_CLEANUP_FUNC(context_t, context_free); - -#define _cleanup_freecon_ _cleanup_(freeconp) #define _cleanup_context_free_ _cleanup_(context_freep) static int cached_use = -1; diff --git a/src/basic/selinux-util.h b/src/basic/selinux-util.h index 639c35b687..b73b7c50e0 100644 --- a/src/basic/selinux-util.h +++ b/src/basic/selinux-util.h @@ -8,6 +8,13 @@ #include "macro.h" #include "label.h" +#if HAVE_SELINUX +#include + +DEFINE_TRIVIAL_CLEANUP_FUNC(char*, freecon); +#define _cleanup_freecon_ _cleanup_(freeconp) +#endif + bool mac_selinux_use(void); void mac_selinux_retest(void); From cff789b746c68290ddc72705b8eedc0d32d75ef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 6 Feb 2020 21:39:40 +0100 Subject: [PATCH 4/4] core/selinux-access: use _cleanup_ and improve logging Instead of setting the bus error structure and then freeing it, let's only set it if used. If we will ignore the selinux denial, say ", ignore" to make this clear. Also, use _cleanup_ to avoid gotos. --- src/core/selinux-access.c | 69 +++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c index 2ad500d539..00f5241cd1 100644 --- a/src/core/selinux-access.c +++ b/src/core/selinux-access.c @@ -182,10 +182,10 @@ int mac_selinux_generic_access_check( _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; const char *tclass, *scon; - struct audit_info audit_info = {}; _cleanup_free_ char *cl = NULL; - char *fcon = NULL; + _cleanup_freecon_ char *fcon = NULL; char **cmdline = NULL; + bool enforce = false; /* Will be set to the real value later if needed */ int r = 0; assert(message); @@ -204,7 +204,7 @@ int mac_selinux_generic_access_check( SD_BUS_CREDS_AUGMENT /* get more bits from /proc */, &creds); if (r < 0) - goto finish; + return r; /* The SELinux context is something we really should have * gotten directly from the message or sender, and not be an @@ -216,25 +216,39 @@ int mac_selinux_generic_access_check( r = sd_bus_creds_get_selinux_context(creds, &scon); if (r < 0) - goto finish; + return r; if (path) { /* Get the file context of the unit file */ - r = getfilecon_raw(path, &fcon); - if (r < 0) { - log_warning_errno(errno, "SELinux getfilecon_raw on '%s' failed (perm=%s): %m", path, permission); - r = sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get file context on %s.", path); - goto finish; + if (getfilecon_raw(path, &fcon) < 0) { + r = -errno; + enforce = security_getenforce() > 0; + + log_warning_errno(r, "SELinux getfilecon_raw on '%s' failed%s (perm=%s): %m", + path, + enforce ? "" : ", ignoring", + permission); + if (!enforce) + return 0; + + return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get file context on %s.", path); } tclass = "service"; + } else { - r = getcon_raw(&fcon); - if (r < 0) { - log_warning_errno(errno, "SELinux getcon_raw failed (perm=%s): %m", permission); - r = sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get current context."); - goto finish; + if (getcon_raw(&fcon) < 0) { + r = -errno; + enforce = security_getenforce() > 0; + + log_warning_errno(r, "SELinux getcon_raw failed%s (perm=%s): %m", + enforce ? "" : ", ignoring", + permission); + if (!enforce) + return 0; + + return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get current context."); } tclass = "system"; @@ -243,25 +257,24 @@ int mac_selinux_generic_access_check( sd_bus_creds_get_cmdline(creds, &cmdline); cl = strv_join(cmdline, " "); - audit_info.creds = creds; - audit_info.path = path; - audit_info.cmdline = cl; + struct audit_info audit_info = { + .creds = creds, + .path = path, + .cmdline = cl, + }; r = selinux_check_access(scon, fcon, tclass, permission, &audit_info); - if (r < 0) - r = sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "SELinux policy denies access."); + if (r < 0) { + r = errno_or_else(EPERM); + enforce = security_getenforce() > 0; - log_debug("SELinux access check scon=%s tcon=%s tclass=%s perm=%s path=%s cmdline=%s: %i", scon, fcon, tclass, permission, path, cl, r); - -finish: - freecon(fcon); - - if (r < 0 && security_getenforce() != 1) { - sd_bus_error_free(error); - r = 0; + if (enforce) + sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "SELinux policy denies access."); } - return r; + log_debug_errno(r, "SELinux access check scon=%s tcon=%s tclass=%s perm=%s path=%s cmdline=%s: %m", + scon, fcon, tclass, permission, path, cl); + return enforce ? r : 0; } #else