keys decrypt (CLI and GUI) should auto-detect armoring and mode, GUI <-> CLI interop issues
grempe opened this issue · 11 comments
Version 0.0.39 (macOS GUI and CLI)
I think that the keys decrypt function (and the equivalent in the GUI if it doesn't shell out to CLI) should auto detect the armoring and the mode (encrypt | signcrypt) when decrypting content.
Especially for cyphertext that was signcrypt vs. encrypt as this is not something the recipient can know in advance. This causes UX issues in both the GUI and the CLI. Some examples.
Also, there are interoperability issues between GUI and CLI which is due to the related problem of improper mode selection for both signcrypt and decrypt type operations.
GUI
OK : Using the GUI Encrypt tab to sign and encrypt a message (in this case signed by grempe@github and encrypted to myself as well) and then pasting that ciphertext into the Decrypt tab properly decrypts the contents and verifies the signature. So cyphertext BOTH created and decrypted WITHIN the GUI is OK.
ERROR : In GUI, pasting a signcrypt message with the same users that was created in the CLI into the Decrypt GUI pane fails with message Wrong saltpack message type: wanted an encrypted message, but got a signed and encrypted message instead. The GUI should just 'do the right thing' and handle a signed and encrypted message created by the CLI.
The command used on the CLI to create the ciphertext for the GUI was:
echo "foo" | keys encrypt -r grempe@github -s grempe@github -m signcrypt -a
ERROR : If using the GUI to create encrypted text, and also choosing that it should be signed by someone is only creating encrypt mode ciphertext, not signcrypt as expected.
e.g. in GUI I encrypt 'FOOBAR' to myself, and also choose to sign with my same key results in ciphertext that is in encrypt mode, not signcrypt mode.
Note, I had to specify encrypt mode not signcrypt mode.
echo "BEGIN SALTPACK ENCRYPTED MESSAGE. kcJn5brvybfNjz6 D5ll2Nk0YySyo5u S8VITxP1Nu9IZBo GKPXIiQJPm0Pp0f 19D7dVOcGMstPuc lCqpOhGMzz8wFgS DRIO9gnIKuCZJwN dHpiNUpllGRU3LS 4vusrjLtGnrkzlE vbaUnW9HMd4IM9P N2ENoDnrXGbWTSW aZ3hELM4HppRWxJ Otp4pA2f6K8xGsD 9GVOcXCzAlnqJCw 24HRMmMHxNPwA7r isdGXipkuZfZJhZ ZlFMQ7KJrikG5cw kII7wNE4760U2jL RtYqclNUh36Hn2N Add. END SALTPACK ENCRYPTED MESSAGE." | keys decrypt -a -m encrypt
verified kex1ygrarq8l83l0tvhhesqwepuzza26zq75t9nm8pq78svz086ktk2sl5mtul grempe@github
FOOBAR%
CLI
echo "foo" | keys encrypt -r grempe@github -s grempe@github -m signcrypt -a | keys decrypt -a -m signcrypt
ERROR : this throws an error when no mode is supplied to decrypt, even though the decrypt function clearly knows that this content was both signed and encrypted. It should use this knowledge to just automatically apply the right mode.
❯ echo "foo" | keys encrypt -r grempe@github -s grempe@github -m signcrypt -a | keys decrypt -a
Wrong saltpack message type: wanted an encrypted message, but got a signed and encrypted message instead
ERROR : this throws an error when armor setting is not applied to decrypt. This despite the fact that decrypt did not see the expected header bytes. In which case it should apply armoring mode as needed to attempt decrypt. The message is also not helpful in guiding the user to a proper solution (providing the -a flag).
❯ echo "foo" | keys encrypt -r grempe@github -s grempe@github -m signcrypt -a | keys decrypt -m signcrypt
failed to read header bytes
Same for when neither armor or mode option is provided to decrypt:
❯ echo "foo" | keys encrypt -r grempe@github -s grempe@github -m signcrypt -a | keys decrypt
failed to read header bytes
SUMMARY
Implementing these heuristics on decrypt, in GUI and CLI, would allow decrypt to be much more forgiving (especially in a GUI context!). You should be forgiving as there is no way for the user to know the mode by looking at the ciphertext. In all cases content created by the GUI or CLI MUST be fully interoperable.
Related UX issues on the encrypt side (@atoponce and I were sending some test messages back and forth to explore keys.pub).
https://old.reddit.com/r/crypto/comments/fv0p8h/keyspub_request_for_feedback/fpwb6bu/
I'll create a separate UX issue to follow. Totally digging it by the way. Just fiddling around for now, but I think there is potential.
This is great feedback thank you! Might be a few days before I can respond fully though.
Can you please change the label on this issue from enhancement to bug. The core of this report is serious broken interoperability between (and within) the GUI and the CLI versions (tested as of latest Mac released version keys version 0.0.47 a37e6c8c36eb2c2c46c685b7be66c5c3dfb4d491 2020-06-03T01:50:20Z)
This can't be considered a replacement for anything if it can't encrypt/decrypt/verify reliably within and between itself.
Thanks
Here is a matrix that show where things are working or not for CLI -> GUI. You'll need to scroll to the right as it is wide. This matrix also only shows using CLI generated output in the GUI. There need to be similar interoperability tests for GUI to CLI, GUI to GUI, and CLI to CLI which really need to be created (and automated for test).
CLI to GUI Examples
| to GUI Encrypt | to GUI Encrypt w/ Sign | to GUI Decrypt | to GUI Sign | to GUI Verify | |
|---|---|---|---|---|---|
| CLI encrypt (armored) | - | - | ok | - | No output (should indicate wrong message type) |
| CLI encrypt (binary) | - | - | FAIL (message: open .tmp: read-only file system) |
- | - |
| CLI signcrypt (armored) | - | - | FAIL | - | No output (should indicate wrong message type) |
| CLI signcrypt (binary) | - | - | FAIL (message: Wrong saltpack message type: wanted an encrypted message, but got a signed and encrypted message instead) |
- | - |
| CLI decrypt | - | - | - | - | - |
| CLI sign (armor,attached) | - | - | - | - | FAIL : (message: failed to read header bytes ) |
| CLI sign (armor,detached) | - | - | - | - | FAIL : (message: failed to read header bytes ) |
| CLI sign (binary,attached) | - | - | - | - | FAIL : (message: open .tmp: read-only file system) |
| CLI sign (binary,detached) | - | - | - | - | FAIL : (message : Wrong saltpack message type: wanted an attached signature, but got a detached signature instead ) |
| CLI verify | - | - | - | - | - |
CLI encrypt (armored) -> GUI Decrypt : OK
❯ echo "foo" | keys encrypt -r grempe@github -s grempe@github -a
BEGIN SALTPACK ENCRYPTED MESSAGE. kcJn5brvybfNjz6 D5ll2Nk0Z00kOCS fnr6380v5IY0fcJ H4R4m72iVw99qIh amDYQF7SdhVPKNs wvu9RwROr0Es1dG vv9YZUrdMD3DADc tpyvU3BukoFnZB1 CZY5VMWIf5nR0Lc GmtEdnbELwR1YNF zg85IviL1ISaZjz zg3myiHA5DQJ2ZK fIwAu4zEW4uS3W6 anhrEDVndkGR3Sr 7fS5xD6e1xjVu4a YMbFWyh4PiYSSRc 9VHCYpWg9gTj7k6 rmGLG3rvgEao56J Q4gBob326Qd6nlY . END SALTPACK ENCRYPTED MESSAGE.
CLI encrypt (binary file) -> GUI Decrypt : FAILS
Note : The -o or --out flag does not seem to actually work. Even with it specified and a filename provided it still writes to STDOUT (STDERR?).
❯ echo "foo" | keys encrypt -r grempe@github -s grempe@github -m encrypt --out ~/Desktop/key-enc-binary
ė��saltpack�� �L�����9��/��q&�E̞|G�өā�c�0��:
\���h��KýF��1�Ya��ø�Pa���$&E�ERQ���Q�*�����0ݫ�j�'Ő��/˺O�����rs(�y�W�Ĥ�rʴ
�����eģ�Ñ� ��^ECV���Qㅙa�I=�c�O����M�\���;���@J��8��r��%
Note : Using redirection, you can seemingly workaround the broken --out option to write the file. But this file is not able to be read by GUI Decrypt. Trying to read this binary file in the GUI Decrypt results in a open .tmp: read-only file system error message (in red) in the GUI.
❯ echo "foo" | keys encrypt -r grempe@github -s grempe@github -m encrypt > ~/Desktop/key-enc-binary
❯ file ~/Desktop/key-enc-binary
/Users/glenn/Desktop/key-enc-binary: data
CLI encrypt -m signcrypt -> GUI Decrypt : FAILS (Wrong saltpack message type: wanted an encrypted message, but got a signed and encrypted message instead
Note : The GUI Decrypt tab seems to be unable to handle signcrypt messages from the CLI.
❯ echo "foo" | keys encrypt -r grempe@github -s grempe@github -m signcrypt -a
BEGIN SALTPACK ENCRYPTED MESSAGE. keDIDMQWYvVR58B FTfTeDQNHxwMjvH vkYMpe97CYBEyaC CxF3KrnE3oh7voF 32XmMKXuuTdWQ64 tPGrbYiJsJ04AVG nEcbQUb9UCgtW4s GwE3pPF2vvjRvbG dfY1DSv4nOB3Egk hhrEhWj4RFu6vNG PoJTqEvIXJe9Rvv 3zDQpxBLmqm1Wbj X1wnmxM8WgkpC9C 6jnrp6KDmLFqaYF S14m6UKaX31nnhA 08UwNlWukmf2LWU vDHPbJvC2FGKBhV EG2tkq4i92BtsCO dcy8NyNS8Dsn444 NAWilGC14NXdzCF QhHmVAYoToBe9K6 3KQJtfui5qfrBwZ TDpBtbsDeJ42tR1 LGcrCmngQoG82Bv CXRpfgi3v. END SALTPACK ENCRYPTED MESSAGE.
CLI encrypt -m signcrypt -> GUI Verify : FAILS ([no error message provided in GUI])
Note : While it is not necessarily expected that GUI Verify should be able to verify a signcrypt message, it would be nicer to display a message saying that this is the wrong type and the user should instead paste this ciphertext in the Decrypt tab.
❯ echo "foo" | keys encrypt -r grempe@github -s grempe@github -m signcrypt -a
BEGIN SALTPACK ENCRYPTED MESSAGE. keDIDMQWYvVR58B FTfTeDQNHxwMjvH vkYMpe97CYBEyaC CxF3KrnE3oh7voF 32XmMKXuuTdWQ64 tPGrbYiJsJ04AVG nEcbQUb9UCgtW4s GwE3pPF2vvjRvbG dfY1DSv4nOB3Egk hhrEhWj4RFu6vNG PoJTqEvIXJe9Rvv 3zDQpxBLmqm1Wbj X1wnmxM8WgkpC9C 6jnrp6KDmLFqaYF S14m6UKaX31nnhA 08UwNlWukmf2LWU vDHPbJvC2FGKBhV EG2tkq4i92BtsCO dcy8NyNS8Dsn444 NAWilGC14NXdzCF QhHmVAYoToBe9K6 3KQJtfui5qfrBwZ TDpBtbsDeJ42tR1 LGcrCmngQoG82Bv CXRpfgi3v. END SALTPACK ENCRYPTED MESSAGE.
Sample commands used for signature test options:
All of these fail in the GUI Verify tab when selecting the file using the file picker. I did not also test pasting in armored signatures. The GUI also doesn't allow for the possibility of selecting the content to match with a detached signature.
❯ keys sign --signer grempe@github --in /tmp/foo.txt --mode armor,detached --out ~/Desktop/keys-sign-armor-detached
❯ keys sign --signer grempe@github --in /tmp/foo.txt --mode armor,attached --out ~/Desktop/keys-sign-armor-attached
❯ keys sign --signer grempe@github --in /tmp/foo.txt --mode binary,attached --out ~/Desktop/keys-sign-binary-attached
❯ keys sign --signer grempe@github --in /tmp/foo.txt --mode binary,detached --out ~/Desktop/keys-sign-binary-detached
I hope this is helpful.
This is super helpful! I've been working on new features, but yeah I agree this should be a priority.
It might still be a week or so before I can get to this though.
OK! I did a bunch of work on this, and did a prerelease build (v0.1.7).
It is much smarter now...
For decrypt, it will automatically detect the mode (signcrypt vs encrypt) and if armored or binary. The mode and armor option for keys decrypt was removed.
For verify, it will automatically detect if armored or binary. The armored option for keys verify was removed.
What this means is that verify and decrypt should now work on any valid inputs/formats in both command line and GUI. I am still testing this though.
There was an issue with specifying -out without -in. I made it return an error -out option is unsupported without -in, the workaround is if you use stdin, you can redirect stdout instead. I might revisit this in the near future though. (The RPC calls currently do streaming or file paths but not mixed input types, so it's not super trivial, and seems like an edge case so will wait on that.)
After some more testing, will release this in the next few days.
I look forward to checking it out!
Smoothing out this interoperability matrix will go a long way for usability.
Ok, pushed out a couple new versions v0.1.9 is latest.
This should all be resolved. I also changed the sign options to be less weird (removed "mode" strings and allowing regular options), like keys sign -armor instead of keys sign -mode=armor.
Things are definitely improving. Nice work.
I went through my cases above and filed a few new very focused issues that I spotted while testing 0.1.9. Some are new enhancement requests, and some are issues that remain that were jumbled into this original "stream of consciousness" bug report that I teased out to better discuss.