Skip to content

Commit 0fca608

Browse files
committed
webui: collect the support bundle with the support-collect RPC
The handler ran the tool itself, as root and past NACM, so any logged-in user could download the archive. The RPC runs as the user. Signed-off-by: Mattias Walström <lazzer@gmail.com>
1 parent 7a2ec72 commit 0fca608

9 files changed

Lines changed: 309 additions & 38 deletions

File tree

‎doc/ChangeLog.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ All notable changes to the project are documented in this file.
1919
- `support collect` now redacts private keys, password hashes and other
2020
secrets from the configuration files in the archive, use `--no-redact`
2121
to keep them. The environment dump is no longer collected
22+
- WebUI: the support bundle is collected with the `infix-system:support-collect`
23+
RPC as the logged-in user, so NACM decides who may download it, rather
24+
than by running the tool as root
2225

2326
### Added
2427

‎package/support/Config.in‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ config BR2_PACKAGE_SUPPORT
22
bool "support"
33
help
44
The support tool collects logs, configuration and system state
5-
into an archive for troubleshooting. It is called from the CLI,
6-
the WebUI and the infix-system:support-collect RPC.
5+
into an archive for troubleshooting. It is called from the CLI
6+
and the infix-system:support-collect RPC, which the WebUI uses.
77

88
https://github.com/kernelkit/infix
99

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
package handlers
2+
3+
import (
4+
"encoding/base64"
5+
"encoding/json"
6+
"io"
7+
"net/http"
8+
"net/http/httptest"
9+
"strconv"
10+
"strings"
11+
"testing"
12+
13+
"infix/webui/internal/restconf"
14+
)
15+
16+
// fakeSupportRPC serves infix-system:support-collect, recording the input
17+
// it received and answering with reply.
18+
func fakeSupportRPC(t *testing.T, reply string, status int) (*httptest.Server, *map[string]any) {
19+
t.Helper()
20+
var got map[string]any
21+
srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
22+
if r.URL.Path != "/operations/infix-system:support-collect" || r.Method != http.MethodPost {
23+
t.Errorf("unexpected request %s %s", r.Method, r.URL.Path)
24+
http.Error(w, "unexpected", http.StatusNotFound)
25+
return
26+
}
27+
if user, pass, ok := r.BasicAuth(); !ok || user != "admin" || pass != "secret" {
28+
t.Errorf("credentials not forwarded: %q %q %v", user, pass, ok)
29+
}
30+
body, _ := io.ReadAll(r.Body)
31+
got = nil
32+
if len(body) > 0 {
33+
if err := json.Unmarshal(body, &got); err != nil {
34+
t.Errorf("input is not JSON: %v", err)
35+
}
36+
}
37+
w.Header().Set("Content-Type", "application/yang-data+json")
38+
w.WriteHeader(status)
39+
io.WriteString(w, reply) //nolint:errcheck
40+
}))
41+
t.Cleanup(srv.Close)
42+
return srv, &got
43+
}
44+
45+
func supportRequest(srv *httptest.Server, form string) *httptest.ResponseRecorder {
46+
h := &SystemHandler{RC: restconf.NewClient(srv.URL, true)}
47+
req := httptest.NewRequest(http.MethodPost, "/maintenance/support-bundle", strings.NewReader(form))
48+
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
49+
req = req.WithContext(restconf.ContextWithCredentials(req.Context(),
50+
restconf.Credentials{Username: "admin", Password: "secret"}))
51+
rec := httptest.NewRecorder()
52+
h.SupportBundle(rec, req)
53+
return rec
54+
}
55+
56+
func TestSupportBundleDownloadsArchive(t *testing.T) {
57+
archive := []byte("\x1f\x8b\x08not really gzip")
58+
reply := `{"infix-system:output":{"size":` + strconv.Itoa(len(archive)) + `,"data":"` +
59+
base64.StdEncoding.EncodeToString(archive) + `"}}`
60+
srv, got := fakeSupportRPC(t, reply, http.StatusOK)
61+
62+
rec := supportRequest(srv, "")
63+
64+
if rec.Code != http.StatusOK {
65+
t.Fatalf("status %d: %s", rec.Code, rec.Body.String())
66+
}
67+
if *got != nil {
68+
t.Errorf("no password given, but input sent: %v", *got)
69+
}
70+
if rec.Body.String() != string(archive) {
71+
t.Errorf("body is not the decoded archive: %q", rec.Body.String())
72+
}
73+
if ct := rec.Header().Get("Content-Type"); ct != "application/gzip" {
74+
t.Errorf("Content-Type %q", ct)
75+
}
76+
cd := rec.Header().Get("Content-Disposition")
77+
if !strings.HasPrefix(cd, `attachment; filename="support-`) || !strings.HasSuffix(cd, `.tar.gz"`) {
78+
t.Errorf("Content-Disposition %q", cd)
79+
}
80+
}
81+
82+
func TestSupportBundlePasswordEncrypts(t *testing.T) {
83+
reply := `{"infix-system:output":{"size":3,"data":"` + base64.StdEncoding.EncodeToString([]byte("gpg")) + `"}}`
84+
srv, got := fakeSupportRPC(t, reply, http.StatusOK)
85+
86+
rec := supportRequest(srv, "password=hunter2")
87+
88+
if rec.Code != http.StatusOK {
89+
t.Fatalf("status %d: %s", rec.Code, rec.Body.String())
90+
}
91+
input, _ := (*got)["infix-system:input"].(map[string]any)
92+
if input["password"] != "hunter2" {
93+
t.Errorf("password not passed to the RPC: %v", *got)
94+
}
95+
if ct := rec.Header().Get("Content-Type"); ct != "application/pgp-encrypted" {
96+
t.Errorf("Content-Type %q", ct)
97+
}
98+
if !strings.HasSuffix(rec.Header().Get("Content-Disposition"), `.tar.gz.gpg"`) {
99+
t.Errorf("Content-Disposition %q", rec.Header().Get("Content-Disposition"))
100+
}
101+
}
102+
103+
func TestSupportBundleTooLarge(t *testing.T) {
104+
reply := `{"infix-system:output":{"size":20000000,"filename":"/var/lib/support/support-host-x.tar.gz"}}`
105+
srv, _ := fakeSupportRPC(t, reply, http.StatusOK)
106+
107+
rec := supportRequest(srv, "")
108+
109+
if rec.Code != http.StatusInternalServerError {
110+
t.Fatalf("status %d", rec.Code)
111+
}
112+
if !strings.Contains(rec.Body.String(), "19 MB") ||
113+
!strings.Contains(rec.Body.String(), "/var/lib/support/support-host-x.tar.gz") {
114+
t.Errorf("message does not say where the archive is: %q", rec.Body.String())
115+
}
116+
}
117+
118+
func TestSupportBundleReportsRPCError(t *testing.T) {
119+
reply := `{"ietf-restconf:errors":{"error":[{"error-type":"application","error-tag":"operation-failed",` +
120+
`"error-message":"gpg is not available on this device"}]}}`
121+
srv, _ := fakeSupportRPC(t, reply, http.StatusInternalServerError)
122+
123+
rec := supportRequest(srv, "password=x")
124+
125+
if rec.Code != http.StatusInternalServerError {
126+
t.Fatalf("status %d", rec.Code)
127+
}
128+
if !strings.Contains(rec.Body.String(), "gpg is not available on this device") {
129+
t.Errorf("server's reason not surfaced: %q", rec.Body.String())
130+
}
131+
}
132+
133+
func TestSupportBundleDeniedIsForbidden(t *testing.T) {
134+
reply := `{"ietf-restconf:errors":{"error":[{"error-type":"application","error-tag":"access-denied",` +
135+
`"error-message":"Access denied."}]}}`
136+
srv, _ := fakeSupportRPC(t, reply, http.StatusForbidden)
137+
138+
rec := supportRequest(srv, "")
139+
140+
if rec.Code != http.StatusForbidden {
141+
t.Fatalf("status %d", rec.Code)
142+
}
143+
if !strings.Contains(rec.Body.String(), "Access denied") {
144+
t.Errorf("reason not surfaced: %q", rec.Body.String())
145+
}
146+
}

