On 2021-01-28 at 09:24 +0100, Werner Koch via Gnupg-devel wrote:
> The rationale for the "-" thingy is to allow a config file to
> override what for example the command line has already set. The "#"
> can be used to disable a globally set option from the commandline or
> ~/.ssh/config.
This seems backwards. I would expect command line to have hidher
priority than ssh_config, not ~/.ssh/config to be able to disable an
explicit command line option.
However, this may be useful for ~/.ssh/configto override
/etc/ssh/ssh_config (or as a command line parameter -oAcceptEnv=-)
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")
On a quick review of the patch:
> @@ -313,11 +313,22 @@ static struct {
> { "proxyjump", oProxyJump },
> { "securitykeyprovider", oSecurityKeyProvider },
> { "knownhostscommand", oKnownHostsCommand },
> + { "agentenv", oAgentEnv },
There is an extra space identation here.
> + 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
whole block of variables and values, not just one variable.
> 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 -
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.
+ if (options->num_agent_env >= INT_MAX) {
It is a bit strange to compare >= INT_MAX, since num_agent_env is an
int, but after reviewing, you only need the == comparison, so probably
ok.
There are more indentation sheningans around this block.
> +++ b/readconf.h
> @@ -126,6 +126,10 @@ typedef struct {
> char **send_env;
> int num_setenv;
> char **setenv;
> + int no_more_agent_env;
> + int num_agent_env;
> + char **agent_env;
> +
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.
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).
> +++ b/sshconnect.c
> @@ -1718,6 +1718,12 @@ maybe_add_key_to_agent(const char *authfile,
> struct sshkey *private,
> }
> if (sshkey_is_sk(private))
> skprovider = options.sk_provider;
> + if ((r = ssh_send_agent_env (auth_sock, options.agent_env,
> + options.num_agent_env)) != 0) {
> + debug("agent does not support AgentEnv: (%d)", r);
> + close(auth_sock);
> + return;
> + }
Indentation with 8 spaces, not with tabs
> +++ b/sshconnect2.c
>
> +/* Helper for pubkey_prepare. */
> +static int
> +authentication_socket(int *fdp)
More indentation errors (there is a mixture in the function itself)
Best regards
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
