Skip to content

Commit 6d2d554

Browse files
danischmdanischm
committed
Add full request and response content logging
1 parent 27bcb04 commit 6d2d554

File tree

3 files changed

+317
-0
lines changed

3 files changed

+317
-0
lines changed

‎client.go‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"strings"
1414
"sync"
1515
"time"
16+
"unicode/utf8"
1617

1718
"github.com/netascode/xmldot"
1819
"github.com/scrapli/scrapligo/driver/netconf"
@@ -62,6 +63,27 @@ var defaultRedactionPatterns = []*regexp.Regexp{
6263
regexp.MustCompile(`<key>.*?</key>`),
6364
regexp.MustCompile(`<community>.*?</community>`),
6465

66+
// CDATA section handling (must come before namespace-aware to avoid conflicts)
67+
// Matches: <password><![CDATA[value]]></password>
68+
regexp.MustCompile(`<password><!\[CDATA\[.*?\]\]></password>`),
69+
regexp.MustCompile(`<secret><!\[CDATA\[.*?\]\]></secret>`),
70+
regexp.MustCompile(`<key><!\[CDATA\[.*?\]\]></key>`),
71+
regexp.MustCompile(`<community><!\[CDATA\[.*?\]\]></community>`),
72+
73+
// Namespace-aware element content
74+
// Matches: <prefix:password>value</prefix:password>
75+
// Note: Go regexp doesn't support backreferences, so we match any namespace in closing tag
76+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:password[^>]*>.*?</[a-zA-Z0-9_-]+:password>`),
77+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:secret[^>]*>.*?</[a-zA-Z0-9_-]+:secret>`),
78+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:key[^>]*>.*?</[a-zA-Z0-9_-]+:key>`),
79+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:community[^>]*>.*?</[a-zA-Z0-9_-]+:community>`),
80+
81+
// Namespaced CDATA sections
82+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:password[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:password>`),
83+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:secret[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:secret>`),
84+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:key[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:key>`),
85+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:community[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:community>`),
86+
6587
// Attribute values (double quotes)
6688
regexp.MustCompile(`password="[^"]*"`),
6789
regexp.MustCompile(`secret="[^"]*"`),
@@ -790,6 +812,24 @@ func (c *Client) redactSensitiveData(xml string) string {
790812
"<key>[REDACTED]</key>",
791813
"<community>[REDACTED]</community>",
792814

815+
// CDATA sections (must match pattern order)
816+
"<password><![CDATA[[REDACTED]]]></password>",
817+
"<secret><![CDATA[[REDACTED]]]></secret>",
818+
"<key><![CDATA[[REDACTED]]]></key>",
819+
"<community><![CDATA[[REDACTED]]]></community>",
820+
821+
// Namespace-aware elements (generic replacement works for any namespace)
822+
"<ns:password>[REDACTED]</ns:password>",
823+
"<ns:secret>[REDACTED]</ns:secret>",
824+
"<ns:key>[REDACTED]</ns:key>",
825+
"<ns:community>[REDACTED]</ns:community>",
826+
827+
// Namespaced CDATA sections
828+
"<ns:password><![CDATA[[REDACTED]]]></ns:password>",
829+
"<ns:secret><![CDATA[[REDACTED]]]></ns:secret>",
830+
"<ns:key><![CDATA[[REDACTED]]]></ns:key>",
831+
"<ns:community><![CDATA[[REDACTED]]]></ns:community>",
832+
793833
// Attributes (double quotes)
794834
`password="[REDACTED]"`,
795835
`secret="[REDACTED]"`,
@@ -1334,6 +1374,57 @@ func (c *Client) executeRPC(ctx context.Context, req *Req) (Res, error) {
13341374
return Res{}, fmt.Errorf("operation %s: received nil response from driver", req.Operation)
13351375
}
13361376

1377+
// Log request XML content (Debug level only)
1378+
// Pre-check size and level to avoid expensive processing when not needed
1379+
if len(scrapligoRes.Input) > 0 {
1380+
// Pre-check size limit before string conversion (avoid allocation)
1381+
if len(scrapligoRes.Input) <= MaxXMLSizeForLogging {
1382+
// Validate UTF-8 encoding
1383+
if !utf8.Valid(scrapligoRes.Input) {
1384+
c.logger.Warn(ctx, "Invalid UTF-8 in NETCONF request XML",
1385+
"operation", req.Operation,
1386+
"size", len(scrapligoRes.Input))
1387+
} else {
1388+
requestXML := c.prepareXMLForLogging(string(scrapligoRes.Input))
1389+
c.logger.Debug(ctx, "NETCONF RPC request XML",
1390+
"operation", req.Operation,
1391+
"xml", requestXML)
1392+
}
1393+
} else {
1394+
// Log truncation message only (cheap operation)
1395+
c.logger.Debug(ctx, "NETCONF RPC request XML (truncated)",
1396+
"operation", req.Operation,
1397+
"size", len(scrapligoRes.Input),
1398+
"limit", MaxXMLSizeForLogging,
1399+
"xml", "[XML TOO LARGE FOR LOGGING]")
1400+
}
1401+
}
1402+
1403+
// Log response XML content (Debug level only)
1404+
if scrapligoRes.Result != "" {
1405+
// Pre-check size limit before processing
1406+
if len(scrapligoRes.Result) <= MaxXMLSizeForLogging {
1407+
// Validate UTF-8 encoding
1408+
if !utf8.ValidString(scrapligoRes.Result) {
1409+
c.logger.Warn(ctx, "Invalid UTF-8 in NETCONF response XML",
1410+
"operation", req.Operation,
1411+
"size", len(scrapligoRes.Result))
1412+
} else {
1413+
responseXML := c.prepareXMLForLogging(scrapligoRes.Result)
1414+
c.logger.Debug(ctx, "NETCONF RPC response XML",
1415+
"operation", req.Operation,
1416+
"xml", responseXML)
1417+
}
1418+
} else {
1419+
// Log truncation message only (cheap operation)
1420+
c.logger.Debug(ctx, "NETCONF RPC response XML (truncated)",
1421+
"operation", req.Operation,
1422+
"size", len(scrapligoRes.Result),
1423+
"limit", MaxXMLSizeForLogging,
1424+
"xml", "[XML TOO LARGE FOR LOGGING]")
1425+
}
1426+
}
1427+
13371428
// Parse response XML
13381429
return c.parseResponse(scrapligoRes)
13391430
}

