Skip to content

Commit 91c2e3d

Browse files
aiharosmjuraga
authored andcommitted
BUG/MEDIUM: runtime: ensure heredoc payloads have a blank-line terminator
When connected to HAProxy in master-worker mode, runtime_single_client.go appends ";quit\n" after the inner command. For "<command> <<\n%s\n" heredocs (set ssl cert, add ssl ca-file, set ssl crl-file, etc.) the resulting bytes on the wire are: <command> <<\n<payload>\n;quit\n HAProxy's `<<` heredoc terminates only on a blank line, so when the caller-supplied payload does not end with "\n" there is no blank line between payload and ";quit". HAProxy stalls reading the socket until the deadline fires, callers fall back to reload, and any layer with a tighter HTTP timeout above sees `context deadline exceeded` on the request. Normalize each heredoc payload via a new terminateHeredocPayload helper so the inner command always ends with "\n\n". Apply at every site that builds a `<<` heredoc: set ssl cert, set/add ssl ca-file, set ssl crl-file, set ssl ocsp-response, add map (versioned and not), and add ssl crt-list extended-form. Also fix the missing trailing "\n" in the SetCAFile format string so the new payload "\n" plus the format "\n" produce the blank line. Add TestHeredocPayloadAlwaysTerminated, which captures the bytes written to a Unix socket for each affected command in both master-worker and single-process modes and asserts a blank-line terminator appears before any ";" character. Reverting any single fix in this commit makes the corresponding subtest fail. Reproducible on HAProxy 3.2 in master-worker mode by speaking the master socket directly with the byte sequence this code previously produced.
1 parent da7cd00 commit 91c2e3d

9 files changed

Lines changed: 256 additions & 7 deletions

File tree

.aspell.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ allowed:
102102
- gptstr
103103
- hapee
104104
- haproxy
105+
- heredoc
106+
- heredocs
105107
- healthcheck
106108
- healthz
107109
- hostname
@@ -191,6 +193,7 @@ allowed:
191193
- subnet
192194
- subresource
193195
- subresources
196+
- subtest
194197
- sudo
195198
- symlinks
196199
- syslog

