Re: [Savannah-hackers-public] On resetting passwords

[Ineiev]

Hello, Assaf!

On Fri, Mar 10, 2017 at 03:59:40PM +0000, Assaf Gordon wrote:
> 1.
> not passing user/pw for database from the PHP to the PERL script
> as command-line arguments.

Do you think it would be better if I passed them in the first lines
of the input?

> Bob re-wrote 'sv_get_authorized_keys' in Perl
> with example of how to access the config file with the DB passwords,
> see vcs0:/root/bin/sv_get_authorized_keys .

It uses hard-coded paths in the Perl code; these paths don't work
on my local Savane installation.

> 2.
> not including hard-coded paths in the PHP code.
> Based on our recent migration efforts, this is bound to cause troubles
> later.

I can only see "/usr/bin/perl". do you think "perl" would be better?

> 3.
> It would be good to add a lot more input validation and error checking
> in the perl script.
> Again based on our recent efforts, chasing obscure errors
> in nginx's log from scripts executed by the PHP frontend is not fun.
>
> 4.
> I'm a bit wary of executing gpg in this way based on un-sanitized user
> input. Basically, the script takes whatever the user uploaded as his
> GPG key and pipes it into GPG.
> This sounds like inviting problems.

Savane already uses it this way; if we would like to sanitize
the input, we should do it when the user submits the key. and to tell
the truth, I'm not sure how to sanitize it otherwise than
to actually submit it to GPG.

> 5.
> The 'open' command should escape/sanitize the parameters before
> executing them as shell commands (e.g. the "$key_id" which is
> set by the user who uploads the GPG key, but could be others
> that I've missed).

$key_id is set by GPG after parsing user's certificate. I can't
think of a way to usefully sanitize it.

> 6.
> There seems to be an implicit requirement that the uploaded
> key be able to encrypt (based on the comment "Get the first ID of a
> public key with encryption capability.").
> To be user-friendly, this requires proper error checking and
> reporting if no such key is found.
> Otherwise we're just inviting lots of support calls from confused users.

I agree that more detailed reporting wouldn't hurt.

> Lastly,
> If you'd like to have a dedicated development environment for you
> (e.g. 'https://ineiev.frontend0.sv.gnu.org') where you can
> experiment with new features, let me know and I'll be happy to
> set it up.

Sincerely speaking, I'd prefer if you improved the instructions
on how to run Savane locally without having to ask for such
an environment: the previous version worked better for me,
because the current one required a PHP version I don't
always have (instead of Apache), and I had some issues
with database initialization.

Thank you!

signature.asc
Description: Digital signature