Builtin SSH server bug when client sends environment variables

[nicoulaj]

• Gitea version (or commit ref): 1.7.2
• Git version: 2.20.1
• OpenSSH version: 7.9p1
• Operating system: Linux on client, Docker official image on server
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
  • No
  • Not relevant
• Log gist:

Description

Using the builtin SSH server, I have this in my ~/.ssh/config on client side:

Host *
  SendEnv LANG LC_*

For an unrelated reason, LC_MESSAGES was set to an empty value. In this case all operations fail, with client side debug logs:

$ GIT_SSH_COMMAND="ssh -vvv" git clone [...]
debug1: Authentication succeeded (publickey).
Authenticated to [...].
[...]
debug1: Sending env LC_TELEPHONE = en_US.UTF-8
debug1: Sending env LC_MESSAGES = 
[...]
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Server side logs (trace level):

[...a/modules/ssh/ssh.go:64 func1()] [E] env: exit status 1

If I set LC_MESSAGES to a non empty value, it works but the server logs makes me think some parsing is happening on server side and it fails:

[W] SSH: Invalid env arguments: '[]string{"", "LC_MESSAGES", "en_US.UTF-8"}'
[W] SSH: Invalid env arguments: '[]string{"", "LC_MONETARY", "en_US.UTF-8"}'

Screenshots

N/A

[foxx1337]

The workaround for this is relatively simple. I just created an empty program called env.exe and placed it in the system-wide PATH. Gitea calls it at

gitea/modules/ssh/ssh.go

Line 62 in 2d213b6

_, _, err := com.ExecCmdBytes("env", args[0]+"="+args[1])

and nothing happens, connection continues without error and without the (desired on *nixes) env side effects.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

[sapk]

Just to be clear, this bug is only on windows only since they don't have an env binary ?

If it is the case, we just need to not rely on the env command and build a env variable array to propagate for future call of exec.

	env := append(os.Environ(),
		"LC_TELEPHONE=en_US.UTF-8",
	)

[sapk]

Rethinking of it, I think calling env doesn't have done anything previously to subsequent calls so I don't even know if we shouldn't totally ignore it.

[hacker]

Umm… ssh with LC_TERMINAL and LC_TERMINAL_VERSION (set by iTerm) doesn't work also with the dockerized version that has env binary?

[sapk]

@hacker the docker version use openssh and not the internal ssh server by default. Even configured with internal and with the env passed by ssh it should work. Just the env are ignored by internal ssh implementation (even previously removing it #6825)

[hacker]

@sapk it may be using openssh by default, but I did configure it to use inernal:

Escape character is '^]'.
SSH-2.0-Go

And yes, I understand you believe it should work, that's why I mentioned that. Moreover, setting LANG does not seem to hurt, whereas setting LC_TERMINAL does.

debug1: Sending environment.
debug1: Sending env LANG = en_US.UTF-8
debug1: Sending env LC_TERMINAL = zxc
debug1: Sending command: git-upload-pack '…'
debug1: channel 0: free: client-session, nchannels 1
debug1: fd 0 clearing O_NONBLOCK
Transferred: sent 3644, received 1848 bytes, in 0.0 seconds
Bytes per second: sent 372444.8, received 188879.8
debug1: Exit status -1
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

debug1: Sending environment.
debug1: Sending env LANG = en_US.UTF-8
debug1: Sending command: git-upload-pack '…'
debug1: client_input_channel_req: channel 0 rtype exit-status reply 0
debug1: channel 0: free: client-session, nchannels 1
debug1: fd 0 clearing O_NONBLOCK
Transferred: sent 3648, received 2304 bytes, in 0.2 seconds
Bytes per second: sent 18102.6, received 11433.2
debug1: Exit status 0

[sapk]

@hacker Can you re-try with the :latest docker build ? (published 9 hour ago)

[hacker]

Hey, yes, seems to do the trick.

[hacker]

Still wonder what was the reason. While ignoring env completely may be generally a good thing and the code removed made no sense whatsoever anyway and probably did also parsing wrong and running the input through cleanCommand which it shouldn't have (and you probably may want to watch out for that if you add more clauses to the switch or reintroduce env), I still don't get why would it choke on LC_TERMINAL and LC_TERMINAL_VERSION, but not on LANG, LC_ALL and even LC_SOMETHING (I've just moved back to 1.8.1 and tried out of curiosity). Running env LC_TERMINAL=… and stuff with gitea's docker container seems to be no problem.

The above is nothing but the curiosity though ;-)