timvisee / prs

🔐 A secure, fast & convenient password manager CLI using GPG and git to sync.
https://gitlab.com/timvisee/prs
GNU General Public License v3.0
211 stars 8 forks source link

prs do not strip comments of gpg_id file #22

Open shemeshg opened 1 year ago

shemeshg commented 1 year ago

in pass we have (line 101)

    local gpg_id
    while read -r gpg_id; do
        gpg_id="${gpg_id%%#*}" # strip comment
        [[ -n $gpg_id ]] || continue
        GPG_RECIPIENT_ARGS+=( "-r" "$gpg_id" )
        GPG_RECIPIENTS+=( "$gpg_id" )
    done < "$current"

howeverprs consider the complete line as UserId and no whatever before the ' # ' only

To Reproduce

Steps to reproduce the behavior:

  1. Add remarks to gpg_id entry in the .gpg_id file

Something like:

21347213469hdsaklfha # username <username@whatever.com>
  1. ~/.cargo/bin/prs edit ali

3, Change something

  1. save and exit

prs, will now say it can not find the ID (After inspecting with export RUST_BACKTRACE=1)

Expected behavior

prs should not care if entry has ramarks or not.

if it helps

in my implementation of pass pass simple I've used simply

Pseudocode

line.split(" ")[0]

Sorry I'm not a Rust developer and I can not just submit PR, but I can gladly test and confirm problem solved after it will be fixed.

timvisee commented 1 year ago

Thanks for your report!

I cannot reproduce this myself, but did add logic for stripping comments from fingerprints.

Would you mind to compile from source and give the latest commit from master a spin to see if that fixed it?

shemeshg commented 1 year ago

The bad news

It did not worked, While pass Does work

ubuntu@ubuntu:~/Documents/prs$ prs add jojo
thread 'main' panicked at 'attempting to encrypt secret for empty list of recipients', lib/src/crypto/backend/gnupg_bin/raw.rs:29:5
stack backtrace:
   0: std::panicking::begin_panic
   1: prs_lib::crypto::backend::gnupg_bin::raw::encrypt
   2: <prs_lib::crypto::backend::gnupg_bin::context::Context as prs_lib::crypto::IsContext>::encrypt
   3: <prs_lib::crypto::Context as prs_lib::crypto::IsContext>::encrypt
   4: prs_lib::crypto::IsContext::encrypt_file
   5: prs::action::add::Add::invoke
   6: prs::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
