Re: Sending envvars via ssh agent protocol

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

 



On Fri, 29 Jan 2021 00:06, Ángel said:

> This seems backwards. I would expect command line to have hidher
> priority than ssh_config, not ~/.ssh/config to be able to disable an

I also wondered about this but it is clearly stated at the top of
ssh_config(5)

> Also, I would suggest using none instead of -, as that's what other ssh
> options use. (This might cause issues if you wanted to pass an
> environment variable named "none", but the same problem already exists
> for "auto")

I agree, lowercase envvars and in particular "none" and "auto" should be
rearly be exportable.

>> +                               error("%s line %d: Invalid environment name.",
> Maybe nitpicking, but on this error (appears twice) I would say
> "Invalid name of environment variable". The environment would be the

Its longer but fir sure more correct.

>> if (*arg == '-') {
>
>> if (*arg == '#') {
>
> You are comparing against the first character of the argument.
> Per your description I would expect that you compared that the whole
> was that, not just that it began with # or -

You need to look at the previous condition:

   if ((*arg == '-' || *arg == '#') && arg[1]) {
       ERROR-RETURN

> And particularly, I can easily see how one might want to prefix an
> environment variable with a minus to *substract* it from the set of
> accepted vars.

You like in SendEnv.  I decded against doing this because the number of
envvars to send here should be pretty limited and does not need for more
complex code.

> +                        if (options->num_agent_env >= INT_MAX) {
>
> It is a bit strange to compare >= INT_MAX, since num_agent_env is an

Copied from SendEnv

> Bad indentation. send_env, num_setenv and setenv are indented with a
> tab, no_more_agent_env with 8 spaces, num_agent_env with 3 spaces and
> agent_env with a tab.

I guess I did not read the hacking guide completely.  Frankly I didn't
expect that any software these days still uses tabs to compress the
source.  I consider invisible characters a no-go unless POSIX says to
use them (Makefile ungliness).  Thanks for the patch in your other mail.

> The fixme comments of ssh-add.c and ssh-keygen.c also use 8 spaces
> instead of a tab (but these should probably end up implemented).

(See above.)


Any other technical opinions?



Salam-Shalom,

   Werner



-- 
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.

Attachment: signature.asc
Description: PGP signature

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

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

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux