From 36dbd3b97dbc455c6d710f4530ec8a07474dee64 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Wed, 14 Apr 2021 10:23:58 -0700 Subject: Automatically enforce limited netstack dependencies Netstack is supposed to be somewhat independent of the rest of gVisor, and others should be able to use it without pulling in excessive dependencies. Currently, there is no way to fight dependency creep besides careful code review. This change introduces a test rule `netstack_deps_check` that ensures the target only relies on gVisor targets and a short allowlist of external dependencies. Users who add a dependency will see an error and have to manually update the allowlist. The set of packages to test comes from //runsc, as it uses packages we would expect users to commonly rely on. It was generated via: $ find ./runsc -name BUILD | xargs grep tcpip | awk '{print $2}' | sort | uniq (Note: We considered giving //pkg/tcpip it's own go.mod, but this breaks go tooling.) PiperOrigin-RevId: 368456711 --- pkg/tcpip/BUILD | 31 +++++++++++++++ tools/BUILD | 8 ++++ tools/deps.bzl | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+) create mode 100644 tools/deps.bzl diff --git a/pkg/tcpip/BUILD b/pkg/tcpip/BUILD index f979d22f0..aa30cfc85 100644 --- a/pkg/tcpip/BUILD +++ b/pkg/tcpip/BUILD @@ -1,4 +1,5 @@ load("//tools:defs.bzl", "go_library", "go_test") +load("//tools:deps.bzl", "deps_test") load("//tools/go_generics:defs.bzl", "go_template_instance") package(licenses = ["notice"]) @@ -33,6 +34,36 @@ go_library( ], ) +deps_test( + name = "netstack_deps_test", + allowed = [ + "@com_github_google_btree//:go_default_library", + "@org_golang_x_sys//unix:go_default_library", + "@org_golang_x_time//rate:go_default_library", + ], + allowed_prefixes = [ + "//", + "@org_golang_x_sys//internal/unsafeheader", + ], + targets = [ + "//pkg/tcpip", + "//pkg/tcpip/header", + "//pkg/tcpip/link/fdbased", + "//pkg/tcpip/link/loopback", + "//pkg/tcpip/link/packetsocket", + "//pkg/tcpip/link/qdisc/fifo", + "//pkg/tcpip/link/sniffer", + "//pkg/tcpip/network/arp", + "//pkg/tcpip/network/ipv4", + "//pkg/tcpip/network/ipv6", + "//pkg/tcpip/stack", + "//pkg/tcpip/transport/icmp", + "//pkg/tcpip/transport/raw", + "//pkg/tcpip/transport/tcp", + "//pkg/tcpip/transport/udp", + ], +) + go_test( name = "tcpip_test", size = "small", diff --git a/tools/BUILD b/tools/BUILD index faf310676..3861ff2a5 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -9,3 +9,11 @@ bzl_library( "//:sandbox", ], ) + +bzl_library( + name = "deps_bzl", + srcs = ["deps.bzl"], + visibility = [ + "//:sandbox", + ], +) diff --git a/tools/deps.bzl b/tools/deps.bzl new file mode 100644 index 000000000..ed1135a9e --- /dev/null +++ b/tools/deps.bzl @@ -0,0 +1,114 @@ +"""Rules for dependency checking.""" + +# DepsInfo provides a list of dependencies found when building a target. +DepsInfo = provider( + "lists dependencies encountered while building", + fields = { + "nodes": "a dict from targets to a list of their dependencies", + }, +) + +def _deps_check_impl(target, ctx): + # Check the target's dependencies and add any of our own deps. + deps = [] + for dep in ctx.rule.attr.deps: + deps.append(dep) + nodes = {} + if len(deps) != 0: + nodes[target] = deps + + # Keep and propagate each dep's providers. + for dep in ctx.rule.attr.deps: + nodes.update(dep[DepsInfo].nodes) + + return [DepsInfo(nodes = nodes)] + +_deps_check = aspect( + implementation = _deps_check_impl, + attr_aspects = ["deps"], +) + +def _is_allowed(target, allowlist, prefixes): + # Check for allowed prefixes. + for prefix in prefixes: + workspace, pfx = prefix.split("//", 1) + if len(workspace) > 0 and workspace[0] == "@": + workspace = workspace[1:] + if target.workspace_name == workspace and target.package.startswith(pfx): + return True + + # Check the allowlist. + for allowed in allowlist: + if target == allowed.label: + return True + + return False + +def _deps_test_impl(ctx): + nodes = {} + for target in ctx.attr.targets: + for (node_target, node_deps) in target[DepsInfo].nodes.items(): + # Ignore any disallowed targets. This generates more useful error + # messages. Consider the case where A dependes on B and B depends + # on C, and both B and C are disallowed. Avoid emitting an error + # that B depends on C, when the real issue is that A depends on B. + if not _is_allowed(node_target.label, ctx.attr.allowed, ctx.attr.allowed_prefixes) and node_target.label != target.label: + continue + bad_deps = [] + for dep in node_deps: + if not _is_allowed(dep.label, ctx.attr.allowed, ctx.attr.allowed_prefixes): + bad_deps.append(dep) + if len(bad_deps) > 0: + nodes[node_target] = bad_deps + + # If there aren't any violations, write a passing test. + if len(nodes) == 0: + ctx.actions.write( + output = ctx.outputs.executable, + content = "#!/bin/bash\n\nexit 0\n", + ) + return [] + + # If we're here, we've found at least one violation. + script_lines = [ + "#!/bin/bash", + "echo Invalid dependencies found. If you\\'re sure you want to add dependencies,", + "echo modify this target.", + "echo", + ] + + # List the violations. + for target, deps in nodes.items(): + script_lines.append( + 'echo "{target} depends on:"'.format(target = target.label), + ) + for dep in deps: + script_lines.append('echo "\t{dep}"'.format(dep = dep.label)) + + # The test must fail. + script_lines.append("exit 1\n") + + ctx.actions.write( + output = ctx.outputs.executable, + content = "\n".join(script_lines), + ) + return [] + +# Checks that library and its deps only depends on gVisor and an allowlist of +# other dependencies. +deps_test = rule( + implementation = _deps_test_impl, + attrs = { + "targets": attr.label_list( + doc = "The targets to check the transitive dependencies of.", + aspects = [_deps_check], + ), + "allowed": attr.label_list( + doc = "The allowed dependency targets.", + ), + "allowed_prefixes": attr.string_list( + doc = "Any packages beginning with these prefixes are allowed.", + ), + }, + test = True, +) -- cgit v1.2.3