The sound distributed version control system

#706 Improved identity management

Closed on June 3, 2023
finchie on August 16, 2022

I have adapted the changes from #673 for a simpler review process. The main goal was to remove the common foot-gun of identity management while also allowing for power-users to have more complex setups. Some highlights of these changes are as follows:

  • Support for multiple identities throughout the codebase
  • Commands to add, edit, delete, list, repair identities
  • Simplified handling of push/pull over SSH
  • Test suite of all the most common operations

I am more than happy to make any changes required. Please let me know what I can do to help make reviewing easier!

finchie added a change on August 16, 2022
4EN4MDBQC3DDMYWY7GNCOR2CUTNQJSKKBUSAH7OTJGNO6FBPY55QC
main
finchie added a change on August 16, 2022
FVQYZQFL7WHSC3UUPJ4IWTP7SKTDQ4K6K5HY4EDK3JKXG3CQNZEAC
main
finchie added a change on August 16, 2022
4KJ45IJLTIE35KQZUSFMFS67RNENG4P2FZMKMULJLGGYMKJUVRSQC
main
finchie added a change on August 16, 2022
DWSAYGVEOR4D2EKIICEZUWCRGJTUXQQLOUWMYIFV7XN62K44F4FAC
main
finchie added a change on August 16, 2022
TI7PCK7JLPU4KYE65XIMBUPPY7DRPVDAETPDFMG3VAQGGDRGQHPQC
main
finchie added a change on August 16, 2022
FOCBVLOUXYA7ZCUZA2CU3JU2QGF3ZOXW6EAVL5KZINN43GXNL7CQC
main
joyously on August 16, 2022

As stated on Zulip, I got a conflict on Cargo.lock and libpijul/src/tests/mod.rs on the pull of these changes. Running the tests with cargo test --package pijul, I got

  Compiling pijul v1.0.0-beta.2 (/media/bigdisk/Data/_working/contrib/pijul/pijul)
error[E0658]: `let` expressions in this position are unstable
  --> pijul/tests/common/mod.rs:90:12
   |
