macos: always pass -a "${VOLUME}" and -s "Nix Store"
#1236
Conversation
`/usr/bin/security {add,find,delete}-generic-password` accepts both `-a account` and `-s service` parameters as password lookup keys. Always pass `-a "${VOLUME}"` and `-s "Nix Store"`. With this change, installation succeeds even with `--encrypt` set to `true` and `--volume-label` set to any value deviating from `Nix Store`.
`/usr/bin/security {add,find,delete}-generic-password` accepts both `-a account` and `-s service` parameters as password lookup keys. Always pass `-a "${VOLUME}"` and `-s "Nix Store"`. With this change, installation succeeds even with `--encrypt` set to `true` and `--volume-label` set to any value deviating from `Nix Store`.
| @@ -258,7 +258,7 @@ async fn generate_mount_plist( | |||
| // The official Nix scripts uppercase the UUID, so we do as well for compatibility. | |||
| let uuid_string = uuid.to_string().to_uppercase(); | |||
| let mount_command = if encrypt { | |||
| let encrypted_command = format!("/usr/bin/security find-generic-password -s {apfs_volume_label_with_quotes} -w | /usr/sbin/diskutil apfs unlockVolume {apfs_volume_label_with_quotes} -mountpoint {mount_point:?} -stdinpassphrase"); | |||
| let encrypted_command = format!("/usr/bin/security find-generic-password -a {apfs_volume_label_with_quotes} -s \"Nix Store\" -w | /usr/sbin/diskutil apfs unlockVolume {apfs_volume_label_with_quotes} -mountpoint {mount_point:?} -stdinpassphrase"); | |||
There was a problem hiding this comment.
I wonder if instead we should pass the label here as well?
There was a problem hiding this comment.
Hmm I suppose your thinking here is the Service is "nix store" and the "account" is this particular store. That makes sense.
@cole-h I wonder if we need to similarly update where we create / store the password?
There was a problem hiding this comment.
Looking at the generic-password occurrences in plan() and execute(), they're already set up as proposed in this PR, so no change is necessary. This simply aligns the behavior of revert() and, arguably more importantly, generate_mount_plist(), so mounting the volume immediately after creating it actually works.
Passing the label for both is what revert() is doing prior to this change, so you could say there is precedent in either direction. I suspect it doesn't much matter which way the cat is skinned as long as it's consistent.
The explicit mention of Nix emphasizes that this isn't some generic encrypted volume, but one managed by Nix and nix-installer. It also looks a bit more polished in Keychain Access.
There shouldn't be any more generic-password calls in the tree unless GitHub search isn't as thorough as I imagine.
I hope this answers your questions!
Description
/usr/bin/security {add,find,delete}-generic-passwordaccepts both-a accountand-s serviceparameters as password lookup keys. Always pass-a "${VOLUME}"and-s "Nix Store". With this change, installation succeeds even with--encryptset totrueand--volume-labelset to any value deviating fromNix Store.Checklist
cargo fmtnix buildnix flake checkValidating with
install.determinate.systemsIf a maintainer has added the
upload to s3label to this PR, it will become available for installation viainstall.determinate.systems: