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

CLI freezes in shell mode when connecting using interactive password a with long value #103

Closed
sauroter opened this issue Oct 22, 2019 · 2 comments · Fixed by #107
Closed
Assignees
Labels
bug Something isn't working

Comments

@sauroter
Copy link
Member

sauroter commented Oct 22, 2019

Expected behavior

Client connects with password set to a long value.

Actual behavior

CLI becomes unresponsive and needs to be terminated with CTRL+C.

To Reproduce

Steps

$ mqtt sh
$ con -pw
Enter value for --password (The password for authentication): $

Enter long password, e.g. a JWT.

Reproducer code

No code.

Details

1.0.1
Picocli 4.0.4
JVM: 11.0.4 (AdoptOpenJDK OpenJDK 64-Bit Server VM 11.0.4+11)
OS: Mac OS X 10.15 x86_64

@gitseti gitseti added the bug Something isn't working label Oct 22, 2019
@gitseti gitseti self-assigned this Oct 22, 2019
@gitseti
Copy link
Contributor

gitseti commented Oct 24, 2019

Thanks @sauroter,

the bug is happening because the picocli interactive mode for password reading is using the Console.readPassword() function. This functions seems to only support reading up to 1024 characters. If you copy-paste a string with more than 1024 characters the excess characters get truncated. The problem now is that the readPassword method cannot process the next char which would be the newLine character to confirm the password. You can verify this by deleting the last character in the interactive mode via backspace and then pressing Enter which should work.

This problem can be tracked back to the tty driver’s internal buffer which seems to be capped at 1024 characters for OS X.

Suggested solution:

I think the best and more secure practice to read longer passwords should be to provide a functionality which lets me read the password from a file or via an environment variable as suggested in a picocli example:

    @Option(names = "--pw:file")
    File passwordFile;

    @Option(names = "--pw:env")
    String passwordEnvironmentVariable;

    @Option(names = "--pw", interactive = true, /* ... */)
    String password;

	/* ... */

		if (password != null) {
            login(password);
        } else if (passwordEnvironmentVariable != null) {
            login(System.getenv(passwordEnvironmentVariable));
        } else if (passwordFile != null) {
            // below uses Java 8 NIO, create your own on older Java versions
            /*
            login(new String(Files.readAllBytes(passwordFile.toPath())));
            */

What do you think of this solution or could it be that I'm missing something?

@sauroter
Copy link
Member Author

Thanks for looking into this!

I like the solution with environment variables. But thats just my opinion :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants