Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prs do not strip comments of gpg_id file #22

Open
shemeshg opened this issue May 3, 2023 · 7 comments
Open

prs do not strip comments of gpg_id file #22

shemeshg opened this issue May 3, 2023 · 7 comments

Comments

@shemeshg
Copy link

shemeshg commented May 3, 2023

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
Copy link
Owner

timvisee commented May 4, 2023

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
Copy link
Author

shemeshg commented May 4, 2023

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
Copy link
Author

shemeshg commented May 4, 2023

Ok I got this,

1, Yes we solved the problem of the remarks
2. 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

  • Like within pass it will search by id, full id, email, full name, etc... by the built in gpgme find key function.
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;
}
  • if found more then one, throw exception and not continue,

  • if not found, it will throw exception and not continue,

    Now, it silently ignore the ID, and there is a risk that user will restore his data, and find out it can not read it
    because they have counted on ignored ID.

Screenshot example from c++ debugger

image

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

Question

  • Should I open different ticket for that, or can we keep using this one?

@timvisee
Copy link
Owner

timvisee commented May 6, 2023

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
Copy link
Author

shemeshg commented May 6, 2023

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

  • files created with echo full_id>.gpg_id' and prs housekeeping sync-keys` is passed
  • files created with echo short_id>.gpg_id' and prs housekeeping sync-keys` failed

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
Copy link
Author

shemeshg commented May 7, 2023

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
Copy link
Author

shemeshg commented Jun 4, 2023

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())
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants