Merge pull request #48 from Icinga/fix-syslog-identifier-for-journald

logging: Sanitize Journald Field Key Names

Author: julianbrost

logging/journald_core.go

@@ -5,7 +5,6 @@ import (
 	"github.com/pkg/errors"
 	"github.com/ssgreg/journald"
 	"go.uber.org/zap/zapcore"
-	"strings"
 )
 
 // priorities maps zapcore.Level to journal.Priority.
@@ -25,15 +24,13 @@ func NewJournaldCore(identifier string, enab zapcore.LevelEnabler) zapcore.Core
 	return &journaldCore{
 		LevelEnabler: enab,
 		identifier:   identifier,
-		identifierU:  strings.ToUpper(identifier),
 	}
 }
 
 type journaldCore struct {
 	zapcore.LevelEnabler
-	context     []zapcore.Field
-	identifier  string
-	identifierU string
+	context    []zapcore.Field
+	identifier string
 }
 
 func (c *journaldCore) Check(ent zapcore.Entry, ce *zapcore.CheckedEntry) *zapcore.CheckedEntry {
@@ -62,6 +59,7 @@ func (c *journaldCore) Write(ent zapcore.Entry, fields []zapcore.Field) error {
 	}
 
 	enc := zapcore.NewMapObjectEncoder()
+	// Ensure that all field keys are valid journald field keys. If in doubt, use encodeJournaldFieldKey.
 	c.addFields(enc, fields)
 	c.addFields(enc, c.context)
 	enc.Fields["SYSLOG_IDENTIFIER"] = c.identifier
@@ -74,11 +72,55 @@ func (c *journaldCore) Write(ent zapcore.Entry, fields []zapcore.Field) error {
 	return journald.Send(message, pri, enc.Fields)
 }
 
+// addFields adds all given fields to enc with an altered key, prefixed with the journaldCore.identifier and sanitized
+// via encodeJournaldFieldKey.
 func (c *journaldCore) addFields(enc zapcore.ObjectEncoder, fields []zapcore.Field) {
 	for _, field := range fields {
-		field.Key = c.identifierU +
-			"_" +
-			strcase.ScreamingSnake(field.Key)
+		field.Key = encodeJournaldFieldKey(c.identifier + "_" + field.Key)
 		field.AddTo(enc)
 	}
 }
+
+// encodeJournaldFieldKey alters a string to be used as a journald field key.
+//
+// When journald receives a field with an invalid key, it silently discards this field. This makes syntactically correct
+// keys a necessity. Unfortunately, there was no specific documentation about the field key syntax available. This
+// function follows the logic enforced in systemd's journal_field_valid function[0].
+//
+// This boils down to:
+// - Key length MUST be within (0, 64] characters.
+// - Key MUST start with [A-Z].
+// - Key characters MUST be [A-Z0-9_].
+//
+//	[0]: https://github.com/systemd/systemd/blob/11d5e2b5fbf9f6bfa5763fd45b56829ad4f0777f/src/libsystemd/sd-journal/journal-file.c#L1703
+func encodeJournaldFieldKey(key string) string {
+	if len(key) == 0 {
+		// While this is definitely an error, panicking would be too destructive and silently dropping fields is against
+		// the very idea of ensuring key conformity.
+		return "EMPTY_KEY"
+	}
+
+	isAsciiUpper := func(r rune) bool { return 'A' <= r && r <= 'Z' }
+	isAsciiDigit := func(r rune) bool { return '0' <= r && r <= '9' }
+
+	keyParts := []rune(strcase.ScreamingSnake(key))
+	for i, r := range keyParts {
+		if isAsciiUpper(r) || isAsciiDigit(r) || r == '_' {
+			continue
+		}
+		keyParts[i] = '_'
+	}
+	key = string(keyParts)
+
+	if !isAsciiUpper(rune(key[0])) {
+		// Escape invalid leading characters with a generic "ESC_" prefix. This was seen as a safer choice instead of
+		// iterating over the key and removing parts.
+		key = "ESC_" + key
+	}
+
+	if len(key) > 64 {
+		key = key[:64]
+	}
+
+	return key
+}

logging/journald_core_test.go

@@ -0,0 +1,41 @@
+package logging
+
+import (
+	"github.com/stretchr/testify/require"
+	"regexp"
+	"testing"
+)
+
+func Test_journaldFieldEncode(t *testing.T) {
+	tests := []struct {
+		name   string
+		input  string
+		output string
+	}{
+		{"empty", "", "EMPTY_KEY"},
+		{"lowercase", "foo", "FOO"},
+		{"uppercase", "FOO", "FOO"},
+		{"dash", "foo-bar", "FOO_BAR"},
+		{"non ascii", "snow_☃", "SNOW__"},
+		{"lowercase non ascii alpha", "föö", "F__"},
+		{"uppercase non ascii alpha", "FÖÖ", "F__"},
+		{"leading number", "23", "ESC_23"},
+		{"leading underscore", "_foo", "ESC__FOO"},
+		{"leading invalid", " foo", "ESC__FOO"},
+		{"max length", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1234", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1234"},
+		{"too long", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA12345", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1234"},
+		{"too long leading number", "1234567890123456789012345678901234567890123456789012345678901234", "ESC_123456789012345678901234567890123456789012345678901234567890"},
+		{"concrete example", "icinga-notifications" + "_" + "error", "ICINGA_NOTIFICATIONS_ERROR"},
+		{"example syslog_identifier", "SYSLOG_IDENTIFIER", "SYSLOG_IDENTIFIER"},
+	}
+
+	check := regexp.MustCompile(`^[A-Z][A-Z0-9_]{0,63}$`)
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			out := encodeJournaldFieldKey(test.input)
+			require.Equal(t, test.output, out)
+			require.True(t, check.MatchString(out), "check regular expression")
+		})
+	}
+}