fix!: validate HostnameSuffix and SSHConfigOptions' (#26154) (#26227) · coder/coder@fb52711
@@ -10,6 +10,8 @@ import (
1010"testing"
11111212"github.com/stretchr/testify/require"
13+14+"github.com/coder/coder/v2/codersdk"
1315)
14161517func Test_sshConfigSplitOnCoderSection(t *testing.T) {
@@ -302,6 +304,140 @@ func Test_sshConfigExecEscapeSeparatorForce(t *testing.T) {
302304 }
303305}
304306307+func Test_mergeSSHOptions_RejectsUnsafeServerConfig(t *testing.T) {
308+t.Parallel()
309+310+testCases := []struct {
311+name string
312+coderd codersdk.SSHConfigResponse
313+wantErr string
314+ }{
315+ {
316+name: "HostnameSuffix",
317+coderd: codersdk.SSHConfigResponse{
318+HostnameSuffix: "coder\nHost *",
319+ },
320+wantErr: "workspace hostname suffix",
321+ },
322+ {
323+name: "HostnamePrefix",
324+coderd: codersdk.SSHConfigResponse{
325+HostnamePrefix: "coder.\nHost *",
326+ },
327+wantErr: "workspace hostname prefix",
328+ },
329+ {
330+name: "ProxyCommand",
331+coderd: codersdk.SSHConfigResponse{
332+SSHConfigOptions: map[string]string{"ProxyCommand": "ssh -W %h:%p bastion"},
333+ },
334+wantErr: `ssh config option "ProxyCommand" is not allowed`,
335+ },
336+ {
337+name: "PermitLocalCommand",
338+coderd: codersdk.SSHConfigResponse{
339+SSHConfigOptions: map[string]string{"PermitLocalCommand": "yes"},
340+ },
341+wantErr: `ssh config option "PermitLocalCommand" is not allowed`,
342+ },
343+ {
344+name: "KnownHostsCommand",
345+coderd: codersdk.SSHConfigResponse{
346+SSHConfigOptions: map[string]string{"KnownHostsCommand": "echo key"},
347+ },
348+wantErr: `ssh config option "KnownHostsCommand" is not allowed`,
349+ },
350+ {
351+name: "PKCS11Provider",
352+coderd: codersdk.SSHConfigResponse{
353+SSHConfigOptions: map[string]string{"PKCS11Provider": "/tmp/evil.so"},
354+ },
355+wantErr: `ssh config option "PKCS11Provider" is not allowed`,
356+ },
357+ {
358+name: "NewlineInValue",
359+coderd: codersdk.SSHConfigResponse{
360+SSHConfigOptions: map[string]string{"UserKnownHostsFile": "/tmp/known_hosts\nHost *"},
361+ },
362+wantErr: `ssh config option "UserKnownHostsFile" must not contain carriage return, newline, or NUL characters`,
363+ },
364+ {
365+name: "SmartcardDevice",
366+coderd: codersdk.SSHConfigResponse{
367+SSHConfigOptions: map[string]string{"SmartcardDevice": "/path/to/lib"},
368+ },
369+wantErr: `not allowed`,
370+ },
371+ {
372+name: "XAuthLocation",
373+coderd: codersdk.SSHConfigResponse{
374+SSHConfigOptions: map[string]string{"XAuthLocation": "/usr/bin/xauth"},
375+ },
376+wantErr: `not allowed`,
377+ },
378+ {
379+name: "ProxyJump",
380+coderd: codersdk.SSHConfigResponse{
381+SSHConfigOptions: map[string]string{"ProxyJump": "bastion.example.com"},
382+ },
383+wantErr: `conflicts with`,
384+ },
385+ {
386+name: "HostnameSuffixGlob",
387+coderd: codersdk.SSHConfigResponse{
388+HostnameSuffix: "*",
389+ },
390+wantErr: `glob`,
391+ },
392+ }
393+394+for _, tt := range testCases {
395+t.Run(tt.name, func(t *testing.T) {
396+t.Parallel()
397+398+_, err := mergeSSHOptions(sshConfigOptions{}, tt.coderd, t.TempDir(), "/tmp/coder")
399+require.ErrorContains(t, err, tt.wantErr)
400+ })
401+ }
402+}
403+404+func Test_mergeSSHOptions_UserOptionsOverrideServerConfig(t *testing.T) {
405+t.Parallel()
406+407+user := sshConfigOptions{
408+userHostPrefix: "dev.",
409+hostnameSuffix: "local",
410+ }
411+got, err := mergeSSHOptions(user, codersdk.SSHConfigResponse{
412+HostnamePrefix: "coder.",
413+HostnameSuffix: "coder",
414+ }, t.TempDir(), "/tmp/coder")
415+require.NoError(t, err)
416+require.Equal(t, "dev.", got.userHostPrefix)
417+require.Equal(t, "local", got.hostnameSuffix)
418+}
419+420+func Test_mergeSSHOptions_AllowsSafeServerConfig(t *testing.T) {
421+t.Parallel()
422+423+got, err := mergeSSHOptions(sshConfigOptions{}, codersdk.SSHConfigResponse{
424+HostnamePrefix: "coder.",
425+HostnameSuffix: "coder",
426+SSHConfigOptions: map[string]string{
427+"HostName": "example.com",
428+"User": "coder",
429+"Port": "22",
430+"SetEnv": "FOO=bar BAZ=qux",
431+"UserKnownHostsFile": "/tmp/coder_known_hosts",
432+ },
433+ }, t.TempDir(), "/tmp/coder")
434+require.NoError(t, err)
435+require.Equal(t, "coder.", got.userHostPrefix)
436+require.Equal(t, "coder", got.hostnameSuffix)
437+require.Contains(t, got.sshOptions, "HostName example.com")
438+require.Contains(t, got.sshOptions, "SetEnv FOO=bar BAZ=qux")
439+}
440+305441func Test_sshConfigOptions_addOption(t *testing.T) {
306442t.Parallel()
307443testCases := []struct {