‎src/webui/internal/handlers/system.go‎

Lines changed: 46 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"bytes"
77
"context"
88
"encoding/json"
9+
"errors"
910
"fmt"
1011
"html/template"
1112
"io"
@@ -208,52 +209,65 @@ func (h *SystemHandler) Backup(w http.ResponseWriter, r *http.Request) {
208209
}
209210
}
210211

211-
// SupportBundle runs the on-device `support collect` tool and streams the
212-
// resulting archive back as a download. The WebUI runs as root, so the
213-
// collection is complete (dmesg, ethtool, etc.). An optional password
214-
// encrypts the archive via the tool's GPG support and is fed on stdin so
215-
// it never lands in the process list.
212+
// SupportBundle collects a support archive with the support-collect RPC
213+
// and streams it back as a download. The RPC runs with the logged-in
214+
// user's credentials, so NACM decides who gets it. An optional password
215+
// has the archive GPG encrypted on the device.
216216
//
217-
// Collection emits nothing on stdout until it finishes (~50 s), then the
218-
// whole archive at once. We buffer it and only commit response headers
219-
// once the tool exits successfully, so a mid-collection failure becomes a
220-
// clean 500 rather than a truncated download. --work-dir /tmp keeps the
221-
// transient files in tmpfs; the tool cleans up after itself.
217+
// The RPC returns nothing until the collection is done, up to a minute,
218+
// so the response headers are committed only once the archive is in
219+
// hand and a failure becomes a clean error rather than a truncated
220+
// download.
222221
// POST /maintenance/support-bundle
223222
func (h *SystemHandler) SupportBundle(w http.ResponseWriter, r *http.Request) {
224-
// Collection blocks ~50 s with no output, but the server's 15 s
225-
// WriteTimeout would close the connection long before then (nginx
226-
// then logs a 502 "upstream prematurely closed connection"). Push
227-
// the write deadline out for this long-running download.
223+
// The server's 15 s WriteTimeout would close the connection long
224+
// before the RPC returns (nginx then logs a 502 "upstream prematurely
225+
// closed connection"). Push the write deadline out.
228226
if err := http.NewResponseController(w).SetWriteDeadline(time.Now().Add(4 * time.Minute)); err != nil {
229227
log.Printf("support bundle: extend write deadline: %v", err)
230228
}
231229

232230
password := r.FormValue("password")
233-
encrypt := password != ""
234-
235-
args := []string{"--work-dir", "/tmp", "collect"}
236231
ext, ctype := "tar.gz", "application/gzip"
237-
if encrypt {
238-
args = append(args, "-p")
232+
var input any
233+
if password != "" {
234+
input = map[string]any{"infix-system:input": map[string]string{"password": password}}
239235
ext, ctype = "tar.gz.gpg", "application/pgp-encrypted"
240236
}
241237

238+
var reply struct {
239+
Output struct {
240+
Size uint32 `json:"size"`
241+
Data []byte `json:"data"`
242+
Filename string `json:"filename"`
243+
} `json:"infix-system:output"`
244+
}
245+
242246
ctx, cancel := context.WithTimeout(r.Context(), 3*time.Minute)
243247
defer cancel()
244248

245-
cmd := exec.CommandContext(ctx, "/usr/sbin/support", args...)
246-
if encrypt {
247-
cmd.Stdin = strings.NewReader(password + "\n")
248-
}
249-
out, err := cmd.Output()
250-
if err != nil {
251-
stderr := ""
252-
if ee, ok := err.(*exec.ExitError); ok {
253-
stderr = strings.TrimSpace(string(ee.Stderr))
249+
if err := h.RC.CallRPC(ctx, "/operations/infix-system:support-collect", input, &reply); err != nil {
250+
log.Printf("support bundle: %v", err)
251+
msg, status := "Failed to collect support bundle", http.StatusInternalServerError
252+
var re *restconf.Error
253+
if errors.As(err, &re) {
254+
if re.Message != "" {
255+
msg += ": " + re.Message
256+
}
257+
if re.StatusCode == http.StatusForbidden {
258+
status = re.StatusCode
259+
}
254260
}
255-
log.Printf("support bundle: %v: %s", err, stderr)
256-
http.Error(w, "Failed to collect support bundle", http.StatusInternalServerError)
261+
http.Error(w, msg, status)
262+
return
263+
}
264+
265+
out := reply.Output
266+
if len(out.Data) == 0 {
267+
log.Printf("support bundle: %d bytes, too large to return inline, left in %s", out.Size, out.Filename)
268+
http.Error(w, fmt.Sprintf("Support bundle is %d MB, too large to download here. "+
269+
"It is on the device as %s, fetch it with scp.", out.Size>>20, out.Filename),
270+
http.StatusInternalServerError)
257271
return
258272
}
259273

@@ -265,8 +279,8 @@ func (h *SystemHandler) SupportBundle(w http.ResponseWriter, r *http.Request) {
265279

266280
w.Header().Set("Content-Type", ctype)
267281
w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", fname))
268-
w.Header().Set("Content-Length", fmt.Sprint(len(out)))
269-
w.Write(out) //nolint:errcheck
282+
w.Header().Set("Content-Length", fmt.Sprint(len(out.Data)))
283+
w.Write(out.Data) //nolint:errcheck
270284
}
271285

272286
// RestoreConfig accepts a multipart-uploaded JSON config file and applies it.

‎src/webui/internal/restconf/client.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,51 @@ func (c *Client) PostJSON(ctx context.Context, path string, body any) error {
156156
return c.writeJSON(ctx, http.MethodPost, path, body)
157157
}
158158

159+
// CallRPC invokes a RESTCONF operation and decodes its output into output,
160+
// nil for an operation without one. input, when not nil, is sent as the
161+
// operation's input container. Only the context's deadline bounds the
162+
// call, not the client's timeout for plain requests; support-collect runs
163+
// for up to a minute.
164+
func (c *Client) CallRPC(ctx context.Context, path string, input, output any) error {
165+
if err := checkPath(path); err != nil {
166+
return err
167+
}
168+
var body io.Reader
169+
if input != nil {
170+
var buf bytes.Buffer
171+
if err := json.NewEncoder(&buf).Encode(input); err != nil {
172+
return fmt.Errorf("encoding request body: %w", err)
173+
}
174+
body = &buf
175+
}
176+
177+
req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.baseURL+path, body)
178+
if err != nil {
179+
return err
180+
}
181+
// Required by rousette on every POST, body or not
182+
req.Header.Set("Content-Type", "application/yang-data+json")
183+
req.Header.Set("Accept", "application/yang-data+json")
184+
creds := CredentialsFromContext(ctx)
185+
req.SetBasicAuth(creds.Username, creds.Password)
186+
187+
hc := *c.httpClient
188+
hc.Timeout = 0
189+
resp, err := hc.Do(req)
190+
if err != nil {
191+
return fmt.Errorf("restconf request failed: %w", err)
192+
}
193+
defer resp.Body.Close()
194+
195+
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNoContent {
196+
return parseError(resp)
197+
}
198+
if output == nil || resp.StatusCode == http.StatusNoContent {
199+
return nil
200+
}
201+
return json.NewDecoder(resp.Body).Decode(output)
202+
}
203+
159204
// Put replaces a RESTCONF config resource with the given value.
160205
func (c *Client) Put(ctx context.Context, path string, body any) error {
161206
return c.writeJSON(ctx, http.MethodPut, path, body)

‎src/webui/internal/restconf/errors.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func parseError(resp *http.Response) error {
101101

102102
var parts []string
103103
for _, e := range errs {
104-
msg := e.Message
104+
msg := unwrap(e.Message)
105105
if msg == "" {
106106
msg = e.Tag
107107
}
@@ -115,3 +115,23 @@ func parseError(resp *http.Response) error {
115115
re.Message = strings.Join(parts, "; ")
116116
return re
117117
}
118+
119+
// unwrap strips rousette's framing of a sysrepo error, which quotes the
120+
// same message twice:
121+
//
122+
// Internal server error due to sysrepo exception: Couldn't send RPC:
123+
// SR_ERR_OPERATION_FAILED <msg> (SR_ERR_OPERATION_FAILED) NETCONF:
124+
// application: operation-failed: <msg>
125+
//
126+
// Only the message after the NETCONF type and tag is of any use to a user.
127+
func unwrap(msg string) string {
128+
i := strings.LastIndex(msg, "NETCONF: ")
129+
if i < 0 {
130+
return msg
131+
}
132+
parts := strings.SplitN(msg[i+len("NETCONF: "):], ": ", 3)
133+
if len(parts) < 3 {
134+
return msg
135+
}
136+
return parts[2]
137+
}

0 commit comments

Comments
 (0)