‎client_test.go‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,21 @@ func TestClient_redactSensitiveData_Attributes(t *testing.T) {
836836
regexp.MustCompile(`<secret>.*?</secret>`),
837837
regexp.MustCompile(`<key>.*?</key>`),
838838
regexp.MustCompile(`<community>.*?</community>`),
839+
// CDATA sections
840+
regexp.MustCompile(`<password><!\[CDATA\[.*?\]\]></password>`),
841+
regexp.MustCompile(`<secret><!\[CDATA\[.*?\]\]></secret>`),
842+
regexp.MustCompile(`<key><!\[CDATA\[.*?\]\]></key>`),
843+
regexp.MustCompile(`<community><!\[CDATA\[.*?\]\]></community>`),
844+
// Namespace-aware elements
845+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:password[^>]*>.*?</[a-zA-Z0-9_-]+:password>`),
846+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:secret[^>]*>.*?</[a-zA-Z0-9_-]+:secret>`),
847+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:key[^>]*>.*?</[a-zA-Z0-9_-]+:key>`),
848+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:community[^>]*>.*?</[a-zA-Z0-9_-]+:community>`),
849+
// Namespaced CDATA sections
850+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:password[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:password>`),
851+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:secret[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:secret>`),
852+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:key[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:key>`),
853+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:community[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:community>`),
839854
// Attributes (double quotes)
840855
regexp.MustCompile(`password="[^"]*"`),
841856
regexp.MustCompile(`secret="[^"]*"`),
@@ -1026,6 +1041,21 @@ func TestClient_redactSensitiveData_XPathFilters(t *testing.T) {
10261041
regexp.MustCompile(`<secret>.*?</secret>`),
10271042
regexp.MustCompile(`<key>.*?</key>`),
10281043
regexp.MustCompile(`<community>.*?</community>`),
1044+
// CDATA sections
1045+
regexp.MustCompile(`<password><!\[CDATA\[.*?\]\]></password>`),
1046+
regexp.MustCompile(`<secret><!\[CDATA\[.*?\]\]></secret>`),
1047+
regexp.MustCompile(`<key><!\[CDATA\[.*?\]\]></key>`),
1048+
regexp.MustCompile(`<community><!\[CDATA\[.*?\]\]></community>`),
1049+
// Namespace-aware elements
1050+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:password[^>]*>.*?</[a-zA-Z0-9_-]+:password>`),
1051+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:secret[^>]*>.*?</[a-zA-Z0-9_-]+:secret>`),
1052+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:key[^>]*>.*?</[a-zA-Z0-9_-]+:key>`),
1053+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:community[^>]*>.*?</[a-zA-Z0-9_-]+:community>`),
1054+
// Namespaced CDATA sections
1055+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:password[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:password>`),
1056+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:secret[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:secret>`),
1057+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:key[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:key>`),
1058+
regexp.MustCompile(`<[a-zA-Z0-9_-]+:community[^>]*><!\[CDATA\[.*?\]\]></[a-zA-Z0-9_-]+:community>`),
10291059
// Attribute values (double quotes)
10301060
regexp.MustCompile(`password="[^"]*"`),
10311061
regexp.MustCompile(`secret="[^"]*"`),
@@ -1099,3 +1129,144 @@ func TestClient_redactSensitiveData_XPathFilters(t *testing.T) {
10991129
})
11001130
}
11011131
}
1132+
1133+
// TestClient_redactSensitiveData_NamespaceAndCDATA tests namespace-aware and CDATA redaction patterns
1134+
// This test validates the security enhancement that added 12 new patterns for comprehensive coverage
1135+
func TestClient_redactSensitiveData_NamespaceAndCDATA(t *testing.T) {
1136+
client := &Client{
1137+
redactionPatterns: defaultRedactionPatterns,
1138+
}
1139+
1140+
tests := []struct {
1141+
name string
1142+
input string
1143+
shouldContain string // What MUST be in output
1144+
mustNotContain []string // What MUST NOT be in output (sensitive data)
1145+
desc string
1146+
}{
1147+
{
1148+
name: "Namespace-aware password element",
1149+
input: `<config><auth:password>secret123</auth:password><hostname>router1</hostname></config>`,
1150+
shouldContain: "[REDACTED]",
1151+
mustNotContain: []string{"secret123"},
1152+
desc: "Should redact <auth:password> with namespace prefix",
1153+
},
1154+
{
1155+
name: "Namespace-aware secret element",
1156+
input: `<data><cisco:secret>my_secret_value</cisco:secret></data>`,
1157+
shouldContain: "[REDACTED]",
1158+
mustNotContain: []string{"my_secret_value"},
1159+
desc: "Should redact <cisco:secret> with namespace prefix",
1160+
},
1161+
{
1162+
name: "Namespace-aware key element",
1163+
input: `<config><vpn:key>encryption_key_123</vpn:key></config>`,
1164+
shouldContain: "[REDACTED]",
1165+
mustNotContain: []string{"encryption_key_123"},
1166+
desc: "Should redact <vpn:key> with namespace prefix",
1167+
},
1168+
{
1169+
name: "Namespace-aware community element",
1170+
input: `<snmp><mgmt:community>public_string</mgmt:community></snmp>`,
1171+
shouldContain: "[REDACTED]",
1172+
mustNotContain: []string{"public_string"},
1173+
desc: "Should redact <mgmt:community> with namespace prefix",
1174+
},
1175+
{
1176+
name: "CDATA password element",
1177+
input: `<config><password><![CDATA[p@ssw0rd!]]></password></config>`,
1178+
shouldContain: "[REDACTED]",
1179+
mustNotContain: []string{"p@ssw0rd!"},
1180+
desc: "Should redact CDATA password content",
1181+
},
1182+
{
1183+
name: "CDATA secret element",
1184+
input: `<auth><secret><![CDATA[secret_token_xyz]]></secret></auth>`,
1185+
shouldContain: "[REDACTED]",
1186+
mustNotContain: []string{"secret_token_xyz"},
1187+
desc: "Should redact CDATA secret content",
1188+
},
1189+
{
1190+
name: "CDATA key element",
1191+
input: `<crypto><key><![CDATA[api_key_12345]]></key></crypto>`,
1192+
shouldContain: "[REDACTED]",
1193+
mustNotContain: []string{"api_key_12345"},
1194+
desc: "Should redact CDATA key content",
1195+
},
1196+
{
1197+
name: "CDATA community element",
1198+
input: `<snmp><community><![CDATA[community_ro]]></community></snmp>`,
1199+
shouldContain: "[REDACTED]",
1200+
mustNotContain: []string{"community_ro"},
1201+
desc: "Should redact CDATA community content",
1202+
},
1203+
{
1204+
name: "Namespace CDATA password",
1205+
input: `<config><ns:password><![CDATA[ns_password_value]]></ns:password></config>`,
1206+
shouldContain: "[REDACTED]",
1207+
mustNotContain: []string{"ns_password_value"},
1208+
desc: "Should redact namespace-aware CDATA password",
1209+
},
1210+
{
1211+
name: "Namespace CDATA secret",
1212+
input: `<data><auth:secret><![CDATA[secret_in_ns]]></auth:secret></data>`,
1213+
shouldContain: "[REDACTED]",
1214+
mustNotContain: []string{"secret_in_ns"},
1215+
desc: "Should redact namespace-aware CDATA secret",
1216+
},
1217+
{
1218+
name: "Namespace CDATA key",
1219+
input: `<vpn><config:key><![CDATA[key_in_ns]]></config:key></vpn>`,
1220+
shouldContain: "[REDACTED]",
1221+
mustNotContain: []string{"key_in_ns"},
1222+
desc: "Should redact namespace-aware CDATA key",
1223+
},
1224+
{
1225+
name: "Namespace CDATA community",
1226+
input: `<snmp><cisco:community><![CDATA[community_in_ns]]></cisco:community></snmp>`,
1227+
shouldContain: "[REDACTED]",
1228+
mustNotContain: []string{"community_in_ns"},
1229+
desc: "Should redact namespace-aware CDATA community",
1230+
},
1231+
{
1232+
name: "Mixed redaction types",
1233+
input: `<config><password>plain_pass</password><auth:password>ns_pass</auth:password><secret><![CDATA[cdata_secret]]></secret><vpn:key><![CDATA[ns_cdata_key]]></vpn:key></config>`,
1234+
shouldContain: "[REDACTED]",
1235+
mustNotContain: []string{"plain_pass", "ns_pass", "cdata_secret", "ns_cdata_key"},
1236+
desc: "Should handle multiple redaction types simultaneously",
1237+
},
1238+
{
1239+
name: "Preserve non-sensitive content",
1240+
input: `<config><hostname>router1</hostname><auth:password>secret</auth:password><interface>GigE0/0</interface></config>`,
1241+
shouldContain: "router1",
1242+
mustNotContain: []string{"secret"},
1243+
desc: "Should preserve non-sensitive data like hostnames",
1244+
},
1245+
}
1246+
1247+
for _, tt := range tests {
1248+
t.Run(tt.name, func(t *testing.T) {
1249+
result := client.redactSensitiveData(tt.input)
1250+
1251+
// Verify sensitive data is NOT in output
1252+
for _, sensitive := range tt.mustNotContain {
1253+
if strings.Contains(result, sensitive) {
1254+
t.Errorf("%s FAILED: Sensitive data '%s' was not redacted\n"+
1255+
"Input: %s\n"+
1256+
"Output: %s\n"+
1257+
"Reason: %s",
1258+
tt.name, sensitive, tt.input, result, tt.desc)
1259+
}
1260+
}
1261+
1262+
// Verify [REDACTED] IS in output
1263+
if !strings.Contains(result, tt.shouldContain) {
1264+
t.Errorf("%s FAILED: Expected '%s' in output\n"+
1265+
"Input: %s\n"+
1266+
"Output: %s\n"+
1267+
"Reason: %s",
1268+
tt.name, tt.shouldContain, tt.input, result, tt.desc)
1269+
}
1270+
})
1271+
}
1272+
}

