Signing 3 - Key loading/handling in Conductor

[lucksus]

on top of #968

• I have added a summary of my changes to the changelog

Context

All the pre-work enabling the Conductor to create real signatures from real keys read out of real files.

Steps

• Conductor gets a new call back KeyLoader that is implemented with a default that reads files from the filesystem and prompts for a passphrase
• nodejs Conductor provides its own key loader callback to inject deterministic keys for tests that are created from the agent name's SHA256
• Conductor checks that public keys given in the config match the key load from file

[lucksus]

@neonphog, @zo-el, I had to declare SodiumBuf Send for this to work: 19cde59
Is it safe to send these across threads? We sure need that and I don't see why not, but rustc can't say much about a c_void..

[neonphog]

@lucksus - so long as sodium_init() is called before any functions are invoked, all libsodium functions are thread safe. The memory allocated in SodiumBuf is not mutexed or anything, so not safe to access from multiple threads, but, as you say, sending should be perfectly safe.

I misread this question at first, and thought we needed a clone for SecBuf... here's that code in case it's useful someday, lol. Keys are generally very small so cloning is cheap: https://github.com/holochain/holochain-rust/compare/clone-secbuf

[neonphog]

passphrases should use secure memory

Ideally, we'd use a low-level keyboard binding to prevent a class of malware keyloggers and input the data directly into a secure buffer.

For the short-term, we should be sure to zero the memory in "passphrase" returned by the rpassword utility.

[lucksus]

I had tried that already in hc keygen but SodiumBuf doesn't allow arbitrary sizes for secure buffers. Only multiples of 8. Also, since the passphrase is coming in as a String, it wouldn't make it much safer, right?

But yeah, so you are saying, we should roll our own read_password_from_tty eventually?

[neonphog]

: ) right, that totally makes sense. I'll create an issue to do the alignment in the background and deal with appropriately sized slice references in the SecBuf so we don't have to worry about the sizing during implementation.

[neonphog]

#1017

[neonphog]

Some security comments. Looking awesome, I'm excited to see this coming together!

[lucksus]

@neonphog, I've implemented your change requests, except for using a secure SecBuf as that enforces sizes % 8. I've tried padding with zeros but I guess that makes it be a different passphrase. Can't unlock keys with that. Any hint how to use SecBuf with arbitrary length passphrases?

[zippy]

one tiny change in an error message

[zippy]

Suggested change

	.ok_or(format!("Agent '{}' not found", agent_id))?;
	.ok_or(format!("Agent '{}' not found in config", agent_id))?;