runtime/ca_files.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func (s *SingleRuntime) SetCAFile(caFile, payload string) error {
111111
if payload == "" {
112112
return fmt.Errorf("%s %w", "Argument payload empty", native_errors.ErrGeneral)
113113
}
114-
cmd := fmt.Sprintf("set ssl ca-file %s <<\n%s", caFile, payload)
114+
cmd := fmt.Sprintf("set ssl ca-file %s <<\n%s\n", caFile, terminateHeredocPayload(payload))
115115
response, err := s.ExecuteWithResponse(cmd)
116116
if err != nil {
117117
return fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)
@@ -172,7 +172,7 @@ func (s *SingleRuntime) DeleteCAFile(caFile string) error {
172172

173173
// AddCAFileEntry adds an entry into the CA file
174174
func (s *SingleRuntime) AddCAFileEntry(caFile, payload string) error {
175-
cmd := fmt.Sprintf("add ssl ca-file %s <<\n%s\n", caFile, payload)
175+
cmd := fmt.Sprintf("add ssl ca-file %s <<\n%s\n", caFile, terminateHeredocPayload(payload))
176176
response, err := s.ExecuteWithResponse(cmd)
177177
if err != nil {
178178
return fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)

runtime/certs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ func (s *SingleRuntime) SetCertificate(storageName string, payload string) error
209209
if payload == "" {
210210
return fmt.Errorf("%s %w", "Argument payload empty", native_errors.ErrGeneral)
211211
}
212-
cmd := fmt.Sprintf("set ssl cert %s <<\n%s\n", storageName, payload)
212+
cmd := fmt.Sprintf("set ssl cert %s <<\n%s\n", storageName, terminateHeredocPayload(payload))
213213
response, err := s.ExecuteWithResponse(cmd)
214214
if err != nil {
215215
return fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)

runtime/crl_files.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ func (s *SingleRuntime) SetCrlFile(crlFile string, payload string) error {
225225
if payload == "" {
226226
return fmt.Errorf("%s %w", "Argument payload empty", native_errors.ErrGeneral)
227227
}
228-
cmd := fmt.Sprintf("set ssl crl-file %s <<\n%s\n", crlFile, payload)
228+
cmd := fmt.Sprintf("set ssl crl-file %s <<\n%s\n", crlFile, terminateHeredocPayload(payload))
229229
response, err := s.ExecuteWithResponse(cmd)
230230
if err != nil {
231231
return fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)

runtime/crt_lists.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,13 @@ func (s *SingleRuntime) AddCrtListEntry(crtList string, entry models.SslCrtListE
189189
sb.WriteString(strings.Join(entry.SNIFilter, " "))
190190
}
191191
sb.WriteByte('\n')
192+
if extended {
193+
// HAProxy's `<<` heredoc terminates on a blank line; once the
194+
// outer framing in runtime_single_client.go appends `;quit\n`,
195+
// the lone `\n` above is consumed as the entry-line terminator
196+
// and there is no blank line left, so the heredoc never ends.
197+
sb.WriteByte('\n')
198+
}
192199

193200
response, err := s.ExecuteWithResponse(sb.String())
194201
if err != nil {

runtime/heredoc.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright 2025 HAProxy Technologies
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package runtime
16+
17+
import "strings"
18+
19+
// terminateHeredocPayload returns payload guaranteed to end with "\n".
20+
//
21+
// HAProxy's `<<` heredoc terminates only on a blank line. The conventional
22+
// command shape used in this package is "<command> <<\n<payload>\n", so for
23+
// the heredoc to terminate, payload itself must end with "\n" (producing
24+
// "\n\n" — a blank line — at the end of the command).
25+
//
26+
// Without that, the outer framing in runtime_single_client.go that appends
27+
// ";quit\n" (master-socket / worker > 0 path) places ";quit" immediately
28+
// after the payload's lone trailing newline — turning what should be the
29+
// blank-line terminator into a non-empty command line — and HAProxy then
30+
// blocks reading the socket until the deadline fires.
31+
func terminateHeredocPayload(payload string) string {
32+
if strings.HasSuffix(payload, "\n") {
33+
return payload
34+
}
35+
return payload + "\n"
36+
}

runtime/heredoc_test.go

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
// Copyright 2025 HAProxy Technologies
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package runtime
16+
17+
import (
18+
"net"
19+
"os"
20+
"strings"
21+
"sync"
22+
"testing"
23+
"time"
24+
25+
"github.com/haproxytech/client-native/v6/models"
26+
"github.com/haproxytech/client-native/v6/runtime/options"
27+
)
28+
29+
// captureSocket is a minimal Unix-socket server that records every byte
30+
// a client writes after immediately sending a fixed reply. It exists so
31+
// these tests can assert the exact bytes client-native sends to HAProxy's
32+
// runtime API for heredoc commands.
33+
type captureSocket struct {
34+
listener net.Listener
35+
reply string
36+
mu sync.Mutex
37+
received []byte
38+
addr string
39+
}
40+
41+
func newCaptureSocket(t *testing.T, reply string) *captureSocket {
42+
t.Helper()
43+
f, err := os.CreateTemp("", "client-native-cap-*")
44+
if err != nil {
45+
t.Fatalf("CreateTemp: %v", err)
46+
}
47+
addr := f.Name()
48+
_ = f.Close()
49+
_ = os.Remove(addr)
50+
l, err := net.Listen("unix", addr)
51+
if err != nil {
52+
t.Fatalf("Listen: %v", err)
53+
}
54+
s := &captureSocket{listener: l, reply: reply, addr: addr}
55+
go s.serve()
56+
t.Cleanup(func() { _ = l.Close(); _ = os.Remove(addr) })
57+
return s
58+
}
59+
60+
func (s *captureSocket) serve() {
61+
for {
62+
conn, err := s.listener.Accept()
63+
if err != nil {
64+
return
65+
}
66+
go func(c net.Conn) {
67+
defer c.Close()
68+
// Send reply right away so the client's read loop sees data
69+
// and unblocks; afterwards the client gets EOF when we close
70+
// at the deferred Close, which lets readFromSocket return.
71+
_, _ = c.Write([]byte(s.reply))
72+
// Drain everything the client sends for up to 500ms.
73+
_ = c.SetReadDeadline(time.Now().Add(500 * time.Millisecond))
74+
buf := make([]byte, 4096)
75+
var got []byte
76+
for {
77+
n, err := c.Read(buf)
78+
if n > 0 {
79+
got = append(got, buf[:n]...)
80+
}
81+
if err != nil {
82+
break
83+
}
84+
}
85+
s.mu.Lock()
86+
s.received = append(s.received, got...)
87+
s.mu.Unlock()
88+
}(conn)
89+
}
90+
}
91+
92+
func (s *captureSocket) Received() string {
93+
s.mu.Lock()
94+
defer s.mu.Unlock()
95+
return string(s.received)
96+
}
97+
98+
// assertHeredocTerminated checks that the bytes sent by the runtime client
99+
// contain a `<<\n` heredoc start AND that the payload after it is followed
100+
// by a blank line (\n\n) BEFORE any `;` command separator (which would
101+
// otherwise be interpreted as part of the heredoc body and stall HAProxy).
102+
func assertHeredocTerminated(t *testing.T, sent string) {
103+
t.Helper()
104+
idx := strings.Index(sent, "<<\n")
105+
if idx < 0 {
106+
t.Fatalf("no `<<\\n` heredoc start in sent bytes:\n%s", quoted(sent))
107+
}
108+
tail := sent[idx+len("<<\n"):]
109+
blankLine := strings.Index(tail, "\n\n")
110+
if blankLine < 0 {
111+
t.Fatalf("heredoc payload is never followed by a blank line; HAProxy will hang.\n sent: %s", quoted(sent))
112+
}
113+
if semi := strings.Index(tail, ";"); semi >= 0 && semi < blankLine {
114+
t.Fatalf("`;` appears before the blank-line terminator at offset %d (blank line at %d).\n Once the framing in runtime_single_client.go appends `;quit\\n`, HAProxy will interpret `;quit` as part of the heredoc body and stall.\n sent: %s",
115+
semi, blankLine, quoted(sent))
116+
}
117+
}
118+
119+
func quoted(s string) string { return `"` + strings.ReplaceAll(s, "\n", `\n`) + `"` }
120+
121+
// TestHeredocPayloadAlwaysTerminated drives each runtime command that
122+
// builds a `<<` heredoc with a payload that explicitly does NOT end in
123+
// "\n", in both master-worker and single-process framing modes, and
124+
// asserts the bytes actually written to the socket are properly
125+
// terminated. Catches the class of bug where a missing payload `\n`
126+
// combined with the master-worker `;quit\n` framing leaves the heredoc
127+
// without a blank-line terminator and HAProxy stalls until the socket
128+
// deadline fires.
129+
func TestHeredocPayloadAlwaysTerminated(t *testing.T) {
130+
const noTrailingNewline = "-----BEGIN CERTIFICATE-----\nMIIB...\n-----END CERTIFICATE-----"
131+
132+
commands := []struct {
133+
name string
134+
call func(s *SingleRuntime) error
135+
}{
136+
{
137+
name: "SetCertificate",
138+
call: func(s *SingleRuntime) error { return s.SetCertificate("/etc/cert.pem", noTrailingNewline) },
139+
},
140+
{
141+
name: "SetCAFile",
142+
call: func(s *SingleRuntime) error { return s.SetCAFile("/etc/ca.pem", noTrailingNewline) },
143+
},
144+
{
145+
name: "AddCAFileEntry",
146+
call: func(s *SingleRuntime) error { return s.AddCAFileEntry("/etc/ca.pem", noTrailingNewline) },
147+
},
148+
{
149+
name: "SetCrlFile",
150+
call: func(s *SingleRuntime) error { return s.SetCrlFile("/etc/crl.pem", noTrailingNewline) },
151+
},
152+
{
153+
name: "SetOcspResponse",
154+
call: func(s *SingleRuntime) error { return s.SetOcspResponse(noTrailingNewline) },
155+
},
156+
{
157+
name: "AddMapPayload",
158+
call: func(s *SingleRuntime) error { return s.AddMapPayload("/etc/map", "k1 v1\nk2 v2") },
159+
},
160+
{
161+
name: "AddMapPayloadVersioned",
162+
call: func(s *SingleRuntime) error { return s.AddMapPayloadVersioned("1", "/etc/map", "k1 v1\nk2 v2") },
163+
},
164+
{
165+
name: "AddCrtListEntry-extended",
166+
call: func(s *SingleRuntime) error {
167+
return s.AddCrtListEntry("/etc/crt-list", models.SslCrtListEntry{
168+
File: "/etc/cert.pem",
169+
SSLBindConfig: "verify required",
170+
})
171+
},
172+
},
173+
}
174+
175+
for _, mode := range []struct {
176+
name string
177+
masterWorkerMode bool
178+
}{
179+
{"master-worker", true},
180+
{"single-process", false},
181+
} {
182+
for _, cmd := range commands {
183+
t.Run(mode.name+"/"+cmd.name, func(t *testing.T) {
184+
mock := newCaptureSocket(t, " Transaction created for certificate /etc/cert.pem!\n\n")
185+
186+
s := &SingleRuntime{}
187+
if err := s.Init(mock.addr, mode.masterWorkerMode, options.RuntimeOptions{DoNotCheckRuntimeOnInit: true}); err != nil {
188+
t.Fatalf("Init: %v", err)
189+
}
190+
// Return value of the call is not checked: the mock doesn't
191+
// emit a real success response for every command shape, so
192+
// some calls legitimately error. What we care about is the
193+
// bytes that hit the wire before that.
194+
_ = cmd.call(s)
195+
196+
// Give the serve goroutine a moment to drain.
197+
time.Sleep(50 * time.Millisecond)
198+
199+
assertHeredocTerminated(t, mock.Received())
200+
})
201+
}
202+
}
203+
}

runtime/maps.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func (s *SingleRuntime) AddMapEntryVersioned(version, name, key, value string) e
230230
func (s *SingleRuntime) AddMapPayload(name, payload string) error {
231231
prefix := "<<\n"
232232
if len(payload) < len(prefix) || payload[0:len(prefix)] != prefix {
233-
payload = "<<\n" + payload + "\n"
233+
payload = "<<\n" + terminateHeredocPayload(payload) + "\n"
234234
}
235235
cmd := fmt.Sprintf("add map %s %s", name, payload)
236236
if err := s.Execute(cmd); err != nil {
@@ -259,7 +259,7 @@ func (s *SingleRuntime) PrepareMap(name string) (string, error) {
259259
func (s *SingleRuntime) AddMapPayloadVersioned(version, name, payload string) error {
260260
prefix := "<<\n"
261261
if len(payload) < len(prefix) || payload[0:len(prefix)] != prefix {
262-
payload = "<<\n" + payload + "\n"
262+
payload = "<<\n" + terminateHeredocPayload(payload) + "\n"
263263
}
264264
cmd := fmt.Sprintf("add map @%s %s %s", version, name, payload)
265265
if err := s.Execute(cmd); err != nil {

runtime/ocsp.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ func (s *SingleRuntime) SetOcspResponse(payload string) error {
241241
if payload == "" {
242242
return fmt.Errorf("%s %w", "Argument payload empty", native_errors.ErrGeneral)
243243
}
244-
cmd := fmt.Sprintf("set ssl ocsp-response <<\n%s\n", payload)
244+
cmd := fmt.Sprintf("set ssl ocsp-response <<\n%s\n", terminateHeredocPayload(payload))
245245
response, err := s.ExecuteWithResponse(cmd)
246246
if err != nil {
247247
return fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)

0 commit comments

Comments
 (0)