Pre-release hardening: secret access, user grants, rolling-upgrade heartbeats, removed-driver reporting - #1133
Merged
Merged
Conversation
…objects from the guests A guest of the root namespace read cluster.secret through the keyword endpoint of the cluster configuration, and through the object and instance file endpoints, which only asserted the guest role: only the cluster file endpoint required root. That secret is the key every sec and usr value is encrypted with. A guest of a namespace read the keys of its usr objects in clear: GetObjectDataKey required admin for a sec object and guest for every other kind, and a usr key is the password or the certificate private key of a user, which authenticate as that user. And the configuration endpoints served a guest the encrypted keys of the sec objects the key endpoint refused it, the ca private key the api signs its tokens with among them. configReadAccess now gates every configuration read, keyword, object file and instance file alike. A cluster configuration needs root, or the join grant of a node joining the cluster. Any other configuration is read by the guests of its namespace, with its secrets redacted: the keywords declared RedactSecret and the keys of the sec and usr objects show as asterisks, raw and evaluated, to a reader who is neither an admin of the namespace, root, nor joining. The admins keep the exact file, so an edit does not write the placeholders back, and a joining node gets the ca, certificate and heartbeat secrets it installs. A usr key is read by root only. object.IsSecretKey is the check RedactSecrets makes per key, shared with the keyword endpoint. It looks up the base option of a key, so a secret keyword set for one node is redacted too.
…mes one A start reserved an address from the network of the resource whether or not its name set the address to use. The resource plumbed the named address, and the reservation held another one nobody used, counted against the network claim of the namespace, until the resource stopped. The address is drawn only when the name is empty. A stop still releases whatever the resource holds, so the reservations earlier starts took by mistake go at the next stop.
…dentials only to who holds its grants An administrator of the system namespace, where the users live, could make itself root: create a user granted root, add root to a user, point the cn of a user at another's certificate, or reset the password of a root user. v2 refused the first three, and let the last one through. A write of a usr configuration now refuses the grants it adds that the writer does not hold, and a change of what the cn of a user evaluates to, unless the writer is root; a new user's cn must be its own name. The grants a user already holds are not asked for again, so an administrator still edits a user it could not have made, and takes grants away. A write of a key of a usr object, put, post, patch or delete, needs holding every grant of that user: the keys are the password and the certificate the user authenticates with, and setting them is taking its grants. It is asked on the node holding the user, where a proxied request keeps the identity of its caller. rbac.Grants.Covers says whether a set of grants holds a grant: root holds all, a role granted with no scope holds it on every namespace, a cluster role holds itself only.
…d document the drivers removed A section naming a driver this agent does not have, or a driver group it does not know, was skipped with a trace log. The object started and reported up without it: a v2 service holding its data on a disk.vxdg resource ran its applications without their storage, and nothing said so but a validation warning. The actor now records these sections as it configures the resources, and the instance status lists each one as an optional resource, not applicable, warning that its driver is not supported by this agent. The warning raises the overall status of the instance, and the availability is left alone: an instance going warn over it could trigger its monitor action, a node crash among them, for a resource that never ran. The sections are still not configured, so no action touches them and none is blocked. A subset section is no resource and is not reported. The CHANGELOG lists the drivers of v2.1 that have no v3 counterpart, the envoy ingress sections among them, and says how an upgraded object reports them. It also stops saying the vdisk, vmdg, vxdg and vxvol driver group names are replaced by disk drivers v3 does not have.
…the task container again The v2 container tasks took the keywords of the container drivers. The v3 ones declared no read_only, sysctl or stop_timeout, so a v2 task asking for a read-only root filesystem ran with a writable one, and only the configuration validation said so, as an unknown keyword. The three keywords are declared again on the task drivers, and passed to the container the task runs in. task.oci runs a docker or a podman task, so it has them too. umask and group stay on task.host: the v2 container tasks accepted them and applied neither.
…t, as v2 did The v2 policy kept for root the container keywords deciding what of the node a container reaches: a privileged container, the host network namespace, host devices, host path mounts, and the host network namespace of an ip resource. The v3 policy only kept the host path mounts of a container resource. The container and task groups now require the root grant for a true privileged, a netns of host, any devices, and a volume_mounts source that is a path of the node; the ip group for a netns of host. A task runs in a container like a container resource does, and is held to the same rules. The keyword documentation reads the same table, so it says so too.
…f a later version as unknown A heartbeat message holding one enum value the receiver did not know, as a monitor state added by a later version, failed to decode as a whole, and the receivers counted a peer alive only once its message decoded. During a rolling upgrade, the first such value an upgraded node published had every link drop it on the older nodes, which found it dead: the split action, or a failover of what it still ran, followed. The instance and node monitor states, local and global expects decode a value they do not know as a new unknown value, which marshals back. Status, provisioned and placement state decode it as undef, and the placement policy as invalid, which places nothing. A node in an unknown state is not ranked for placement. The unicast, multicast, relay and disk receivers count a peer alive once its message decrypts, from the sender they verified, and skip a message that does not decode, logging it once per peer and error, and again when it decodes again. The unicast and multicast receivers count only a peer of the heartbeat. imon and nmon do not adopt a global expect they do not know: they could not end their part of it, and the peers would wait for them for good. A node holding no orchestration id reads as done to them, so the nodes that know it finish it. The deep copy test fillers must round trip as themselves, now that an unknown value decodes rather than fails.
…d, not by the age its clock stamped The disk rx kept one last read time for all the peers, and read the slot of each in turn, so with two peers or more a slot unchanged since its last read was never seen as unchanged. What stopped a stale slot from counting was its age, measured as the time of this node since the update time the peer stamped with its own clock: a dead peer whose clock ran an hour ahead looked alive to the others for that hour. The rx keeps the update time last read of each peer, and counts a peer alive when its slot holds another one: a new time is a new write, whatever the clocks. The age of the write is judged on the first read only, which has nothing to compare with.
…ince the last read, not by the age the relay clock stamped The relay rx kept one last read time for all the peers, and read the message of each in turn, so with two peers or more a message unchanged since its last read was never seen as unchanged. What stopped a stale message from counting was its age, measured as the time of this node since the time the relay stored it, stamped by the relay clock: a relay whose clock ran ahead kept a dead peer looking alive. The rx keeps the store time last read of each peer, and counts a peer alive when the relay holds another one: the tx posts at every interval, changed or not, so a live peer is stored again before each read. The age of the store is judged on the first read only, which has nothing to compare with.
…elled hb log messages
… decode error, even unchanged
…ce to containerd/cgroups/v3
A process can't be matched by its env for a few milliseconds while it execve's: its /proc/<pid>/environ reads empty until the kernel sets the env_end bound of the new program memory, and, when the execve is called from a non-leader thread as Go programs do, the environ and stat opens fail with ESRCH or ENOENT while the kernel makes the exec'ing thread the thread group leader. The app.simple driver launches its start command through "om exec", which execve's in turn "env" then the application, and evaluates the status right after. When the status read hit one of these windows, the start reported the instance down with the application running, as seen in the QA "restart --rid app#0" test. Retry the environ read, with a 1ms delay and a 100ms deadline, while the stat of the process is consistent with an execve in progress. Kernel threads, zombies and processes rewriting their env area (nginx, redis) are not retried, so a process list walk keeps its cost.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the critical and high findings of the pre-release audit, and a few smaller issues found along the way. Each breaking change is recorded in CHANGELOG.md.
Security
Secrets kept from guests (879897e)
rootnamespace could readcluster.secret, the key every sec and usr value is encrypted with, through the cluster keyword and file endpoints.configReadAccesscheck now gates every configuration read. The cluster configuration needsroot, or thejoingrant of a joining node. Other configurations are readable by the namespace guests, with secret keywords and sec/usr keys shown as********to anyone who is not a namespace admin, root, or a joining node. A usr key can be read by root only.User grants (eb8cfb4)
systemnamespace could make itself root: create a root user, add root to a user, repoint a user'scn, or reset a root user's password.cnchange needsroot. Writing a usr key (password or certificate) needs every grant of that user. A new helper,rbac.Grants.Covers, checks these.Container and task rbac (751f73a): the v2 rules are back. Without root, a user may not set on a container or a task:
privilegedto truenetns=hostdevicesvolume_mountssource that is a host pathThe same goes for
netns=hoston an ip resource. Tasks are now held to the host path mounts rule too.Rolling upgrades
unknown,undeforinvalid. A peer counts as alive once its message decrypts, and a message that doesn't decode is logged once per peer.Heartbeats
Drivers
disk.vxdg) was skipped silently, and the object reported up without its storage.read_only,sysctlandstop_timeoutkeywords are declared again and passed to the task container. Before, a v2 task asking for a read-only root filesystem ran with a writable one.Dependencies
cilium/ebpfto v0.22.containerd/cgroupsv1.1.0 does not build against it, soutil/pgandcore/osagentservicemove tocontainerd/cgroups/v3, which was already a dependency.Testing
go build ./...andgo test ./...pass.