Re: [PATCH v4 3/3] send-email: add option to generate passswords like OAuth2 tokens

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On 24 Apr 2025, at 12:33 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> 
> Aditya Garg <gargaditya08@xxxxxxxx> writes:
> 
>> @@ -230,6 +230,14 @@ or on the command line. If a username has been specified (with
>> specified (with `--smtp-pass` or `sendemail.smtpPass`), then
>> a password is obtained using 'git-credential'.
>> 
>> +--smtp-passeval[=<command>]::
> 
> Lose the pair of [] that marks the value optional.  Compare it with,
> say, --smtp-user that is described as:

The value is not optional. It doesn't make sense to leave this empty right?
> 
>    --smtp-user=<user>::
>            Username for SMTP-AUTH. Default is ...
> 
> because they are defined in %options (below) in a similar way, like
> so:
> 
>>            "smtp-user=s" => \$smtp_authuser,
>>            "smtp-pass:s" => \$smtp_authpass,
>> +            "smtp-passeval=s" => \$smtp_authpasseval,
>>            "smtp-ssl" => sub { $smtp_encryption = 'ssl' },
> 
> taking a string value =s that is not optional.
> 
>> +    Generate password like OAuth2 token for SMTP AUTH. If specified,
>> +    it will use the output of the command specified as a password for
>> +    authentication.
>> ++
> 
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index a18e978e22..cafb9aa43b 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -59,6 +59,8 @@ sub usage {
>>     --smtp-server-port      <int>  * Outgoing SMTP server port.
>>     --smtp-user             <str>  * Username for SMTP-AUTH.
>>     --smtp-pass             <str>  * Password for SMTP-AUTH; not necessary.
>> +    --smtp-passeval         <str>  * Path to script or a command to generate
>> +                                     password like OAuth2 token for SMTP-AUTH.
>>     --smtp-encryption       <str>  * tls or ssl; anything else disables.
>>     --smtp-ssl                     * Deprecated. Use '--smtp-encryption ssl'.
>>     --smtp-ssl-cert-path    <str>  * Path to ca-certificates (either directory or file).
> 
> Looking good.
> 
>> +    # If smtpPassEval is set, run the user specified command to get the password
>> +    if (defined $smtp_authpasseval) {
>> +        printf __("Executing token generating script: %s\n"), $smtp_authpasseval;
>> +        chomp(my $generated_password = `$smtp_authpasseval 2>&1`);
> 
> How careful do we need to protect ourselves against a bad value in
> this variable (like "rm -rf $HOME; password-command") ?  Are we OK
> with trusting that the command line and the configuration file are
> not under control of an attacker?

I would ask the same question for the sendmailcmd option which also
invokes the shell. Ideally we should be able to use git credential manager,
but Microsoft has other plans. There are not very good methods to get OAuth2
tokens for free Microsoft accounts, such relatively unsafe ways are some easier
options.

Attackers these days can also get your access token, refresh token etc for OAuth2.
The user should be atleast aware of ways to keep their system secure. Maybe use
LUKS? And commands like rn -rf $HOME, I would say the risk is equal to typing 
sudo rm -rf $HOME on the terminal, but it doesn't mean we remove the terminal.

>  I am assuming it is OK, but your
> folks have thought about this code path much longer than I have, so
> I thought I should ask just to make sure.
> 
> Thanks.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux