flux-core
flux-core copied to clipboard
flux-framework
consider validating guest KVS metadata on job completion
Problem: when a job completes, flux_kvs_copy() is used to add the guest KVS namespace to the primary namespace to become part of the job record when the job transitions to INACTIVE. This operation does not actually traverse the KVS metadata in the namespace, but instead simply links the root blobref of the guest namespace to a new key in the primary namespace. Since the KVS allows any RFC 11 treeobj to be supplied by the user in a commit, it is possible for a user to create various metadata problems in the primary namespace.
We have tests to ensure dangling blobrefs and so forth do not cause internal problems and are propagated to the user on kvs lookup (see t1001-kvs-internals.t) but the fact that there could be kvs lookup errors propagated to callers may not have been adequately audited, and the proposed flux-dump(1) command could potentially encounter a fatal error or enter a cycle.
We should probably sanitize the guest namespace metadata before linking it into the primary namespace in job-exec. That would include detecting cycles that a user might introduce maliciously or otherwise.
In addition, flux-dump(1) probably ought to simply report certain metadata errors instead of treating them as fatal, and be able to detect and prune cycles that would cause it to run forever.
We should probably sanitize the guest namespace metadata before linking it into the primary namespace in job-exec. That would include detecting cycles that a user might introduce maliciously or otherwise.
If detected, what could we do? Thinking about it for a few seconds, perhaps we can substitute the badness with something indicating things were left bad at job exit. Like a valref pointing to a bad/illegal blobref could be pointed to a new blobref that just says "invalid valref". And something that would end up in a ELOOP error could be pointed to some generally global "eloop ocurred" kind of error. And these error blobrefs could be easily archived.
Another idea, perhaps writing a treeobj should be limited to instance owners, not guest users?
Another idea, perhaps writing a treeobj should be limited to instance owners, not guest users?
Yes. I'll open an issue on that one. That would tighten things up considerably I think.
And something that would end up in a ELOOP error could be pointed to some generally global "eloop ocurred" kind of error. And these error blobrefs could be easily archived.
I was thinking about dirref cycles, where a dirref points to a blobref that is a parent directory. Now that I think about it, that would change the parent directory (since all hashes up the tree to the root change). Maybe it's not possible to create a cycle like that since you would have to be able to predict the hash of the directory containing its own hash. Ouch, brain pain!