Hi @redsolver - your technical review is complete and some minor revisions have been requested before this milestone can be approved and the grant deemed complete:
- Fix the restore paths before sharing is used for real. Reject absolute paths and .. in filenames, and drop the setuid bits on restore.
- Add the two fuse fields so it builds on mac.
- Publish to crates.io. I noticed there is already a vup and vup-cli though, it’s an unrelated tool, and crates.io treats vup_cli and vup-cli as the same name, so he’ll have to deal with the naming conflict
- Make the upload timeout configurable. Right now it’s hardcoded, so a slow connection or a new account can’t finish a big backup without editing the code. On a bad connection I also had issues listing vault contents, on a good connection everything seems to work fine though, but increasing timeouts and maybe even retries might be worth it.
- And a question / item for clarification from your reviewer:
One thing I don’t really get is revocation. The design says a revoked device can’t come back by replaying an old record, but I couldn’t find that check. A running device is fine, the registry rejects the old record, but on a fresh recovery there’s nothing to compare against, so it looks like the old record gets accepted and the device is back in. Am I missing something? Enrollment feels similar, the one time code looks like the only check, so anyone who sees it while it’s valid can add their own device.
Let me know when I can inform the reviewer the revisions have been made.
1 Like
Thanks for the detailed review. All four revisions are in: PR #14 is merged to main (restore path fix, fuse fields for mac, better indexd defaults and more configurable options), and the crates are now published on crates.io with vup_cli being renamed to s5_vup: crates.io: Rust Package Registry (the CLI name is still just vup)
Upload timeout and listing. The whole-pack upload deadline is now configurable (upload_timeout_secs). I also added bounded retries to the indexer enumeration and the blob reads, and cut the default pack size from 240 MiB to 80 MiB, which was the real reason a slow uplink couldn’t finish a big backup. Listing on a bad connection should be solid now.
On revocation, you are correct: the only anti-rollback check is the registry’s monotone-revision rule, and a blank machine has nothing to compare against, so a rolled-back record is accepted. This is fundamental. A paper phrase carries no monotone state, so it cannot defeat rollback on its own. However, the paper recovery is designed to be used as a last resort if a user loses (physical) access to all of their devices, while the usual approach with a revoked device is that the user still has other devices which actively rotate secrets etc. So my plan here revolves mostly around making this process and architecture more intuitive to understand for users, which is mostly a user interface engineering problem. I am also looking into making malicious registry rollbacks harder by using multiple independent sources and always using the highest found one (so for example use both the user’s indexd account and a public registry server).
And yes, the one-time code is the only check for device enrollment, so if someone sees it during the window they could enroll their own keys. Although it is visible to the user when the enrollment occurs (or fails on the other device), something like mandatory inviter-side fingerprint confirmation would still make everything a lot more secure and I’m currently experimenting with approaches for that.
Thanks for this explanation @redsolver and for those final items. Your technical review is a success and this grant can now be marked as complete. Congratulations!
1 Like