‎docs/logging.md‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,61 @@ client, _ := netconf.NewClient("192.168.1.1",
599599
[ERROR] NETCONF RPC error index=0 errorType=application errorTag=invalid-value errorMessage=Invalid configuration data
600600
```
601601

602+
### XML Content Logging (Debug Level)
603+
604+
When log level is set to Debug, go-netconf logs the full XML content of RPC requests and responses:
605+
606+
```
607+
[DEBUG] NETCONF RPC request XML operation=get-config xml=<rpc message-id="101">...</rpc>
608+
[DEBUG] NETCONF RPC response XML operation=get-config xml=<rpc-reply message-id="101">...</rpc-reply>
609+
```
610+
611+
**Security Features:**
612+
- Automatically redacts sensitive data (passwords, secrets, keys, community strings)
613+
- Enforces 1MB size limit to prevent logging attacks
614+
- Pretty-prints XML when `WithPrettyPrintLogs(true)` is enabled
615+
616+
**Performance Notes:**
617+
- Only active when log level is Debug or lower
618+
- Zero overhead when using Info/Warn/Error log levels
619+
- Safe for production debug sessions - sensitive data is automatically redacted
620+
621+
**Example:**
622+
```go
623+
// Enable XML content logging for debugging
624+
logger := netconf.NewDefaultLogger(netconf.LogLevelDebug)
625+
client, _ := netconf.NewClient("192.168.1.1",
626+
netconf.WithLogger(logger),
627+
netconf.WithPrettyPrintLogs(true), // Pretty-print XML content
628+
)
629+
630+
// Perform operation - full XML will be logged
631+
res, err := client.GetConfig("running")
632+
```
633+
634+
#### Security Considerations
635+
636+
When Debug logging is enabled, go-netconf logs full XML request/response data. While sensitive fields are automatically redacted, you should still treat debug logs as sensitive:
637+
638+
**Redacted Fields:**
639+
- `<password>`, `<secret>`, `<key>`, `<community>` (element content)
640+
- `password=""`, `secret=""`, `key=""`, `community=""` (attributes)
641+
- XPath predicates with sensitive fields
642+
643+
**Not Redacted:**
644+
- Network topology (IP addresses, interface names)
645+
- Configuration structure (ACL names, VRF names)
646+
- Software versions
647+
- Custom vendor-specific sensitive fields (add custom patterns)
648+
649+
**Best Practices:**
650+
1. **Use Debug logging only in trusted environments** (development, troubleshooting)
651+
2. **Disable Debug in production** (use Info/Warn/Error levels)
652+
3. **Restrict log file permissions** (`chmod 0600` or `0640`)
653+
4. **Implement log rotation** (prevent disk exhaustion)
654+
5. **Encrypt logs at rest** (use encrypted file systems or log aggregation)
655+
6. **Define retention policies** (delete logs after 30-90 days)
656+
602657
## Best Practices
603658

604659
### 1. Use Appropriate Log Levels

0 commit comments

Comments
 (0)