metropolis/node: sanitize served logs to remove invalid UTF-8 characters
Ideally we would instead either use bytes instead of a string, or never
allow submitting log lines which are invalid UTF-8. And some day we
might still prohibit that - but for now let's add this failsafe anyway.
Fixes #277.
Change-Id: I3d057f4bd1de0ce86e9724e2234b2a40efea8507
Reviewed-on: https://review.monogon.dev/c/monogon/+/2406
Tested-by: Jenkins CI
Reviewed-by: Tim Windelschmidt <tim@monogon.tech>
diff --git a/metropolis/node/core/mgmt/svc_logs.go b/metropolis/node/core/mgmt/svc_logs.go
index baf9d9b..9d27d63 100644
--- a/metropolis/node/core/mgmt/svc_logs.go
+++ b/metropolis/node/core/mgmt/svc_logs.go
@@ -1,6 +1,8 @@
package mgmt
import (
+ "strings"
+
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
@@ -19,6 +21,42 @@
LogTree *logtree.LogTree
}
+// sanitizedEntries returns a deep copy of the given log entries, but replaces
+// all invalid UTF-8 characters with "<INVALID>".
+func sanitizedEntries(entries []*cpb.LogEntry) []*cpb.LogEntry {
+ res := make([]*cpb.LogEntry, len(entries))
+ for i, entry := range entries {
+ res[i] = &cpb.LogEntry{
+ Dn: entry.Dn,
+ Kind: nil,
+ }
+ switch k := entry.Kind.(type) {
+ case *cpb.LogEntry_Leveled_:
+ leveled := &cpb.LogEntry_Leveled_{
+ Leveled: &cpb.LogEntry_Leveled{
+ Lines: make([]string, len(k.Leveled.Lines)),
+ Timestamp: k.Leveled.Timestamp,
+ Severity: k.Leveled.Severity,
+ Location: k.Leveled.Location,
+ },
+ }
+ for j, line := range k.Leveled.Lines {
+ leveled.Leveled.Lines[j] = strings.ToValidUTF8(line, "<INVALID>")
+ }
+ res[i].Kind = leveled
+
+ case *cpb.LogEntry_Raw_:
+ res[i].Kind = &cpb.LogEntry_Raw_{
+ Raw: &cpb.LogEntry_Raw{
+ Data: strings.ToValidUTF8(k.Raw.Data, "<INVALID>"),
+ OriginalLength: k.Raw.OriginalLength,
+ },
+ }
+ }
+ }
+ return res
+}
+
func (s *LogService) Logs(req *api.GetLogsRequest, srv api.NodeManagement_LogsServer) error {
if len(req.Filters) > logFilterMax {
return status.Errorf(codes.InvalidArgument, "requested %d filters, maximum permitted is %d", len(req.Filters), logFilterMax)
@@ -113,7 +151,7 @@
if len(chunk) >= maxChunkSize {
err := srv.Send(&api.GetLogsResponse{
- BacklogEntries: chunk,
+ BacklogEntries: sanitizedEntries(chunk),
})
if err != nil {
return err
@@ -125,7 +163,7 @@
// Send last chunk of backlog, if present..
if len(chunk) > 0 {
err := srv.Send(&api.GetLogsResponse{
- BacklogEntries: chunk,
+ BacklogEntries: sanitizedEntries(chunk),
})
if err != nil {
return err
diff --git a/metropolis/node/core/mgmt/svc_logs_test.go b/metropolis/node/core/mgmt/svc_logs_test.go
index d5825ad..9517600 100644
--- a/metropolis/node/core/mgmt/svc_logs_test.go
+++ b/metropolis/node/core/mgmt/svc_logs_test.go
@@ -123,6 +123,7 @@
s.LogTree.MustLeveledFor("main.roleserver.controlplane").Infof("Starting control plane...")
s.LogTree.MustLeveledFor("main.roleserver.kubernetes").Infof("Kubernetes version: 1.21.37")
s.LogTree.MustLeveledFor("main.roleserver.controlplane").Infof("Starting etcd...")
+ s.LogTree.MustLeveledFor("main.weirdo").Infof("Here comes some invalid utf-8: a\xc5z")
mkReq := func(dn string, backlog int64) *api.GetLogsRequest {
var backlogMode api.GetLogsRequest_BacklogMode
@@ -175,7 +176,6 @@
req: mkReq("main.roleserver.kubernetes", 0),
want: nil,
},
-
{
// Test recursion with backlog.
req: mkRecursive(mkReq("main.roleserver", 2)),
@@ -184,6 +184,13 @@
mkLeveledEntry("main.roleserver.controlplane", "i", "Starting etcd..."),
},
},
+ {
+ // Test invalid utf-8 in log data
+ req: mkReq("main.weirdo", 1),
+ want: []*cpb.LogEntry{
+ mkLeveledEntry("main.weirdo", "i", "Here comes some invalid utf-8: a<INVALID>z"),
+ },
+ },
} {
srv, err := mgmt.Logs(ctx, te.req)
if err != nil {