90 |         if let Some(invalid) = self.invalid_input() && !valid {
   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: see issue #53667 <https://github.com/rust-lang/rust/issues/53667> for more information

error[E0658]: `let` expressions in this position are unstable
   --> pijul/tests/common/identity.rs:292:12
    |
292 |         if let Some(password) = self.password.clone() && valid {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #53667 <https://github.com/rust-lang/rust/issues/53667> for more information

For more information about this error, try `rustc --explain E0658`.
error: could not compile `pijul` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error[E0658]: use of unstable library feature 'backtrace'
   --> pijul/src/main.rs:149:49
    |
149 |         log::error!("Error backtrace: {:#?}", e.backtrace());
    |                                                 ^^^^^^^^^
    |
    = note: see issue #53487 <https://github.com/rust-lang/rust/issues/53487> for more information

error: could not compile `pijul` due to previous error
error: could not compile `pijul` due to previous error
finchie added a change on August 25, 2022
SV2PROHVQ5R267ORJVOYZ4RGIEFH6TYM2OFNEBAOL6ZTTVJ6LISQC
finchie on August 25, 2022

@joyously thanks for picking up on this, I was accidentally using a nightly toolchain. The latest patch should resolve these errors :)

finchie added a change on September 22, 2022
4OJWMSOWWNT5N4W4FDMKBZB5UARCLGV3SRZVKGR4EFAYFUMUHM7AC
main
finchie added a change on September 25, 2022
AFZKABZT5EZDS4MUA4UB572ZYMMVTXDOITDBHVF4XO7UGAJJE5FAC
finchie added a change on September 25, 2022
KKNMDXAIU7P44JOSAM23T4RUMLBHWGHTQQS4NXS4FIX5IN6OTXPAC
main
finchie added a change on September 26, 2022
6FRPUHWKBAWIYN6B6YDFQG2SFWZ6MBBYOYXFUN6DRZ4HPDSKFANQC
main
finchie added a change on September 28, 2022
Make bin_diff_test compile again by multun, created on September 9, 2022
DSLBAEDTSUX3IPCX3BNHDLPIKLHIZABP6XL37MDSMNREASVOPZSAC
main
finchie added a change on September 28, 2022
Fix zombie in libpijul tests created on September 26, 2022
DHMXWWMSWJT5EJK4PNCG46ETUJ4CMBWBQXIHCKRCNUUH2ULEFQDAC
main
finchie added a change on September 28, 2022
K3INLJSYZQKG4UDISY4MS76QN6V7H5ZUHS6HQBSTVFX2BBPAK26QC
finchie on September 28, 2022

Whoops, accidentally pushed patches from #713

pmeunier on October 29, 2022

Finally got to review this! I’m really happy that I finally got some time, this looks like great work.

It seems #K3INLJSYZQKG4UDISY4MS76QN6V7H5ZUHS6HQBSTVFX2BBPAK26QC still depends on #DHMXWWMSWJT5EJK4PNCG46ETUJ4CMBWBQXIHCKRCNUUH2ULEFQDAC , and I’m unable to download it (probably due to a bug in the Nest), is that intended?

finchie on November 10, 2022

Sorry, that’s my bad, I think there’s an implicit dependency on #713. I pushed the patches to this one by accident but they were applied while resolving some issues with compiling the tests. Technically I don’t think it’s required, so I’ll remove the dependency in the patch and resubmit.

stel on November 16, 2022

i don’t use d-bus or the linux secret service api, and the use of keyring::Entry in these patches crashes when it fails to connect to d-bus

~/conf/pijul/identities $ pj identity repair
Password does not match secret key
✔ Password for secret key · ********
Error: Platform secure storage failure: zbus error: I/O error: No such file or directory (os error 2)
finchie on November 26, 2022

@stel thanks for testing! This should be related to #729, the patch submitted there should fix your issue.

jcdickinson on December 7, 2022

It looks like it’s trying to connect using my Linux username, irrespective of what I set the identity username to (Remote username vs Password for):

[jono@jono-desktop pijul]$ pijul identity new
# ...
✔ Remote username · jcdickinson
✔ Remote URL · ssh.pijul.com
✔ Do you want to change the default SSH key? (Current key: none) · no
Linking identity jcdickinson@ssh.pijul.com
Warning: Unable to automatically authenticate with server. Please make sure your SSH keys have been uploaded to the Nest.
For more information, please visit https://pijul.org/manual/the_nest/public_keys.html#ssh-public-keys
✔ Password for jono@ssh.pijul.com · ********
Error: Could not prove identity default. Please check your credentials & network connection. If you are on an enterprise network, perhaps try running with `--no-cert-check`

If I set the remote URL to jcdickinson@ssh.pijul.com it works, but does this strange thing:

✔ Remote username · jcdickinson
✔ Remote URL · jcdickinson@ssh.pijul.com
✔ Do you want to change the default SSH key? (Current key: none) · no
Linking identity jcdickinson@jcdickinson@ssh.pijul.com
finchie on December 18, 2022

Thanks @jcdickinson for giving this a shot. I’ll open two discussions to resolve your issues, but here are some preliminary thoughts on both:

  1. There is a logic error in the code which I didn’t catch due to my username being the same on Pijul and Linux. Should be easy to fix!
  2. The tool was expecting ssh.pijul.com as the input in this case, but obviously this wasn’t communicated very well. This could be solved in a few different ways, but I think the best solution would be to clarify the add some kind of input validation so the tool doesn’t confuse other users in the same way.

I will ping you in the relevant discussions when patches have been created to resolve :)

stel on December 23, 2022

@finchie it does not, unfortunately. i do not use d-bus.

~/w/env/pijul $ pj identity repair
Password does not match secret key
✔ Password for secret key · ********
Error: Platform secure storage failure: zbus error: I/O error: No such file or directory (os error 2)

i was able to get past this with this change

1. Replacement in "pijul/src/identity/mod.rs":147 3.15627 "UTF-8"
B:BD 2.4743 -> 2.4743:4824/2
  up 2.4743, new 24:108, down 2.4824
-             keyring::Entry::new("pijul", name).set_password(&password_attempt)?;
+             _ = keyring::Entry::new("pijul", name).set_password(&password_attempt);

but i had further issues.

