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

[BUG] - cardano-cli named pipe for signing key: fails, does not block #4101

Closed
Tracked by #4301
rphair opened this issue Jun 26, 2022 · 5 comments · Fixed by #4342
Closed
Tracked by #4301

[BUG] - cardano-cli named pipe for signing key: fails, does not block #4101

rphair opened this issue Jun 26, 2022 · 5 comments · Fixed by #4342
Assignees
Labels
comp: cardano-cli type: bug Something is not working user type: external Created by an external user

Comments

@rphair
Copy link

rphair commented Jun 26, 2022

Internal/External External
Category CLI syntax, argument processing, file system interface

Either cardano-cli does not block when given a named pipe as an argument for --signing-key-file (and possibly other option arguments) or it fails because the named pipe is not recognised as a file.

Steps to reproduce

$ cardano-cli transaction build \
--mainnet \
--tx-in $(cat payment.utxo) \
--tx-out $(cat wallet.addr)+1000000 \
--change-address $(cat payment.addr) \
--out-file tx.draft
Estimated transaction fee: Lovelace 168185

$ mkfifo /tmp/sign-through-fifo.skey
$ ls -l /tmp/sign-through-fifo.skey
prw-rw-r-- 1 spo spo 0 2022-06-26 18:45 /tmp/sign-through-fifo.skey

$ cardano-cli transaction sign \
--tx-body-file tx.draft  \
--signing-key-file /tmp/sign-through-fifo.skey \
--mainnet \
--out-file tx.signed
Command failed: transaction sign  Error: Error reading signing key: /tmp/sign-through-fifo.skey: Invalid key.

The final command above fails with this error message immediately.

Expected behaviour

The proper behaviour would be for the cardano-cli transaction sign to block until it receives an EOF from reading /tmp/sign-through-fifo.skey (after someone or something else has written the key file contents into that file). From https://linux.die.net/man/3/mkfifo -

Opening a FIFO for reading normally blocks until some other process opens the same FIFO for writing, and vice versa.

System info (please complete the following information):
OS Name: Ubuntu 5.4.0-121-generic
OS Version: 20.04 focal

cardano-cli 1.34.1 - linux-x86_64 - ghc-8.10
git rev 73f9a746362695dc2cb63ba757fbcabb81733d23

Context & reasoning

There is an argument, presented here: https://forum.cardano.org/t/cardano-cli-signing-a-transaction-without-directly-accessing-the-skey-file-in-plain-text/103742/11

... that any effort to "secure" cardano-cli's handling of private keys is actually forcing the private keys into files, where as cleartext they are the least secure of all, not to mention subject to many types of remanence of the file system... especially the ext4 journal which makes files practically impossible to delete.

If security reservations are still maintained by the node developers then we need them to be publicly discussed here (or cited, if this deliberation has already taken place). Until then I propose this is a "bug" since whatever sanity check is being done on the key file argument simply does not recognise the presence of a named pipe.

@rphair rphair added the bug Something isn't working label Jun 26, 2022
@rphair rphair changed the title [BUG] - cardano-cli doesn't block reading named pipe for signing key [BUG] - cardano-cli named pipe for signing key: fails, does not block Jun 26, 2022
@gitmachtl
Copy link
Contributor

This may also relate to #4235

@gitmachtl
Copy link
Contributor

gitmachtl commented Aug 7, 2022

i did a test, this is working with cardano-cli 1.35.3-rc1:

tmpKey=$(cat ${fromAddr}.skey)
${cardanocli} transaction sign --tx-body-file ${txBodyFile} --signing-key-file <(echo "${tmpKey}") ${magicparam} --out-file ${txFile}

@gitmachtl
Copy link
Contributor

gitmachtl commented Aug 8, 2022

but this output redirection to the stdout is not working for the VRF-Key creation:

$ cardano-cli node key-gen-VRF --verification-key-file "test.vrf.vkey" --signing-key-file /dev/stdout

cardano-cli-1.35.3-rc1: /dev/: openTempFile: permission denied (Permission denied)

Why the heck on earth wants cardano-cli to write a TempFile in the path given for the output signing key!?
Writing to /dev/stdout works for all other commands, but not for the VRF keys. We need this to do secure key handling without the need to write them out to file on the HDD.

@rphair
Copy link
Author

rphair commented Aug 9, 2022

It also promotes security when we can read a VRF signing key from a pipe, like we ourselves often do when generating the leaderlog with cncli (though there's no such possibility on cardano-cli). We can run this on any node without its having a physical copy of the key:

mkfifo $myVRFkeyPath || (echo ${myVRFkeyPath}: cannot create FIFO; exit 1)
echo "::::::: NOW put the VRF key here: $myVRFkeyPath ::::::: "
cncli leaderlog --pool-vrf-skey "$myVRFkeyPath" ...

@gitmachtl
Copy link
Contributor

gitmachtl commented Aug 9, 2022

@disassembler said it's most likely caused by libsodium, thats the only exposed cli function that uses libsodium. i recently updaten my SPO scripts to do .skey encryption/decryption on the fly. and catching a generated signing-key via a pipe is working with all commands. but the key-gen-VRF is the only one that is not working.

LudvikGalois pushed a commit that referenced this issue Aug 16, 2022
When reading a key from a file, `readFile` from ByteString was used.
Unfortunately this operation doesn't play nicely with pipes. We now use
`openFileBlocking` so that we can wait for data which comes from a pipe.

Fixes #4101
iohk-bors bot added a commit that referenced this issue Aug 17, 2022
4342: Use openFileBlocking for reading signing keys r=newhoggy a=LudvikGalois

When reading a key from a file, `readFile` from ByteString was used.
Unfortunately this operation doesn't play nicely with pipes. We now use
`openFileBlocking` so that we can wait for data which comes from a pipe.

Fixes #4101

Co-authored-by: Robert 'Probie' Offner <robert.offner@iohk.io>
@iohk-bors iohk-bors bot closed this as completed in b6780ba Aug 17, 2022
Jimbo4350 pushed a commit that referenced this issue Oct 5, 2022
When reading a key from a file, `readFile` from ByteString was used.
Unfortunately this operation doesn't play nicely with pipes. We now use
`openFileBlocking` so that we can wait for data which comes from a pipe.

Fixes #4101
@dorin100 dorin100 added type: bug Something is not working user type: external Created by an external user comp: cardano-cli and removed bug Something isn't working labels Oct 21, 2022
disassembler pushed a commit that referenced this issue Oct 25, 2022
When reading a key from a file, `readFile` from ByteString was used.
Unfortunately this operation doesn't play nicely with pipes. We now use
`openFileBlocking` so that we can wait for data which comes from a pipe.

Fixes #4101
newhoggy pushed a commit to IntersectMBO/cardano-api that referenced this issue May 23, 2023
When reading a key from a file, `readFile` from ByteString was used.
Unfortunately this operation doesn't play nicely with pipes. We now use
`openFileBlocking` so that we can wait for data which comes from a pipe.

Fixes IntersectMBO/cardano-node#4101
newhoggy pushed a commit to IntersectMBO/cardano-cli that referenced this issue May 24, 2023
When reading a key from a file, `readFile` from ByteString was used.
Unfortunately this operation doesn't play nicely with pipes. We now use
`openFileBlocking` so that we can wait for data which comes from a pipe.

Fixes IntersectMBO/cardano-node#4101
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: cardano-cli type: bug Something is not working user type: external Created by an external user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants