Do not require bash

Line 59 in 7708008

$password = \rtrim((string) \shell_exec($command));

Some code I'm using for this, I think this will be more portable (and simpler)...

        // ask for password
        exec('stty -echo');
        echo 'Password: ';
        $userPass = trim(fgets(STDIN));
        echo PHP_EOL.'Password (repeat): ';
        $userPassRepeat = trim(fgets(STDIN));
        exec('stty echo');
        echo PHP_EOL;

Huh. Are there modern systems that don't support bash in 2020?

Bash is not included on the BSDs, and it's on its way out on MacOS. I think this change is a good idea.

The reason for opening this issue actually had nothing to do with whether or not bash is available, which is of course also a valid consideration. The reason is that running bash (or any shell) like this is overkill, i.e. running a lot of extra unnecessary code for functionality that can be accomplished in a much simpler way, maybe even simpler than I propose, I actually hope so!

In addition, this code is difficult to review as one also has to consider the interaction with the shell, environment variables, potentially leaking the password to other (local) users on the system, and probably more that I can't think of right now.

IMHO, simple is better, less is more, especially for security (sensitive) code.

Maybe this comment on php.net can provide an even simpler solution?

The relevant section of the comment:

<?php
    readline_callback_handler_install('', function(){});
    echo("Enter password followed by return. (Do not use a real one!)\n");
    echo("Password: ");
    $strObscured='';
    while(true)
    {
    $strChar = stream_get_contents(STDIN, 1);
    if($strChar===chr(10))
    {
        break;
    }
    $strObscured.=$strChar;
    echo("*");
    }
    echo("\n");
    echo("You entered: ".$strObscured."\n");
?>

Note that this code echoes * characters for keypresses, but it can easily be changed not to

The code does not support backspace

Also note that STDIN may not be available in some cases, in which case this code is an endless loop - add a guard such as defined('STDIN')

The reason for opening this issue actually had nothing to do with whether or not bash is available, which is of course also a valid consideration. The reason is that running bash (or any shell) like this is overkill, i.e. running a lot of extra unnecessary code for functionality that can be accomplished in a much simpler way, maybe even simpler than I propose, I actually hope so!

The code in question is shell_exec(), right? (I thought this was about bin/test-cli.sh at first glance. I just want to make sure I have a good mental model.)

In addition, this code is difficult to review as one also has to consider the interaction with the shell, environment variables, potentially leaking the password to other (local) users on the system, and probably more that I can't think of right now.

IMHO, simple is better, less is more, especially for security (sensitive) code.

I'm torn here. STDIN support makes me more nervous than a stdlib function, especially since it may not be available in some cases. Also, does shell_exec() explicitly require bash, or does it work with any shell?

This should be fixed in v0.3.0.

STDIN support makes me more nervous than a stdlib function, especially since it may not be available in some cases.

Why is that? When is it not available?

If you run the script like this

# don't do this
php -sM file <vendor/bin/minisign

Since the PHP script is read via stdin, stdin is not available for running the script anymore

Explaination:

% php -r 'echo fgets(STDIN);'
foo
foo
% echo '<?php echo fgets(STDIN);' | php

Warning: Use of undefined constant STDIN - assumed 'STDIN' (this will throw an Error in a future version of PHP) in Standard input code on line 1

Warning: fgets() expects parameter 1 to be resource, string given in Standard input code on line 1
%