~/w/env/pijul $ pj identity repair
Password does not match secret key
✔ Password for secret key · ********
It seems you have configured an identity in an older version of Pijul, which uses an older identity format!
Please take a moment to confirm your details are correct.
✔ Unique identity name · default
✔ Display name · stel
✔ Email (leave blank for none) · stel@comfy.monster
✔ Do you want to change the encryption? (Current status: encrypted) · no
✔ Do you want this key to expire? (Current expiry: never) · no
✔ Do you want to link this identity to a remote? · yes
✔ Remote username · stel
✔ Remote URL · ssh.pijul.com
✔ Do you want to change the default SSH key? (Current key: none) · yes
Error: Environment variable `SSH_AUTH_SOCK` not found
~/w/env/pijul $ pj identity repair
Password does not match secret key
✔ Password for secret key · ********
It seems you have configured an identity in an older version of Pijul, which uses an older identity format!
Please take a moment to confirm your details are correct.
✔ Unique identity name · default
✔ Display name · stel
✔ Email (leave blank for none) · stel@comfy.monster
✔ Do you want to change the encryption? (Current status: encrypted) · no
✔ Do you want this key to expire? (Current expiry: never) · no
✔ Do you want to link this identity to a remote? · yes
✔ Remote username · stel
✔ Remote URL · ssh.pijul.com
✔ Do you want to change the default SSH key? (Current key: none) · no
Linking identity stel@ssh.pijul.com
Warning: Unable to automatically authenticate with server. Please make sure your SSH keys have been uploaded to the Nest.
For more information, please visit https://pijul.org/manual/the_nest/public_keys.html#ssh-public-keys
✔ Password for stel@ssh.pijul.com · ********
Warning: could not write new password to keychain: Platform secure storage failure: zbus error: I/O error: No such file or directory (os error 2)
Password does not match secret key
✔ Password for secret key · ********
Password does not match secret key
✔ Password for secret key · ********
~/w/env/pijul $ pj id list
Identities
└─ default
   ├─ Display name: stel
   ├─ Email: stel@comfy.monster
   ├─ Login: stel@ssh.pijul.com
   ├─ Public key
   │  ├─ Key: E6ZtcwtTWvdkbojLFLBbcEY4CeU9mt76ntjCMNNi9vF5
   │  ├─ Version: 0
   │  ├─ Algorithm: Ed25519
   │  └─ Expiry: Never
   ├─ Secret key
   │  ├─ Version: 0
   │  ├─ Algorithm: Ed25519
   │  └─ Encryption: AES 128-bit (Stored in keyring: false)
   └─ Last updated: 2022-12-23 17:44:38 (UTC)
~/w/env/pijul $ pj id edit
Editing identity: default
✔ Unique identity name · stel
✔ Display name · stel
✔ Email (leave blank for none) · stel@comfy.monster
✔ Do you want to change the encryption? (Current status: not encrypted) · no
? Do you want this key to expire? (Current expiry: never) (y/n) › no
~/w/env/pijul $ pj id edit
Editing identity: default
✔ Unique identity name · default
? Display name › stel
~/w/env/pijul $ pj id edit
Editing identity: default
✔ Unique identity name · stel
✔ Display name · stel
✔ Email (leave blank for none) · stel@comfy.monster
✔ Do you want to change the encryption? (Current status: not encrypted) · yes
✔ New password · ********
Error: Platform secure storage failure: zbus error: I/O error: No such file or directory (os error 2)

i do not have ssh-agent either. additionally i have no idea which encryption it’s asking me to change, or why it changed from encrypted to not encrypted when i said i did not want it to change.

stel on December 23, 2022

there are at least three places where set_password is still unwrapped unsafely

message = ''
timestamp = '2022-12-23T18:17:55.257103304Z'
authors = []

# Dependencies
[2] 4OJWMSOWWNT5N4W4FDMKBZB5UARCLGV3SRZVKGR4EFAYFUMUHM7AC # Fully replace crate::Identity
[3]+4KJ45IJLTIE35KQZUSFMFS67RNENG4P2FZMKMULJLGGYMKJUVRSQC # Implement new identity management
[*] 4KJ45IJLTIE35KQZUSFMFS67RNENG4P2FZMKMULJLGGYMKJUVRSQC # Implement new identity management

# Hunks

1. Replacement in "pijul/src/identity/mod.rs":147 3.15627 "UTF-8"
B:BD 2.4743 -> 2.4743:4824/2
  up 2.4743, new 24:108, down 2.4824
-             keyring::Entry::new("pijul", name).set_password(&password_attempt)?;
+             _ = keyring::Entry::new("pijul", name).set_password(&password_attempt);

2. Replacement in "pijul/src/identity/mod.rs":266 3.15627 "UTF-8"
B:BD 2.7658 -> 2.7658:7742/2
  up 2.7658, new 109:196, down 2.7742
-             keyring::Entry::new("pijul", &self.name).set_password(&user_password)?;
+             _ = keyring::Entry::new("pijul", &self.name).set_password(&user_password);

3. Replacement in "pijul/src/identity/create.rs":366 3.28175 "UTF-8"
:D 2.14270 -> 3.40520:40598/2, B:BD 3.40520 -> 3.40520:40598/3
  up 2.14270, new 199:280, down 3.40598
-             Entry::new("pijul", &new_identity.name).set_password(&password)?;
+             _ = Entry::new("pijul", &new_identity.name).set_password(&password);
stel on December 25, 2022

when i try to edit the identity, it seems to overwrite it with a new key, which seems to be why it always says it’s not encrypted.

pmeunier on February 23, 2023

@finchie: I don’t think #K3INLJSYZQKG4UDISY4MS76QN6V7H5ZUHS6HQBSTVFX2BBPAK26QC is needed anymore, Pijul with your patches applied builds on stable Rust.

Is there still work to do in this discussion?

finchie on June 3, 2023

I don’t think there is any reason to keep this tracking discussion open, as any particular issues would probably be suited to a new discussion.

finchie closed this discussion on June 3, 2023