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