ubuntu@ubuntu:~/Documents/prs$ 
ubuntu@ubuntu:~/Documents/prs$ export RUST_BACKTRACE=full
ubuntu@ubuntu:~/Documents/prs$ prs add jojo
thread 'main' panicked at 'attempting to encrypt secret for empty list of recipients', lib/src/crypto/backend/gnupg_bin/raw.rs:29:5
stack backtrace:
   0:     0x55b5e05cd723 - std::backtrace_rs::backtrace::libunwind::trace::he0156af2558114c2
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x55b5e05cd723 - std::backtrace_rs::backtrace::trace_unsynchronized::h1e2672bcf5105eb5
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55b5e05cd723 - std::sys_common::backtrace::_print_fmt::haa919a14d8d859ec
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/sys_common/backtrace.rs:66:5
   3:     0x55b5e05cd723 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hf15e8f9e6884dd5f
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/sys_common/backtrace.rs:45:22
   4:     0x55b5e05f22cc - core::fmt::write::he42254d9e3c27115
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/core/src/fmt/mod.rs:1202:17
   5:     0x55b5e05b0925 - std::io::Write::write_fmt::hfb37e0ab3a125c66
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/io/mod.rs:1679:15
   6:     0x55b5e05b4ee4 - std::sys_common::backtrace::_print::h8078bdb0e2e92b53
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/sys_common/backtrace.rs:48:5
   7:     0x55b5e05b4ee4 - std::sys_common::backtrace::print::h09fd65486fb9c4f7
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/sys_common/backtrace.rs:35:9
   8:     0x55b5e05b4ee4 - std::panicking::default_hook::{{closure}}::hb89b98c578903f40
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/panicking.rs:295:22
   9:     0x55b5e05b4b24 - std::panicking::default_hook::h27aa44be03b01ac8
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/panicking.rs:314:9
  10:     0x55b5e05b54c3 - std::panicking::rust_panic_with_hook::he7013d2ea706cde0
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/panicking.rs:698:17
  11:     0x55b5e049e0cb - std::panicking::begin_panic::{{closure}}::hb487e87eb893767c
  12:     0x55b5e049e096 - std::sys_common::backtrace::__rust_end_short_backtrace::hd5332253a16cb100
  13:     0x55b5e0091696 - std::panicking::begin_panic::hd16e9d6dc6e717ec
  14:     0x55b5e0495716 - prs_lib::crypto::backend::gnupg_bin::raw::encrypt::h36d208c434445674
  15:     0x55b5e04725ec - <prs_lib::crypto::backend::gnupg_bin::context::Context as prs_lib::crypto::IsContext>::encrypt::hdfe030a1e4101bf3
  16:     0x55b5e0488fe9 - <prs_lib::crypto::Context as prs_lib::crypto::IsContext>::encrypt::h9b12d24c662bc27b
  17:     0x55b5e00ba86b - prs_lib::crypto::IsContext::encrypt_file::had575660eb888075
  18:     0x55b5e00b27cd - prs::action::add::Add::invoke::he6fc078e78281f95
  19:     0x55b5e00ad291 - prs::main::h0596a5f99ed2ce8e
  20:     0x55b5e0104843 - std::sys_common::backtrace::__rust_begin_short_backtrace::h0fc3fa954599b274
  21:     0x55b5e00b5699 - std::rt::lang_start::{{closure}}::hd546ee8bb9b87dc4
  22:     0x55b5e05afd58 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h62af992155415807
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/core/src/ops/function.rs:283:13
  23:     0x55b5e05afd58 - std::panicking::try::do_call::hcfafbbba7d4f6a6c
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/panicking.rs:492:40
  24:     0x55b5e05afd58 - std::panicking::try::h7ee1bfcad42abe9b
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/panicking.rs:456:19
  25:     0x55b5e05afd58 - std::panic::catch_unwind::h009aa132eb8dd7d2
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/panic.rs:137:14
  26:     0x55b5e05afd58 - std::rt::lang_start_internal::{{closure}}::h93ae259af980c7d0
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/rt.rs:148:48
  27:     0x55b5e05afd58 - std::panicking::try::do_call::ha0627c997265a210
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/panicking.rs:492:40
  28:     0x55b5e05afd58 - std::panicking::try::h64a0afc1377cc785
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/panicking.rs:456:19
  29:     0x55b5e05afd58 - std::panic::catch_unwind::h1c61ea510b397b89
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/panic.rs:137:14
  30:     0x55b5e05afd58 - std::rt::lang_start_internal::ha75927e1903320fe
                               at /build/rustc-Oic09u/rustc-1.65.0+dfsg0ubuntu1/library/std/src/rt.rs:148:20
  31:     0x55b5e00aec68 - main
  32:     0x7f0be870cd90 - __libc_start_call_main
                               at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  33:     0x7f0be870ce40 - __libc_start_main_impl
                               at ./csu/../csu/libc-start.c:392:3
  34:     0x55b5e00967d5 - _start
  35:                0x0 - <unknown>
ubuntu@ubuntu:~/Documents/prs$ 

the good news, I've tested the code that filters and it filters right

Created simple project with

ubuntu@ubuntu:~/Documents/test/shalom/src$ cat main.rs
use std::fs;
use std::path::{Path};

fn read_fingerprints<P: AsRef<Path>>(path: P) -> Vec<String> {
    fs::read_to_string(path)       
        .expect("Failed to read input")
        .lines()
        // Strip comments
        .map(|fp| match fp.split_once('#') {
            Some((fp, _)) => fp,
            None => fp,
        })
        .map(|fp| fp.trim())
        .filter(|fp| !fp.is_empty())
        .map(Into::into)
        .collect()
}

fn main() {

    // Stores the iterator of lines of the file in lines variable.
    let lines = read_fingerprints("/home/ubuntu/.password-store/.gpg-id".to_string());
    // Iterate over the lines of the file, and in this case print them.
    for line in lines {
        println!("***{}***", line);
    }

}ubuntu@ubuntu:~/Documents/test/shalom/src$ 

