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.

Sign up for GitHub

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

hsm_encryption: read from STDIN if not in a TTY #4571

Merged
merged 4 commits into from
Jun 3, 2021
Merged

hsm_encryption: read from STDIN if not in a TTY #4571

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4c82545
hsm_encryption: read from STDIN if not in a TTY
openoms Jun 1, 2021
8e86a57
lightningd: remove duplicate temp term creation
openoms Jun 1, 2021
07269b0
hsm_encryption.c: remove whitespace from line end
openoms Jun 3, 2021
cf5eabc
hsm_encryption.c: fix source include order
openoms Jun 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 37 additions & 25 deletions common/hsm_encryption.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#include <common/hsm_encryption.h>
#include <sodium/utils.h>
#include <termios.h>
#include <unistd.h>
#include <stdio.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hidden whitespace at the end of line makes it fail make check-source:

common/hsm_encryption.c:6:#include <stdio.h>	
Extraneous whitespace found
make: *** [Makefile:455: check-whitespace/common/hsm_encryption.c] Error 1

Damn our pedantic source checks!

(I would normally just push a fix directly, but some people consider that rude :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you, fixed in 07269b0. It's fair, every character counts!


char *hsm_secret_encryption_key(const char *pass, struct secret *key)
{
Expand Down Expand Up @@ -84,31 +86,41 @@ char *read_stdin_pass(char **reason)
char *passwd = NULL;
size_t passwd_size = 0;

/* Set a temporary term, same as current but with ECHO disabled. */
if (tcgetattr(fileno(stdin), &current_term) != 0) {
*reason = "Could not get current terminal options.";
return NULL;
}
temp_term = current_term;
temp_term.c_lflag &= ~ECHO;
if (tcsetattr(fileno(stdin), TCSAFLUSH, &temp_term) != 0) {
*reason = "Could not disable pass echoing.";
return NULL;
}

/* Read the password, do not take the newline character into account. */
if (getline(&passwd, &passwd_size, stdin) < 0) {
*reason = "Could not read pass from stdin.";
return NULL;
}
if (passwd[strlen(passwd) - 1] == '\n')
passwd[strlen(passwd) - 1] = '\0';

/* Restore the original terminal */
if (tcsetattr(fileno(stdin), TCSAFLUSH, &current_term) != 0) {
*reason = "Could not restore terminal options.";
free(passwd);
return NULL;
if (isatty(fileno(stdin))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that if this fails in lightningd/options.c, we'd fail. This is a bit unfortunate as we'd like to use it for lightningd as well i guess?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, indeed the piped password still fails for lightningd --encrypted_hsm
It is because the temporary terminal is being created as a duplicate in

lightning/lightningd/options.c

Line 397 in 03cfe0b

if (tcgetattr(fileno(stdin), &current_term) != 0)
before calling the read_stdin_pass.
Should be able to clean that, testing now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved with: 8e86a57

tested ok with:
$ (echo "test"; echo "test") | lightningd --encrypted-hsm

and when called in the command line the password (echo) is still hidden

lightningd --encrypted-hsm
The hsm_secret is encrypted with a password. In order to decrypt it and start the node you must provide the password.
Enter hsm_secret password:
Confirm hsm_secret password:

/* Set a temporary term, same as current but with ECHO disabled. */
if (tcgetattr(fileno(stdin), &current_term) != 0) {
*reason = "Could not get current terminal options.";
return NULL;
}
temp_term = current_term;
temp_term.c_lflag &= ~ECHO;
if (tcsetattr(fileno(stdin), TCSAFLUSH, &temp_term) != 0) {
*reason = "Could not disable pass echoing.";
return NULL;
}

/* Read the password, do not take the newline character into account. */
if (getline(&passwd, &passwd_size, stdin) < 0) {
*reason = "Could not read pass from stdin.";
return NULL;
}
if (passwd[strlen(passwd) - 1] == '\n')
passwd[strlen(passwd) - 1] = '\0';

/* Restore the original terminal */
if (tcsetattr(fileno(stdin), TCSAFLUSH, &current_term) != 0) {
*reason = "Could not restore terminal options.";
free(passwd);
return NULL;
}
} else {
/* Read from stdin, do not take the newline character into account. */
if (getline(&passwd, &passwd_size, stdin) < 0) {
*reason = "Could not read pass from stdin.";
return NULL;
}
if (passwd[strlen(passwd) - 1] == '\n')
passwd[strlen(passwd) - 1] = '\0';
}

return passwd;
Expand Down
8 changes: 0 additions & 8 deletions lightningd/options.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,16 +390,8 @@ static char *opt_important_plugin(const char *arg, struct lightningd *ld)
*/
static char *opt_set_hsm_password(struct lightningd *ld)
{
struct termios current_term, temp_term;
char *passwd, *passwd_confirmation, *err;

/* Get the password from stdin, but don't echo it. */
if (tcgetattr(fileno(stdin), &current_term) != 0)
return "Could not get current terminal options.";
temp_term = current_term;
temp_term.c_lflag &= ~ECHO;
if (tcsetattr(fileno(stdin), TCSAFLUSH, &temp_term) != 0)
return "Could not disable password echoing.";
printf("The hsm_secret is encrypted with a password. In order to "
"decrypt it and start the node you must provide the password.\n");
printf("Enter hsm_secret password:\n");
Expand Down