diff --git a/include/trace/events/landlock.h b/include/trace/events/landlock.h index b7e6a6638822..a982a7cfa881 100644 --- a/include/trace/events/landlock.h +++ b/include/trace/events/landlock.h @@ -10,7 +10,10 @@ #if !defined(_TRACE_LANDLOCK_H) || defined(TRACE_HEADER_MULTI_READ) #define _TRACE_LANDLOCK_H +#include +#include #include +#include #include #include #include @@ -790,6 +793,11 @@ TRACE_EVENT(landlock_deny_access_fs, __get_dynamic_array_len(pathname) - 1)) ); +static_assert(offsetof(struct sockaddr_in, sin_port) == + offsetof(struct sockaddr_in6, sin6_port)); +static_assert(sizeof_field(struct sockaddr_in, sin_port) == + sizeof_field(struct sockaddr_in6, sin6_port)); + /** * landlock_deny_access_net - Network access denied * @@ -798,30 +806,31 @@ TRACE_EVENT(landlock_deny_access_fs, * @same_exec: Whether the current task entered the denying domain itself. * @logged: The domain's audit-logging decision for this denial. * @blockers: Request type and final missing access subset (never NULL). - * @sk: Socket object (never NULL), read without a socket lock, so its - * fields are a best-effort snapshot. The denied endpoint is not - * available: the hook runs before :manpage:`bind(2)` / - * :manpage:`connect(2)` sets the socket addresses. - * @sport: Source port in host endianness, set for bind denials (zero for - * an autobind/ephemeral port); zero for connect and send denials. - * @dport: Destination port in host endianness, set for connect and send - * denials; zero for bind denials, and also zero for a UDP send to - * an AF_UNSPEC address on an IPv6 socket (indistinguishable from a - * real destination port 0). The bind-vs-connect direction is - * given by @blockers, not by which port is set. + * @sk: Socket object (never NULL), read without a socket lock, so its fields + * are a best-effort snapshot. + * @socket_family: Socket-family snapshot used by the verdict. + * @address: Authoritative address checked by the verdict (never NULL). + * The producer copies @addrlen bytes from the checked address and + * zeroes the remaining storage before emission. The + * &sockaddr_in.sin_port or &sockaddr_in6.sin6_port member, when + * present, remains in network endianness. + * @addrlen: Validated signed length of @address. * - * Emitted when a Landlock domain denies a network operation. - * - * The port fields are converted from the socket's network byte order to - * host endianness before emitting. + * Emitted when a Landlock domain denies a network operation. The blocker + * identifies whether the address is a bind or connect/send policy object. + * The flattened port field is converted from the checked address to host + * endianness, or is -1 when no port was checked. Zero is a valid checked + * port. */ TRACE_EVENT(landlock_deny_access_net, TP_PROTO(const struct landlock_hierarchy *hierarchy, bool same_exec, bool logged, const struct landlock_blockers *blockers, - const struct sock *sk, u64 sport, u64 dport), + const struct sock *sk, u16 socket_family, + const struct sockaddr_storage *address, int addrlen), - TP_ARGS(hierarchy, same_exec, logged, blockers, sk, sport, dport), + TP_ARGS(hierarchy, same_exec, logged, blockers, sk, socket_family, + address, addrlen), TP_STRUCT__entry( __field( u64, domain_id ) @@ -829,26 +838,36 @@ TRACE_EVENT(landlock_deny_access_net, __field( bool, logged ) __field( enum landlock_request_type, blockers_type ) __field( access_mask_t, blockers_access ) - __field( u64, sport ) - __field( u64, dport ) + __field( s64, port ) ), TP_fast_assign( + const struct sockaddr *const addr = + (const struct sockaddr *)address; + const bool has_port = + addrlen >= (int)offsetofend(struct sockaddr_in, sin_port) && + (addr->sa_family == AF_INET || + addr->sa_family == AF_INET6 || + (addr->sa_family == AF_UNSPEC && + socket_family == AF_INET)); + __entry->domain_id = hierarchy->id; __entry->same_exec = same_exec; __entry->logged = logged; __entry->blockers_type = blockers->type; __entry->blockers_access = blockers->access; - __entry->sport = sport; - __entry->dport = dport; + __entry->port = + has_port ? + ntohs(((const struct sockaddr_in *)addr)->sin_port) : + -1; ), - TP_printk("domain=%llx same_exec=%d logged=%d blockers=%s sport=%llu dport=%llu", - __entry->domain_id, __entry->same_exec, __entry->logged, - __entry->blockers_type == LANDLOCK_REQUEST_NET_ACCESS ? - __print_flags(__entry->blockers_access, "|", _LANDLOCK_ACCESS_NET_NAMES) : - "unknown", - __entry->sport, __entry->dport) + TP_printk("domain=%llx same_exec=%d logged=%d blockers=%s port=%lld", + __entry->domain_id, __entry->same_exec, __entry->logged, + __entry->blockers_type == LANDLOCK_REQUEST_NET_ACCESS ? + __print_flags(__entry->blockers_access, "|", _LANDLOCK_ACCESS_NET_NAMES) : + "unknown", + __entry->port) ); /** diff --git a/security/landlock/log.h b/security/landlock/log.h index 04f3e241e765..4587c2b1566d 100644 --- a/security/landlock/log.h +++ b/security/landlock/log.h @@ -15,6 +15,7 @@ struct landlock_cred_security; struct landlock_hierarchy; +struct sockaddr; enum landlock_request_type { LANDLOCK_REQUEST_PTRACE = 1, @@ -30,6 +31,16 @@ struct landlock_blockers { enum landlock_request_type type; }; +#ifdef CONFIG_TRACEPOINTS + +struct landlock_net_trace { + const struct sockaddr *address; + int addrlen; + u16 socket_family; +}; + +#endif /* CONFIG_TRACEPOINTS */ + /* * We should be careful to only use a variable of this type for * landlock_log_denial(). This way, the compiler can remove it entirely if @@ -57,13 +68,20 @@ struct landlock_request { deny_masks_t deny_masks; optional_access_t quiet_optional_accesses; - /* - * Other-party domain ID for a relational (scope/ptrace) denial, or 0 if - * that party is unsandboxed. An ID, not a pointer: the other task can - * replace its credential and free the domain it referenced. Trace path - * only; audit ignores it. - */ - u64 other_domain_id; + union { + /* + * Other-party domain ID for a relational (scope/ptrace) denial, + * or 0 if that party is unsandboxed. Store an ID, not a + * pointer: the other task can replace its credential and free + * the domain it referenced. Trace-only; audit ignores it. + */ + u64 other_domain_id; + +#ifdef CONFIG_TRACEPOINTS + /* Synchronous context for a network denial. */ + const struct landlock_net_trace *trace_net; +#endif /* CONFIG_TRACEPOINTS */ + }; }; #ifdef CONFIG_SECURITY_LANDLOCK_LOG diff --git a/security/landlock/net.c b/security/landlock/net.c index 5552c60388f8..6fe0dbde3b78 100644 --- a/security/landlock/net.c +++ b/security/landlock/net.c @@ -93,7 +93,7 @@ static int current_check_access_socket(struct socket *const sock, return 0; /* Checks for minimal header length to safely read sa_family. */ - if (addrlen < offsetofend(typeof(*address), sa_family)) + if (addrlen < (int)offsetofend(typeof(*address), sa_family)) return -EINVAL; /* @@ -145,6 +145,17 @@ static int current_check_access_socket(struct socket *const sock, .audit.u.net = &audit_net, .access = access_request, .layer_masks = &layer_masks, +#ifdef CONFIG_TRACEPOINTS + .trace_net = + &(struct landlock_net_trace){ + .address = + address, + .addrlen = + addrlen, + .socket_family = + sock_family, + }, +#endif /* CONFIG_TRACEPOINTS */ }); return -EACCES; } @@ -276,14 +287,22 @@ static int current_check_access_socket(struct socket *const sock, audit_net.family = address->sa_family; audit_net.sk = sock->sk; - landlock_log_denial(subject, - &(struct landlock_request){ - .type = LANDLOCK_REQUEST_NET_ACCESS, - .audit.type = LSM_AUDIT_DATA_NET, - .audit.u.net = &audit_net, - .access = access_request, - .layer_masks = &layer_masks, - }); + landlock_log_denial( + subject, &(struct landlock_request){ + .type = LANDLOCK_REQUEST_NET_ACCESS, + .audit.type = LSM_AUDIT_DATA_NET, + .audit.u.net = &audit_net, + .access = access_request, + .layer_masks = &layer_masks, +#ifdef CONFIG_TRACEPOINTS + .trace_net = + &(struct landlock_net_trace){ + .address = address, + .addrlen = addrlen, + .socket_family = sock_family, + }, +#endif /* CONFIG_TRACEPOINTS */ + }); return -EACCES; } diff --git a/security/landlock/trace.c b/security/landlock/trace.c index 58276cc32d3f..9be86638f905 100644 --- a/security/landlock/trace.c +++ b/security/landlock/trace.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include "access.h" @@ -157,16 +158,31 @@ void landlock_trace_denial( break; case LANDLOCK_REQUEST_NET_ACCESS: if (trace_landlock_deny_access_net_enabled()) { + const struct landlock_net_trace *const trace_net = + request->trace_net; const struct landlock_blockers blockers = { .access = missing, .type = request->type, }; + struct sockaddr_storage address = {}; + if (WARN_ON_ONCE(!trace_net || !trace_net->address)) + return; + + if (WARN_ON_ONCE( + trace_net->addrlen < + (int)offsetofend(struct sockaddr, + sa_family) || + trace_net->addrlen > (int)sizeof(address))) + return; + + memcpy(&address, trace_net->address, + trace_net->addrlen); trace_landlock_deny_access_net( youngest_denied, same_exec, logged, &blockers, request->audit.u.net->sk, - ntohs(request->audit.u.net->sport), - ntohs(request->audit.u.net->dport)); + trace_net->socket_family, &address, + trace_net->addrlen); } break; case LANDLOCK_REQUEST_PTRACE: diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c index 4fb705e1596d..28942438e154 100644 --- a/tools/testing/selftests/landlock/net_test.c +++ b/tools/testing/selftests/landlock/net_test.c @@ -3482,8 +3482,8 @@ TEST_F(trace_net, deny_access_net_bind) /* * Anchors the denial fields shared by every deny_access_net event so a field - * test proves more than sport/dport: the denying domain, the same-exec bit, the - * audit-logging verdict, and the blocked access all stay populated. + * test proves more than the checked endpoint: the denying domain, the same-exec + * bit, the audit-logging verdict, and the blocked access all stay populated. */ static void expect_net_deny_common_fields(struct __test_metadata *const _metadata, @@ -3569,38 +3569,38 @@ FIXTURE_VARIANT(trace_net_connect) { bool deny_connect; }; +/* Denied connect() to the next IPv4 port. */ /* clang-format off */ - -/* Denied connect(): sport=0, dport=. */ FIXTURE_VARIANT_ADD(trace_net_connect, connect_denied) { + /* clang-format on */ .handled = LANDLOCK_ACCESS_NET_CONNECT_TCP, .bind_base_first = false, .deny_connect = true, }; -/* Denied bind(): sport=, dport=0. */ +/* Denied bind() to the next IPv4 port. */ +/* clang-format off */ FIXTURE_VARIANT_ADD(trace_net_connect, bind_fields) { + /* clang-format on */ .handled = LANDLOCK_ACCESS_NET_BIND_TCP, .bind_base_first = false, .deny_connect = false, }; -/* Denied connect() after an allowed bind(): the connect fields (sport=0). */ +/* Denied connect() after an allowed bind() uses the checked destination. */ +/* clang-format off */ FIXTURE_VARIANT_ADD(trace_net_connect, connect_after_bind) { - .handled = LANDLOCK_ACCESS_NET_BIND_TCP | LANDLOCK_ACCESS_NET_CONNECT_TCP, + /* clang-format on */ + .handled = LANDLOCK_ACCESS_NET_BIND_TCP | + LANDLOCK_ACCESS_NET_CONNECT_TCP, .bind_base_first = true, .deny_connect = true, }; -/* clang-format on */ - /* - * A denied TCP bind(2) or connect(2) emits one deny_access_net event. The port - * is reported in the field matching the denied operation, in host endianness - * (the UAPI landlock_net_port_attr.port convention): a connect denial reports - * sport=0 dport=, a bind denial reports sport= dport=0, so a - * byte-order or field-swap bug is caught. A prior allowed bind - * (connect_after_bind) does not change the connect denial's fields. + * A denied TCP bind(2) or connect(2) emits one deny_access_net event with the + * checked IPv4 port in host endianness (the UAPI landlock_net_port_attr.port + * convention). A prior allowed bind does not change a connect denial's port. */ TEST_F(trace_net_connect, deny_access_net) { @@ -3693,21 +3693,13 @@ TEST_F(trace_net_connect, deny_access_net) expect_net_deny_common_fields(_metadata, buf); - /* - * The denied operation's port field carries the port; the other is 0. - */ snprintf(expected, sizeof(expected), "%llu", (unsigned long long)(sock_port_start + 1)); ASSERT_EQ(0, tracefs_extract_field(buf, REGEX_DENY_ACCESS_NET(TRACE_TASK), - "sport", field, sizeof(field))); - EXPECT_STREQ(variant->deny_connect ? "0" : expected, field); - - ASSERT_EQ(0, - tracefs_extract_field(buf, REGEX_DENY_ACCESS_NET(TRACE_TASK), - "dport", field, sizeof(field))); - EXPECT_STREQ(variant->deny_connect ? expected : "0", field); + "port", field, sizeof(field))); + EXPECT_STREQ(expected, field); free(buf); } @@ -3867,11 +3859,4 @@ TEST_F(trace_net_check_rule, check_rule_net_fields) free(buf); } -/* - * IPv6 network trace tests are intentionally elided. IPv6 hook dispatch uses - * the same current_check_access_socket() code path as IPv4, validated by the - * audit tests in this file. The trace events use the same blockers/sport/dport - * fields regardless of address family. - */ - TEST_HARNESS_MAIN diff --git a/tools/testing/selftests/landlock/trace.h b/tools/testing/selftests/landlock/trace.h index e6873853376c..2ec863362173 100644 --- a/tools/testing/selftests/landlock/trace.h +++ b/tools/testing/selftests/landlock/trace.h @@ -145,8 +145,7 @@ "same_exec=[01] " \ "logged=[01] " \ "blockers=[a-z_|]* " \ - "sport=[0-9]\\+ " \ - "dport=[0-9]\\+$" + "port=-\\?[0-9]\\+$" #define REGEX_DENY_PTRACE(task) \ TRACE_PREFIX(task) \