The file content of /home/ubuntu/.password-store/.gpg-id

}ubuntu@ubuntu:~/Documents/test/shalom/src$ cat /home/ubuntu/.password-store/.gpg-id
800421D3A4728717 # albert alban <albert@microsoft.com>
ubuntu@ubuntu:~/Documents/test/shalom/src$ 

Got output of

ubuntu@ubuntu:~/Documents/test/shalom/target/debug$ ./shalom 
***800421D3A4728717***
ubuntu@ubuntu:~/Documents/test/shalom/target/debug$ 

The question is

  1. It seems to ignore the content of this row completely, is it intended.
  2. I've tried to debug but cargo build failed with
buntu@ubuntu:~/Documents/prs/cli$ cargo build 
   Compiling rustix v0.36.7
   Compiling form_urlencoded v1.1.0
   Compiling idna v0.3.0
   Compiling chrono v0.4.23
error[E0554]: `#![feature]` may not be used on the stable release channel
  --> /home/ubuntu/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.36.7/src/lib.rs:99:26
   |
99 | #![cfg_attr(rustc_attrs, feature(rustc_attrs))]
   |                          ^^^^^^^^^^^^^^^^^^^^

error[E0554]: `#![feature]` may not be used on the stable release channel
   --> /home/ubuntu/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.36.7/src/lib.rs:116:5
    |
116 |     feature(core_intrinsics)
    |     ^^^^^^^^^^^^^^^^^^^^^^^^

   Compiling async-io v1.12.0
error[E0554]: `#![feature]` may not be used on the stable release channel
   --> /home/ubuntu/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.36.7/src/lib.rs:116:13
    |
