fix(remote): harden udp bootstrap lifetime
- commit
- 982eec13dd6596c27479a5e38e9c799ca5b69b8e
- author
- brice · · 2026-07-06 21:18 UTC
- parent
- 283db0f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +85 -7
M internal/adapters/dgram/ssh_dialer.go +13 -0
| @@ -27,6 +27,7 @@ type bootstrapProcess interface { | |||
| 27 | 27 | StdoutPipe() (io.ReadCloser, error) | |
| 28 | 28 | Start() error | |
| 29 | 29 | Kill() error | |
| 30 | + | Wait() error | |
| 30 | 31 | } | |
| 31 | 32 | ||
| 32 | 33 | type execBootstrapProcess struct{ cmd *exec.Cmd } | |
| @@ -34,6 +35,7 @@ type execBootstrapProcess struct{ cmd *exec.Cmd } | |||
| 34 | 35 | func (p execBootstrapProcess) StdoutPipe() (io.ReadCloser, error) { return p.cmd.StdoutPipe() } | |
| 35 | 36 | func (p execBootstrapProcess) Start() error { return p.cmd.Start() } | |
| 36 | 37 | func (p execBootstrapProcess) Kill() error { return p.cmd.Process.Kill() } | |
| 38 | + | func (p execBootstrapProcess) Wait() error { return p.cmd.Wait() } | |
| 37 | 39 | ||
| 38 | 40 | var ( | |
| 39 | 41 | startUDPBootstrap = func(ctx context.Context, target, session string, stderr io.Writer) bootstrapProcess { | |
| @@ -98,10 +100,14 @@ func (d RemoteDialer) Dial(ctx context.Context) (ports.Transport, error) { | |||
| 98 | 100 | return nil, udpUnavailable("start bootstrap", err, &stderr) | |
| 99 | 101 | } | |
| 100 | 102 | cleanup := true | |
| 103 | + | waited := false | |
| 101 | 104 | defer func() { | |
| 102 | 105 | if cleanup { | |
| 103 | 106 | _ = proc.Kill() | |
| 104 | 107 | } | |
| 108 | + | if !waited { | |
| 109 | + | _ = proc.Wait() | |
| 110 | + | } | |
| 105 | 111 | _ = stdout.Close() | |
| 106 | 112 | }() | |
| 107 | 113 | ||
| @@ -109,6 +115,13 @@ func (d RemoteDialer) Dial(ctx context.Context) (ports.Transport, error) { | |||
| 109 | 115 | if err != nil { | |
| 110 | 116 | return nil, udpUnavailable("read bootstrap readiness", err, &stderr) | |
| 111 | 117 | } | |
| 118 | + | if err := proc.Wait(); err != nil { | |
| 119 | + | waited = true | |
| 120 | + | cleanup = false | |
| 121 | + | return nil, udpUnavailable("wait bootstrap", err, &stderr) | |
| 122 | + | } | |
| 123 | + | waited = true | |
| 124 | + | cleanup = false | |
| 112 | 125 | peer, err := net.ResolveUDPAddr("udp", net.JoinHostPort(sshTargetHost(d.Target), strconv.Itoa(ready.port))) | |
| 113 | 126 | if err != nil { | |
| 114 | 127 | return nil, udpUnavailable("resolve UDP peer", err, &stderr) | |
| @@ -27,6 +27,7 @@ type bootstrapProcess interface { | |||||
| 27 | StdoutPipe() (io.ReadCloser, error) | 27 | StdoutPipe() (io.ReadCloser, error) | ||
| 28 | Start() error | 28 | Start() error | ||
| 29 | Kill() error | 29 | Kill() error | ||
| 30 | + | Wait() error | |||
| 30 | } | 31 | } | ||
| 31 | 32 | ||||
| 32 | type execBootstrapProcess struct{ cmd *exec.Cmd } | 33 | type execBootstrapProcess struct{ cmd *exec.Cmd } | ||
| @@ -34,6 +35,7 @@ type execBootstrapProcess struct{ cmd *exec.Cmd } | |||||
| 34 | func (p execBootstrapProcess) StdoutPipe() (io.ReadCloser, error) { return p.cmd.StdoutPipe() } | 35 | func (p execBootstrapProcess) StdoutPipe() (io.ReadCloser, error) { return p.cmd.StdoutPipe() } | ||
| 35 | func (p execBootstrapProcess) Start() error { return p.cmd.Start() } | 36 | func (p execBootstrapProcess) Start() error { return p.cmd.Start() } | ||
| 36 | func (p execBootstrapProcess) Kill() error { return p.cmd.Process.Kill() } | 37 | func (p execBootstrapProcess) Kill() error { return p.cmd.Process.Kill() } | ||
| 38 | + | func (p execBootstrapProcess) Wait() error { return p.cmd.Wait() } | |||
| 37 | 39 | ||||
| 38 | var ( | 40 | var ( | ||
| 39 | startUDPBootstrap = func(ctx context.Context, target, session string, stderr io.Writer) bootstrapProcess { | 41 | startUDPBootstrap = func(ctx context.Context, target, session string, stderr io.Writer) bootstrapProcess { | ||
| @@ -98,10 +100,14 @@ func (d RemoteDialer) Dial(ctx context.Context) (ports.Transport, error) { | |||||
| 98 | return nil, udpUnavailable("start bootstrap", err, &stderr) | 100 | return nil, udpUnavailable("start bootstrap", err, &stderr) | ||
| 99 | } | 101 | } | ||
| 100 | cleanup := true | 102 | cleanup := true | ||
| 103 | + | waited := false | |||
| 101 | defer func() { | 104 | defer func() { | ||
| 102 | if cleanup { | 105 | if cleanup { | ||
| 103 | _ = proc.Kill() | 106 | _ = proc.Kill() | ||
| 104 | } | 107 | } | ||
| 108 | + | if !waited { | |||
| 109 | + | _ = proc.Wait() | |||
| 110 | + | } | |||
| 105 | _ = stdout.Close() | 111 | _ = stdout.Close() | ||
| 106 | }() | 112 | }() | ||
| 107 | 113 | ||||
| @@ -109,6 +115,13 @@ func (d RemoteDialer) Dial(ctx context.Context) (ports.Transport, error) { | |||||
| 109 | if err != nil { | 115 | if err != nil { | ||
| 110 | return nil, udpUnavailable("read bootstrap readiness", err, &stderr) | 116 | return nil, udpUnavailable("read bootstrap readiness", err, &stderr) | ||
| 111 | } | 117 | } | ||
| 118 | + | if err := proc.Wait(); err != nil { | |||
| 119 | + | waited = true | |||
| 120 | + | cleanup = false | |||
| 121 | + | return nil, udpUnavailable("wait bootstrap", err, &stderr) | |||
| 122 | + | } | |||
| 123 | + | waited = true | |||
| 124 | + | cleanup = false | |||
| 112 | peer, err := net.ResolveUDPAddr("udp", net.JoinHostPort(sshTargetHost(d.Target), strconv.Itoa(ready.port))) | 125 | peer, err := net.ResolveUDPAddr("udp", net.JoinHostPort(sshTargetHost(d.Target), strconv.Itoa(ready.port))) | ||
| 113 | if err != nil { | 126 | if err != nil { | ||
| 114 | return nil, udpUnavailable("resolve UDP peer", err, &stderr) | 127 | return nil, udpUnavailable("resolve UDP peer", err, &stderr) | ||
M internal/adapters/dgram/ssh_dialer_test.go +33 -4
| @@ -85,12 +85,15 @@ func TestStdioDialerUsesContextBeforeStartingSSH(t *testing.T) { | |||
| 85 | 85 | type fakeBootstrapProcess struct { | |
| 86 | 86 | stdout io.ReadCloser | |
| 87 | 87 | startErr error | |
| 88 | + | waitErr error | |
| 88 | 89 | killed bool | |
| 90 | + | waited bool | |
| 89 | 91 | } | |
| 90 | 92 | ||
| 91 | 93 | func (p *fakeBootstrapProcess) StdoutPipe() (io.ReadCloser, error) { return p.stdout, nil } | |
| 92 | 94 | func (p *fakeBootstrapProcess) Start() error { return p.startErr } | |
| 93 | 95 | func (p *fakeBootstrapProcess) Kill() error { p.killed = true; return nil } | |
| 96 | + | func (p *fakeBootstrapProcess) Wait() error { p.waited = true; return p.waitErr } | |
| 94 | 97 | ||
| 95 | 98 | func withBootstrapStarter(t *testing.T, fn func(context.Context, string, string, io.Writer) bootstrapProcess) { | |
| 96 | 99 | t.Helper() | |
| @@ -115,14 +118,19 @@ func TestRemoteDialerUDPBootstrapFailures(t *testing.T) { | |||
| 115 | 118 | listen error | |
| 116 | 119 | want string | |
| 117 | 120 | killed bool | |
| 121 | + | waited bool | |
| 118 | 122 | }{ | |
| 119 | 123 | {name: "start failure", start: errors.New("ssh missing"), want: "vev: remote UDP transport unavailable: start bootstrap: ssh missing"}, | |
| 120 | - | {name: "malformed readiness", stdout: "hello\n", want: "malformed readiness line", killed: true}, | |
| 121 | - | {name: "packet listen failure", stdout: "VEV-UDP 4444 " + key + "\n", listen: errors.New("bind denied"), want: "listen UDP: bind denied", killed: true}, | |
| 124 | + | {name: "malformed readiness", stdout: "hello\n", want: "malformed readiness line", killed: true, waited: true}, | |
| 125 | + | {name: "bootstrap wait failure", stdout: "VEV-UDP 4444 " + key + "\n", want: "wait bootstrap: exit status 255", waited: true}, | |
| 126 | + | {name: "packet listen failure", stdout: "VEV-UDP 4444 " + key + "\n", listen: errors.New("bind denied"), want: "listen UDP: bind denied", waited: true}, | |
| 122 | 127 | } | |
| 123 | 128 | for _, tt := range tests { | |
| 124 | 129 | t.Run(tt.name, func(t *testing.T) { | |
| 125 | 130 | proc := &fakeBootstrapProcess{stdout: io.NopCloser(strings.NewReader(tt.stdout)), startErr: tt.start} | |
| 131 | + | if tt.name == "bootstrap wait failure" { | |
| 132 | + | proc.waitErr = errors.New("exit status 255") | |
| 133 | + | } | |
| 126 | 134 | withBootstrapStarter(t, func(context.Context, string, string, io.Writer) bootstrapProcess { return proc }) | |
| 127 | 135 | if tt.listen != nil { | |
| 128 | 136 | withListenUDP(t, func(context.Context) (net.PacketConn, error) { return nil, tt.listen }) | |
| @@ -137,10 +145,28 @@ func TestRemoteDialerUDPBootstrapFailures(t *testing.T) { | |||
| 137 | 145 | if proc.killed != tt.killed { | |
| 138 | 146 | t.Fatalf("killed=%v, want %v", proc.killed, tt.killed) | |
| 139 | 147 | } | |
| 148 | + | if proc.waited != tt.waited { | |
| 149 | + | t.Fatalf("waited=%v, want %v", proc.waited, tt.waited) | |
| 150 | + | } | |
| 140 | 151 | }) | |
| 141 | 152 | } | |
| 142 | 153 | } | |
| 143 | 154 | ||
| 155 | + | func TestListenUDPPacketUsesDualStackWildcard(t *testing.T) { | |
| 156 | + | pc, err := listenUDPPacket(context.Background()) | |
| 157 | + | if err != nil { | |
| 158 | + | t.Fatal(err) | |
| 159 | + | } | |
| 160 | + | defer func() { _ = pc.Close() }() | |
| 161 | + | addr, ok := pc.LocalAddr().(*net.UDPAddr) | |
| 162 | + | if !ok { | |
| 163 | + | t.Fatalf("local addr = %T, want *net.UDPAddr", pc.LocalAddr()) | |
| 164 | + | } | |
| 165 | + | if !addr.IP.IsUnspecified() { | |
| 166 | + | t.Fatalf("listen IP = %v, want unspecified wildcard", addr.IP) | |
| 167 | + | } | |
| 168 | + | } | |
| 169 | + | ||
| 144 | 170 | func TestRemoteDialerProbeFailureCleansUpWithoutStdioFallback(t *testing.T) { | |
| 145 | 171 | key := base64.StdEncoding.EncodeToString(make([]byte, pdgram.KeySize)) | |
| 146 | 172 | proc := &fakeBootstrapProcess{stdout: io.NopCloser(strings.NewReader("VEV-UDP 4444 " + key + "\n"))} | |
| @@ -170,8 +196,11 @@ func TestRemoteDialerProbeFailureCleansUpWithoutStdioFallback(t *testing.T) { | |||
| 170 | 196 | if !closed { | |
| 171 | 197 | t.Fatal("packet conn not closed after probe failure") | |
| 172 | 198 | } | |
| 173 | - | if !proc.killed { | |
| 174 | - | t.Fatal("bootstrap process not killed after probe failure") | |
| 199 | + | if proc.killed { | |
| 200 | + | t.Fatal("bootstrap process killed after it was already waited") | |
| 201 | + | } | |
| 202 | + | if !proc.waited { | |
| 203 | + | t.Fatal("bootstrap process not waited after probe failure") | |
| 175 | 204 | } | |
| 176 | 205 | } | |
| 177 | 206 | ||
| @@ -85,12 +85,15 @@ func TestStdioDialerUsesContextBeforeStartingSSH(t *testing.T) { | |||||
| 85 | type fakeBootstrapProcess struct { | 85 | type fakeBootstrapProcess struct { | ||
| 86 | stdout io.ReadCloser | 86 | stdout io.ReadCloser | ||
| 87 | startErr error | 87 | startErr error | ||
| 88 | + | waitErr error | |||
| 88 | killed bool | 89 | killed bool | ||
| 90 | + | waited bool | |||
| 89 | } | 91 | } | ||
| 90 | 92 | ||||
| 91 | func (p *fakeBootstrapProcess) StdoutPipe() (io.ReadCloser, error) { return p.stdout, nil } | 93 | func (p *fakeBootstrapProcess) StdoutPipe() (io.ReadCloser, error) { return p.stdout, nil } | ||
| 92 | func (p *fakeBootstrapProcess) Start() error { return p.startErr } | 94 | func (p *fakeBootstrapProcess) Start() error { return p.startErr } | ||
| 93 | func (p *fakeBootstrapProcess) Kill() error { p.killed = true; return nil } | 95 | func (p *fakeBootstrapProcess) Kill() error { p.killed = true; return nil } | ||
| 96 | + | func (p *fakeBootstrapProcess) Wait() error { p.waited = true; return p.waitErr } | |||
| 94 | 97 | ||||
| 95 | func withBootstrapStarter(t *testing.T, fn func(context.Context, string, string, io.Writer) bootstrapProcess) { | 98 | func withBootstrapStarter(t *testing.T, fn func(context.Context, string, string, io.Writer) bootstrapProcess) { | ||
| 96 | t.Helper() | 99 | t.Helper() | ||
| @@ -115,14 +118,19 @@ func TestRemoteDialerUDPBootstrapFailures(t *testing.T) { | |||||
| 115 | listen error | 118 | listen error | ||
| 116 | want string | 119 | want string | ||
| 117 | killed bool | 120 | killed bool | ||
| 121 | + | waited bool | |||
| 118 | }{ | 122 | }{ | ||
| 119 | {name: "start failure", start: errors.New("ssh missing"), want: "vev: remote UDP transport unavailable: start bootstrap: ssh missing"}, | 123 | {name: "start failure", start: errors.New("ssh missing"), want: "vev: remote UDP transport unavailable: start bootstrap: ssh missing"}, | ||
| 120 | - | {name: "malformed readiness", stdout: "hello\n", want: "malformed readiness line", killed: true}, | 124 | + | {name: "malformed readiness", stdout: "hello\n", want: "malformed readiness line", killed: true, waited: true}, |
| 121 | - | {name: "packet listen failure", stdout: "VEV-UDP 4444 " + key + "\n", listen: errors.New("bind denied"), want: "listen UDP: bind denied", killed: true}, | 125 | + | {name: "bootstrap wait failure", stdout: "VEV-UDP 4444 " + key + "\n", want: "wait bootstrap: exit status 255", waited: true}, |
| 126 | + | {name: "packet listen failure", stdout: "VEV-UDP 4444 " + key + "\n", listen: errors.New("bind denied"), want: "listen UDP: bind denied", waited: true}, | |||
| 122 | } | 127 | } | ||
| 123 | for _, tt := range tests { | 128 | for _, tt := range tests { | ||
| 124 | t.Run(tt.name, func(t *testing.T) { | 129 | t.Run(tt.name, func(t *testing.T) { | ||
| 125 | proc := &fakeBootstrapProcess{stdout: io.NopCloser(strings.NewReader(tt.stdout)), startErr: tt.start} | 130 | proc := &fakeBootstrapProcess{stdout: io.NopCloser(strings.NewReader(tt.stdout)), startErr: tt.start} | ||
| 131 | + | if tt.name == "bootstrap wait failure" { | |||
| 132 | + | proc.waitErr = errors.New("exit status 255") | |||
| 133 | + | } | |||
| 126 | withBootstrapStarter(t, func(context.Context, string, string, io.Writer) bootstrapProcess { return proc }) | 134 | withBootstrapStarter(t, func(context.Context, string, string, io.Writer) bootstrapProcess { return proc }) | ||
| 127 | if tt.listen != nil { | 135 | if tt.listen != nil { | ||
| 128 | withListenUDP(t, func(context.Context) (net.PacketConn, error) { return nil, tt.listen }) | 136 | withListenUDP(t, func(context.Context) (net.PacketConn, error) { return nil, tt.listen }) | ||
| @@ -137,10 +145,28 @@ func TestRemoteDialerUDPBootstrapFailures(t *testing.T) { | |||||
| 137 | if proc.killed != tt.killed { | 145 | if proc.killed != tt.killed { | ||
| 138 | t.Fatalf("killed=%v, want %v", proc.killed, tt.killed) | 146 | t.Fatalf("killed=%v, want %v", proc.killed, tt.killed) | ||
| 139 | } | 147 | } | ||
| 148 | + | if proc.waited != tt.waited { | |||
| 149 | + | t.Fatalf("waited=%v, want %v", proc.waited, tt.waited) | |||
| 150 | + | } | |||
| 140 | }) | 151 | }) | ||
| 141 | } | 152 | } | ||
| 142 | } | 153 | } | ||
| 143 | 154 | ||||
| 155 | + | func TestListenUDPPacketUsesDualStackWildcard(t *testing.T) { | |||
| 156 | + | pc, err := listenUDPPacket(context.Background()) | |||
| 157 | + | if err != nil { | |||
| 158 | + | t.Fatal(err) | |||
| 159 | + | } | |||
| 160 | + | defer func() { _ = pc.Close() }() | |||
| 161 | + | addr, ok := pc.LocalAddr().(*net.UDPAddr) | |||
| 162 | + | if !ok { | |||
| 163 | + | t.Fatalf("local addr = %T, want *net.UDPAddr", pc.LocalAddr()) | |||
| 164 | + | } | |||
| 165 | + | if !addr.IP.IsUnspecified() { | |||
| 166 | + | t.Fatalf("listen IP = %v, want unspecified wildcard", addr.IP) | |||
| 167 | + | } | |||
| 168 | + | } | |||
| 169 | + | ||||
| 144 | func TestRemoteDialerProbeFailureCleansUpWithoutStdioFallback(t *testing.T) { | 170 | func TestRemoteDialerProbeFailureCleansUpWithoutStdioFallback(t *testing.T) { | ||
| 145 | key := base64.StdEncoding.EncodeToString(make([]byte, pdgram.KeySize)) | 171 | key := base64.StdEncoding.EncodeToString(make([]byte, pdgram.KeySize)) | ||
| 146 | proc := &fakeBootstrapProcess{stdout: io.NopCloser(strings.NewReader("VEV-UDP 4444 " + key + "\n"))} | 172 | proc := &fakeBootstrapProcess{stdout: io.NopCloser(strings.NewReader("VEV-UDP 4444 " + key + "\n"))} | ||
| @@ -170,8 +196,11 @@ func TestRemoteDialerProbeFailureCleansUpWithoutStdioFallback(t *testing.T) { | |||||
| 170 | if !closed { | 196 | if !closed { | ||
| 171 | t.Fatal("packet conn not closed after probe failure") | 197 | t.Fatal("packet conn not closed after probe failure") | ||
| 172 | } | 198 | } | ||
| 173 | - | if !proc.killed { | 199 | + | if proc.killed { |
| 174 | - | t.Fatal("bootstrap process not killed after probe failure") | 200 | + | t.Fatal("bootstrap process killed after it was already waited") |
| 201 | + | } | |||
| 202 | + | if !proc.waited { | |||
| 203 | + | t.Fatal("bootstrap process not waited after probe failure") | |||
| 175 | } | 204 | } | ||
| 176 | } | 205 | } | ||
| 177 | 206 | ||||
M internal/app/remote.go +16 -0
| @@ -11,6 +11,22 @@ func parseRemoteAttachTarget(s string) (target, session string, ok bool) { | |||
| 11 | 11 | return "", "", false | |
| 12 | 12 | } | |
| 13 | 13 | afterAt := s[at+1:] | |
| 14 | + | if strings.HasPrefix(afterAt, "[") { | |
| 15 | + | closeBracket := strings.IndexByte(afterAt, ']') | |
| 16 | + | if closeBracket < 0 { | |
| 17 | + | return "", "", false | |
| 18 | + | } | |
| 19 | + | suffix := afterAt[closeBracket+1:] | |
| 20 | + | switch { | |
| 21 | + | case suffix == "": | |
| 22 | + | return s, "", true | |
| 23 | + | case strings.HasPrefix(suffix, ":"): | |
| 24 | + | targetEnd := at + 1 + closeBracket + 1 | |
| 25 | + | return s[:targetEnd], suffix[1:], true | |
| 26 | + | default: | |
| 27 | + | return "", "", false | |
| 28 | + | } | |
| 29 | + | } | |
| 14 | 30 | colon := strings.LastIndexByte(afterAt, ':') | |
| 15 | 31 | if colon < 0 { | |
| 16 | 32 | return s, "", true | |
| @@ -11,6 +11,22 @@ func parseRemoteAttachTarget(s string) (target, session string, ok bool) { | |||||
| 11 | return "", "", false | 11 | return "", "", false | ||
| 12 | } | 12 | } | ||
| 13 | afterAt := s[at+1:] | 13 | afterAt := s[at+1:] | ||
| 14 | + | if strings.HasPrefix(afterAt, "[") { | |||
| 15 | + | closeBracket := strings.IndexByte(afterAt, ']') | |||
| 16 | + | if closeBracket < 0 { | |||
| 17 | + | return "", "", false | |||
| 18 | + | } | |||
| 19 | + | suffix := afterAt[closeBracket+1:] | |||
| 20 | + | switch { | |||
| 21 | + | case suffix == "": | |||
| 22 | + | return s, "", true | |||
| 23 | + | case strings.HasPrefix(suffix, ":"): | |||
| 24 | + | targetEnd := at + 1 + closeBracket + 1 | |||
| 25 | + | return s[:targetEnd], suffix[1:], true | |||
| 26 | + | default: | |||
| 27 | + | return "", "", false | |||
| 28 | + | } | |||
| 29 | + | } | |||
| 14 | colon := strings.LastIndexByte(afterAt, ':') | 30 | colon := strings.LastIndexByte(afterAt, ':') | ||
| 15 | if colon < 0 { | 31 | if colon < 0 { | ||
| 16 | return s, "", true | 32 | return s, "", true | ||
M internal/app/remote_test.go +3 -0
| @@ -17,6 +17,9 @@ func TestParseRemoteAttachTarget(t *testing.T) { | |||
| 17 | 17 | {name: "remote no session", input: "user@example.com", wantTarget: "user@example.com", wantOK: true}, | |
| 18 | 18 | {name: "remote with session", input: "user@example.com:work", wantTarget: "user@example.com", wantSession: "work", wantOK: true}, | |
| 19 | 19 | {name: "remote empty session allowed", input: "user@example.com:", wantTarget: "user@example.com", wantOK: true}, | |
| 20 | + | {name: "bracketed ipv6 no session", input: "user@[2001:db8::1]", wantTarget: "user@[2001:db8::1]", wantOK: true}, | |
| 21 | + | {name: "bracketed ipv6 with session", input: "user@[2001:db8::1]:work", wantTarget: "user@[2001:db8::1]", wantSession: "work", wantOK: true}, | |
| 22 | + | {name: "malformed bracketed ipv6", input: "user@[2001:db8::1"}, | |
| 20 | 23 | {name: "target with at in userinfo", input: "first@user@example.com:work", wantTarget: "first@user@example.com", wantSession: "work", wantOK: true}, | |
| 21 | 24 | } | |
| 22 | 25 | ||
| @@ -17,6 +17,9 @@ func TestParseRemoteAttachTarget(t *testing.T) { | |||||
| 17 | {name: "remote no session", input: "user@example.com", wantTarget: "user@example.com", wantOK: true}, | 17 | {name: "remote no session", input: "user@example.com", wantTarget: "user@example.com", wantOK: true}, | ||
| 18 | {name: "remote with session", input: "user@example.com:work", wantTarget: "user@example.com", wantSession: "work", wantOK: true}, | 18 | {name: "remote with session", input: "user@example.com:work", wantTarget: "user@example.com", wantSession: "work", wantOK: true}, | ||
| 19 | {name: "remote empty session allowed", input: "user@example.com:", wantTarget: "user@example.com", wantOK: true}, | 19 | {name: "remote empty session allowed", input: "user@example.com:", wantTarget: "user@example.com", wantOK: true}, | ||
| 20 | + | {name: "bracketed ipv6 no session", input: "user@[2001:db8::1]", wantTarget: "user@[2001:db8::1]", wantOK: true}, | |||
| 21 | + | {name: "bracketed ipv6 with session", input: "user@[2001:db8::1]:work", wantTarget: "user@[2001:db8::1]", wantSession: "work", wantOK: true}, | |||
| 22 | + | {name: "malformed bracketed ipv6", input: "user@[2001:db8::1"}, | |||
| 20 | {name: "target with at in userinfo", input: "first@user@example.com:work", wantTarget: "first@user@example.com", wantSession: "work", wantOK: true}, | 23 | {name: "target with at in userinfo", input: "first@user@example.com:work", wantTarget: "first@user@example.com", wantSession: "work", wantOK: true}, | ||
| 21 | } | 24 | } | ||
| 22 | 25 | ||||
M internal/app/run.go +12 -3
| @@ -646,10 +646,17 @@ func runUDPBootstrap(ctx context.Context, session string) error { | |||
| 646 | 646 | if session != "" { | |
| 647 | 647 | args = append(args, session) | |
| 648 | 648 | } | |
| 649 | + | devNull, err := os.OpenFile(os.DevNull, os.O_RDWR, 0) | |
| 650 | + | if err != nil { | |
| 651 | + | _ = w.Close() | |
| 652 | + | return err | |
| 653 | + | } | |
| 654 | + | defer func() { _ = devNull.Close() }() | |
| 655 | + | ||
| 649 | 656 | cmd := udpProxyCommand(ctx, exe, args...) | |
| 650 | - | cmd.Stdin = nil | |
| 657 | + | cmd.Stdin = devNull | |
| 651 | 658 | cmd.Stdout = w | |
| 652 | - | cmd.Stderr = os.Stderr | |
| 659 | + | cmd.Stderr = devNull | |
| 653 | 660 | cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} | |
| 654 | 661 | if err := cmd.Start(); err != nil { | |
| 655 | 662 | _ = w.Close() | |
| @@ -677,9 +684,11 @@ func runUDPBootstrap(ctx context.Context, session string) error { | |||
| 677 | 684 | _, err := fmt.Fprint(os.Stdout, line) | |
| 678 | 685 | return err | |
| 679 | 686 | case err := <-errCh: | |
| 687 | + | _ = cmd.Wait() | |
| 680 | 688 | return err | |
| 681 | 689 | case <-readyCtx.Done(): | |
| 682 | 690 | _ = cmd.Process.Kill() | |
| 691 | + | _ = cmd.Wait() | |
| 683 | 692 | return fmt.Errorf("vev: udp bootstrap readiness: %w", readyCtx.Err()) | |
| 684 | 693 | } | |
| 685 | 694 | } | |
| @@ -694,7 +703,7 @@ func runUDPProxy(ctx context.Context, session string, ready io.Writer) error { | |||
| 694 | 703 | log.Debug("udp proxy starting", "session", session) | |
| 695 | 704 | ||
| 696 | 705 | var lc net.ListenConfig | |
| 697 | - | conn, err := lc.ListenPacket(ctx, "udp", "0.0.0.0:0") | |
| 706 | + | conn, err := lc.ListenPacket(ctx, "udp", ":0") | |
| 698 | 707 | if err != nil { | |
| 699 | 708 | return err | |
| 700 | 709 | } | |
| @@ -646,10 +646,17 @@ func runUDPBootstrap(ctx context.Context, session string) error { | |||||
| 646 | if session != "" { | 646 | if session != "" { | ||
| 647 | args = append(args, session) | 647 | args = append(args, session) | ||
| 648 | } | 648 | } | ||
| 649 | + | devNull, err := os.OpenFile(os.DevNull, os.O_RDWR, 0) | |||
| 650 | + | if err != nil { | |||
| 651 | + | _ = w.Close() | |||
| 652 | + | return err | |||
| 653 | + | } | |||
| 654 | + | defer func() { _ = devNull.Close() }() | |||
| 655 | + | ||||
| 649 | cmd := udpProxyCommand(ctx, exe, args...) | 656 | cmd := udpProxyCommand(ctx, exe, args...) | ||
| 650 | - | cmd.Stdin = nil | 657 | + | cmd.Stdin = devNull |
| 651 | cmd.Stdout = w | 658 | cmd.Stdout = w | ||
| 652 | - | cmd.Stderr = os.Stderr | 659 | + | cmd.Stderr = devNull |
| 653 | cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} | 660 | cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} | ||
| 654 | if err := cmd.Start(); err != nil { | 661 | if err := cmd.Start(); err != nil { | ||
| 655 | _ = w.Close() | 662 | _ = w.Close() | ||
| @@ -677,9 +684,11 @@ func runUDPBootstrap(ctx context.Context, session string) error { | |||||
| 677 | _, err := fmt.Fprint(os.Stdout, line) | 684 | _, err := fmt.Fprint(os.Stdout, line) | ||
| 678 | return err | 685 | return err | ||
| 679 | case err := <-errCh: | 686 | case err := <-errCh: | ||
| 687 | + | _ = cmd.Wait() | |||
| 680 | return err | 688 | return err | ||
| 681 | case <-readyCtx.Done(): | 689 | case <-readyCtx.Done(): | ||
| 682 | _ = cmd.Process.Kill() | 690 | _ = cmd.Process.Kill() | ||
| 691 | + | _ = cmd.Wait() | |||
| 683 | return fmt.Errorf("vev: udp bootstrap readiness: %w", readyCtx.Err()) | 692 | return fmt.Errorf("vev: udp bootstrap readiness: %w", readyCtx.Err()) | ||
| 684 | } | 693 | } | ||
| 685 | } | 694 | } | ||
| @@ -694,7 +703,7 @@ func runUDPProxy(ctx context.Context, session string, ready io.Writer) error { | |||||
| 694 | log.Debug("udp proxy starting", "session", session) | 703 | log.Debug("udp proxy starting", "session", session) | ||
| 695 | 704 | ||||
| 696 | var lc net.ListenConfig | 705 | var lc net.ListenConfig | ||
| 697 | - | conn, err := lc.ListenPacket(ctx, "udp", "0.0.0.0:0") | 706 | + | conn, err := lc.ListenPacket(ctx, "udp", ":0") |
| 698 | if err != nil { | 707 | if err != nil { | ||
| 699 | return err | 708 | return err | ||
| 700 | } | 709 | } | ||
M internal/app/run_test.go +8 -0
| @@ -575,9 +575,11 @@ func TestRunUDPBootstrapForwardsReadinessAndExits(t *testing.T) { | |||
| 575 | 575 | t.Fatal(err) | |
| 576 | 576 | } | |
| 577 | 577 | defer func() { _ = r.Close() }() | |
| 578 | + | var gotCmd *exec.Cmd | |
| 578 | 579 | udpProxyCommand = func(context.Context, string, ...string) *exec.Cmd { | |
| 579 | 580 | cmd := exec.Command("/bin/sh", "-c", "printf 'VEV-UDP 4242 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\\n'") | |
| 580 | 581 | cmd.Stdout = w | |
| 582 | + | gotCmd = cmd | |
| 581 | 583 | return cmd | |
| 582 | 584 | } | |
| 583 | 585 | t.Cleanup(func() { udpProxyCommand = oldCommand }) | |
| @@ -591,4 +593,10 @@ func TestRunUDPBootstrapForwardsReadinessAndExits(t *testing.T) { | |||
| 591 | 593 | if got != "VEV-UDP 4242 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\n" { | |
| 592 | 594 | t.Fatalf("readiness = %q", got) | |
| 593 | 595 | } | |
| 596 | + | if gotCmd.Stdin == nil || gotCmd.Stderr == nil { | |
| 597 | + | t.Fatalf("proxy stdio = stdin %T stderr %T, want detached from SSH channels", gotCmd.Stdin, gotCmd.Stderr) | |
| 598 | + | } | |
| 599 | + | if gotCmd.Stdout == os.Stdout || gotCmd.Stderr == os.Stderr || gotCmd.Stdin == os.Stdin { | |
| 600 | + | t.Fatalf("proxy inherited SSH stdio handles") | |
| 601 | + | } | |
| 594 | 602 | } | |
| @@ -575,9 +575,11 @@ func TestRunUDPBootstrapForwardsReadinessAndExits(t *testing.T) { | |||||
| 575 | t.Fatal(err) | 575 | t.Fatal(err) | ||
| 576 | } | 576 | } | ||
| 577 | defer func() { _ = r.Close() }() | 577 | defer func() { _ = r.Close() }() | ||
| 578 | + | var gotCmd *exec.Cmd | |||
| 578 | udpProxyCommand = func(context.Context, string, ...string) *exec.Cmd { | 579 | udpProxyCommand = func(context.Context, string, ...string) *exec.Cmd { | ||
| 579 | cmd := exec.Command("/bin/sh", "-c", "printf 'VEV-UDP 4242 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\\n'") | 580 | cmd := exec.Command("/bin/sh", "-c", "printf 'VEV-UDP 4242 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\\n'") | ||
| 580 | cmd.Stdout = w | 581 | cmd.Stdout = w | ||
| 582 | + | gotCmd = cmd | |||
| 581 | return cmd | 583 | return cmd | ||
| 582 | } | 584 | } | ||
| 583 | t.Cleanup(func() { udpProxyCommand = oldCommand }) | 585 | t.Cleanup(func() { udpProxyCommand = oldCommand }) | ||
| @@ -591,4 +593,10 @@ func TestRunUDPBootstrapForwardsReadinessAndExits(t *testing.T) { | |||||
| 591 | if got != "VEV-UDP 4242 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\n" { | 593 | if got != "VEV-UDP 4242 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\n" { | ||
| 592 | t.Fatalf("readiness = %q", got) | 594 | t.Fatalf("readiness = %q", got) | ||
| 593 | } | 595 | } | ||
| 596 | + | if gotCmd.Stdin == nil || gotCmd.Stderr == nil { | |||
| 597 | + | t.Fatalf("proxy stdio = stdin %T stderr %T, want detached from SSH channels", gotCmd.Stdin, gotCmd.Stderr) | |||
| 598 | + | } | |||
| 599 | + | if gotCmd.Stdout == os.Stdout || gotCmd.Stderr == os.Stderr || gotCmd.Stdin == os.Stdin { | |||
| 600 | + | t.Fatalf("proxy inherited SSH stdio handles") | |||
| 601 | + | } | |||
| 594 | } | 602 | } | ||