116 |     feature(core_intrinsics)
    |             ^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0554`.
error: could not compile `rustix` due to 3 previous errors
warning: build failed, waiting for other jobs to finish...
ubuntu@ubuntu:~/Documents/prs/cli$

So I'm unable to put breaking points with vscode

  1. I can put println for you in the code if you'd ask me to.

Thanks shemeshg

shemeshg commented 1 year ago

Ok I got this,

1, Yes we solved the problem of the remarks

  1. We have different issue where prs search keys by full id only

This will not work

ubuntu@ubuntu:~/.password-store$ gpg --list-keys
/home/ubuntu/.gnupg/pubring.kbx
-------------------------------
pub   rsa3072 2023-02-17 [SC]
      E68783F3D26DBF30EEA62C6A800421D3A4728717
uid           [ultimate] albert alban <albert@microsoft.com>
sub   rsa3072 2023-02-17 [E]

ubuntu@ubuntu:~/.password-store$ cat .gpg-id
800421D3A4728717
ubuntu@ubuntu:~/.password-store$ 

This will work

ubuntu@ubuntu:~/.password-store$ cat .gpg-id
E68783F3D26DBF30EEA62C6A800421D3A4728717 # albert alban <albert@microsoft.com>
ubuntu@ubuntu:~/.password-store$ 

What I thought would happen

  1. prs will search using something like

Example from cpp pgpme project https://github.com/shemeshg/gpgme-cmake-example/blob/b0a9cad212e22779547da1e7aeb99c06b715162d/libpgpfactory/libpgpfactory.cpp

std::vector<GpgKeys> GpgFactory::listKeys(const std::string pattern, bool secret_only)
{
    std::vector<GpgKeys> retKeys = {};

    checkCtxInitialized();
    gpgme_key_t key = nullptr;
    gpgme_error_t err = gpgme_op_keylist_start(ctx, pattern.c_str(), secret_only);
    while (!err)
    {
        err = gpgme_op_keylist_next(ctx, &key);
        if (err)
            break;
        GpgKeys k;
        k.foundUsingPattern=pattern;
        setGpgKeysFromGpgme_key_t(k, key, retKeys);
        gpgme_key_unref(key);
    }

    if (gpg_err_code(err) != GPG_ERR_EOF)
    {
        std::throw_with_nested(std::runtime_error(gpgme_strerror(err)));
    }
    return retKeys;
}

Screenshot example from c++ debugger

image

As you can see in gpgme there is keyId (short notation) and fpr (long notation)

Question

timvisee commented 1 year ago

The code you've suggested is exactly the same as what happens internally, for stripping remarks off the fingerprints.

What happens when you run the following?

prs housekeeping sync-keys

Also, do you have the same issues when running prs with different backends?

cargo run --no-default-features --features backend-gnupg-bin -- 
cargo run --no-default-features --features backend-gpgme -- 
shemeshg commented 1 year ago

Hope I've done it correctly, the short id notation fails to sync

The short notation to produce in browser terminal from long notation might be

let a= "E68783F3D26DBF30EEA62C6A800421D3A4728717"
undefined
a.substr(-16)
'800421D3A4728717'
ubuntu@ubuntu:~/.password-store$ export RUST_BACKTRACE=1
ubuntu@ubuntu:~/.password-store$ cat .gpg-id
800421D3A4728717
ubuntu@ubuntu:~/.password-store$ prs edit ali
thread 'main' panicked at 'attempting to encrypt secret for empty list of recipients', lib/src/crypto/backend/gnupg_bin/raw.rs:29:5
stack backtrace:
   0: std::panicking::begin_panic
   1: prs_lib::crypto::backend::gnupg_bin::raw::encrypt
   2: <prs_lib::crypto::backend::gnupg_bin::context::Context as prs_lib::crypto::IsContext>::encrypt
   3: <prs_lib::crypto::Context as prs_lib::crypto::IsContext>::encrypt
   4: prs_lib::crypto::IsContext::encrypt_file
   5: prs::action::edit::Edit::invoke
   6: prs::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

ubuntu@ubuntu:~/.password-store$ cat .gpg-id
E68783F3D26DBF30EEA62C6A800421D3A4728717 # albert alban <albert@microsoft.com>
ubuntu@ubuntu:~/.password-store$ prs edit ali
Secret updated

ubuntu@ubuntu:~/.password-store$ cat .gpg-id
800421D3A4728717 # albert alban <albert@microsoft.com>
ubuntu@ubuntu:~/.password-store$ prs edit ali

-----------------------------------------------------------------
ubuntu@ubuntu:~/.password-store$ prs housekeeping sync-keys
Cannot import missing public key, not available in store: 800421D3A4728717
Keys synced

ubuntu@ubuntu:~/.password-store$ cat .gpg-id
800421D3A4728717 # albert alban <albert@microsoft.com>

ubuntu@ubuntu:~/Documents/prs$ cargo run --no-default-features --features backend-gnupg-bin --  housekeeping sync-keys
warning: no interactive select mode features configured, falling back to basic mode
warning: use any of: select-skim, select-skim-bin, select-fzf-bin
    Finished dev [unoptimized + debuginfo] target(s) in 0.37s
     Running `target/debug/prs housekeeping sync-keys`
Cannot import missing public key, not available in store: 800421D3A4728717
Keys synced

buntu@ubuntu:~/Documents/prs$ cargo run --no-default-features --features backend-gpgme -- 
warning: no interactive select mode features configured, falling back to basic mode
warning: use any of: select-skim, select-skim-bin, select-fzf-bin
    Finished dev [unoptimized + debuginfo] target(s) in 0.38s
     Running `target/debug/prs`
prs 0.5.1
Usage: prs [FLAGS] <SUBCOMMAND> ...
Secure, fast & convenient password manager CLI with GPG & git sync

Add your own key as recipient or generate a new one:
    prs recipients add --secret
    prs recipients generate

Show a secret:
    prs show [NAME]

Sync your password store:
    prs sync

Show all subcommands, features and other help:
    prs help [SUBCOMMAND]
ubuntu@ubuntu:~/Documents/prs$ 

ubuntu@ubuntu:~/Documents/prs$ pass edit ali
gpg: Signature made Sat 06 May 2023 18:10:35 IDT
gpg:                using RSA key E68783F3D26DBF30EEA62C6A800421D3A4728717
gpg: Good signature from "albert alban <albert@microsoft.com>" [ultimate]
[master 7845191] Edit password for ali using editor.
 1 file changed, 0 insertions(+), 0 deletions(-)
 rewrite ali.gpg (100%)
ubuntu@ubuntu:~/Documents/prs$ 

ubuntu@ubuntu:~/Documents/prs$ prs housekeeping sync-keys
Cannot import missing public key, not available in store: 800421D3A4728717
Keys synced
ubuntu@ubuntu:~/Documents/prs$ gpg --list-keys
/home/ubuntu/.gnupg/pubring.kbx
-------------------------------
pub   rsa3072 2023-02-17 [SC]
      E68783F3D26DBF30EEA62C6A800421D3A4728717
uid           [ultimate] albert alban <albert@microsoft.com>
sub   rsa3072 2023-02-17 [E]

ubuntu@ubuntu:~/.password-store$ echo 800421D3A4728717> .gpg-id

ubuntu@ubuntu:~/.password-store$ git commit -am "commit"
[master 1d3d0eb] commit
 1 file changed, 1 insertion(+), 1 deletion(-)
ubuntu@ubuntu:~/.password-store$ prs housekeeping sync-keys
Cannot import missing public key, not available in store: 800421D3A4728717
Keys synced
ubuntu@ubuntu:~/.password-store$ cat .gpg-id
800421D3A4728717
ubuntu@ubuntu:~/.password-store$ 

And for the full ID

ubuntu@ubuntu:~/.password-store$ echo E68783F3D26DBF30EEA62C6A800421D3A4728717 > .gpg-id
ubuntu@ubuntu:~/.password-store$ git commit -am "commit"
[master 9e1dd0a] commit
 1 file changed, 1 insertion(+), 1 deletion(-)
ubuntu@ubuntu:~/.password-store$ prs housekeeping sync-keys
Keys synced
ubuntu@ubuntu:~/.password-store$ 

Test with gopass also ok (in addition to pass) - not sure if it is relevant

ubuntu@ubuntu:~/.password-store$ echo 800421D3A4728717> .gpg-id
ubuntu@ubuntu:~/.password-store$ gopass edit ali
ubuntu@ubuntu:~/.password-store$ 
shemeshg commented 1 year ago

The ubuntu I have is a virtual test/dev machine, so I don't mind

  1. destroy the ./passwordstore folder,
  2. empty all users
  3. run a test procedure from scratch that you will provide
  4. document that and post it here

I got this issue on production machine too with a (macos)

shemeshg commented 1 year ago

Ok Heres the tested fix I got, I'll upload a pull request (without the debug line)

macos@macoss-Mac-mini prs % cargo run --no-default-features --features backend-gnupg-bin --  housekeeping sync-keys
   Compiling prs-lib v0.5.0 (/Volumes/RAM_Disk_4G/rust/prs/lib)
warning: no interactive select mode features configured, falling back to basic mode
warning: use any of: select-skim, select-skim-bin, select-fzf-bin
   Compiling prs-cli v0.5.0 (/Volumes/RAM_Disk_4G/rust/prs/cli)
    Finished dev [unoptimized + debuginfo] target(s) in 2.81s
     Running `target/debug/prs housekeeping sync-keys`
SHEMESHG We have ["1CA9424DDD85177F"]
SHEMESHG Compare 0487760A36356046DAED24FC62682119BD7B8BF5 with 1CA9424DDD85177F
SHEMESHG Compare 6A05080C049FCD4EDCBB3F611CA9424DDD85177F with 1CA9424DDD85177F
SHEMESHG Compare 0487760A36356046DAED24FC62682119BD7B8BF5 with 1CA9424DDD85177F
SHEMESHG Compare 6A05080C049FCD4EDCBB3F611CA9424DDD85177F with 1CA9424DDD85177F
Keys synced
macos@macoss-Mac-mini prs % git status
On branch master
Your branch is up to date with 'origin/master'.

diff --git a/lib/src/crypto/util.rs b/lib/src/crypto/util.rs
index d9d6ca7..15768e5 100644
--- a/lib/src/crypto/util.rs
+++ b/lib/src/crypto/util.rs
@@ -13,8 +13,9 @@ pub fn format_fingerprint<S: AsRef<str>>(fingerprint: S) -> String {

 /// Check whether two fingerprints match.
 pub fn fingerprints_equal<S: AsRef<str>, T: AsRef<str>>(a: S, b: T) -> bool {
+    println!("SHEMESHG Compare {} with {}",a.as_ref().trim().to_uppercase(),b.as_ref().trim().to_uppercase());
     !a.as_ref().trim().is_empty()
-        && a.as_ref().trim().to_uppercase() == b.as_ref().trim().to_uppercase()
+        && a.as_ref().trim().to_uppercase().contains(&b.as_ref().trim().to_uppercase())
 }