Skip to content

Commit

Permalink
Refactor security logging
Browse files Browse the repository at this point in the history
Customize security logger to avoid logging secret values.
* Add a new safe string printers.
* Only log the safe strings.
* Add some new type string generators.
* Bump codeql[0].

[0]: https://github.blog/changelog/2022-04-27-code-scanning-deprecation-of-codeql-action-v1/

Signed-off-by: SuperQ <superq@gmail.com>
  • Loading branch information
SuperQ committed Dec 15, 2022
1 parent 9a70609 commit 2f729f9
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 6 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ jobs:

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@v1
uses: github/codeql-action/init@v2
# Override language selection by uncommenting this and choosing your languages
# with:
# languages: go, javascript, csharp, python, cpp, java

# Autobuild attempts to build any compiled languages (C/C++, C#, or Java).
# If this step fails, then you should remove it and run the build manually (see below)
- name: Autobuild
uses: github/codeql-action/autobuild@v1
uses: github/codeql-action/autobuild@v2

# ℹ️ Command-line programs to run using the OS shell.
# 📚 https://git.io/JvXDl
Expand All @@ -51,4 +51,4 @@ jobs:
# make release

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v1
uses: github/codeql-action/analyze@v2
26 changes: 24 additions & 2 deletions marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ const (
Report PDUType = 0xa8 // v3
)

//go:generate stringer -type=PDUType

// SNMPv3: User-based Security Model Report PDUs and
// error types as per https://tools.ietf.org/html/rfc3414
const (
Expand Down Expand Up @@ -154,6 +156,26 @@ func NewLogger(logger LoggerInterface) Logger {
}
}

func (packet *SnmpPacket) SafeString() string {
return fmt.Sprintf("Version:%s, MsgFlags:%s, SecurityModel:%s, ContextEngineID:%s, ContextName:%s, Community:%s, PDUType:%s, MsgID:%d, RequestID:%d, MsgMaxSize:%d, Error:%s, ErrorIndex:%d, NonRepeaters:%d, MaxRepetitions:%d, Variables:%v",
packet.Version,
packet.MsgFlags,
packet.SecurityModel,
packet.ContextEngineID,
packet.ContextName,
packet.Community,
packet.PDUType,
packet.MsgID,
packet.RequestID,
packet.MsgMaxSize,
packet.Error,
packet.ErrorIndex,
packet.NonRepeaters,
packet.MaxRepetitions,
packet.Variables,
)
}

// GoSNMP
// send/receive one snmp request
func (x *GoSNMP) sendOneRequest(packetOut *SnmpPacket,
Expand Down Expand Up @@ -238,7 +260,7 @@ func (x *GoSNMP) sendOneRequest(packetOut *SnmpPacket,
if x.PreSend != nil {
x.PreSend(x)
}
x.Logger.Printf("SENDING PACKET: %#+v", *packetOut)
x.Logger.Printf("SENDING PACKET: %s", packetOut.SafeString())
// If using UDP and unconnected socket, send packet directly to stored address.
if uconn, ok := x.Conn.(net.PacketConn); ok && x.uaddr != nil {
_, err = uconn.WriteTo(outBuf, x.uaddr)
Expand Down Expand Up @@ -437,7 +459,7 @@ func (x *GoSNMP) send(packetOut *SnmpPacket, wait bool) (result *SnmpPacket, err
}

if result.Version == Version3 {
x.Logger.Printf("SEND STORE SECURITY PARAMS from result: %+v", result)
x.Logger.Printf("SEND STORE SECURITY PARAMS from result: %s", result.SecurityParameters.String())
err = x.storeSecurityParameters(result)

if result.PDUType == Report && len(result.Variables) == 1 {
Expand Down
42 changes: 42 additions & 0 deletions pdutype_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 37 additions & 0 deletions snmpv3msgflags_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 24 additions & 0 deletions snmpv3securitymodel_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const (
Reportable SnmpV3MsgFlags = 0x4 // Report PDU must be sent.
)

//go:generate stringer -type=SnmpV3MsgFlags

// SnmpV3SecurityModel describes the security model used by a SnmpV3 connection
type SnmpV3SecurityModel uint8

Expand All @@ -35,11 +37,14 @@ const (
UserSecurityModel SnmpV3SecurityModel = 3
)

//go:generate stringer -type=SnmpV3SecurityModel

// SnmpV3SecurityParameters is a generic interface type to contain various implementations of SnmpV3SecurityParameters
type SnmpV3SecurityParameters interface {
Log()
Copy() SnmpV3SecurityParameters
Description() string
String() string
validate(flags SnmpV3MsgFlags) error
init(log Logger) error
initPacket(packet *SnmpPacket) error
Expand Down
18 changes: 17 additions & 1 deletion v3_usm.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,27 @@ func (sp *UsmSecurityParameters) Description() string {
return sb.String()
}

// String returns a logging safe (no secrets) string of the UsmSecurityParameters
func (sp *UsmSecurityParameters) String() string {
sp.mu.Lock()
defer sp.mu.Unlock()
return fmt.Sprintf("AuthoritativeEngineID:%s, AuthoritativeEngineBoots:%d, AuthoritativeEngineTimes:%d, UserName:%s, AuthenticationParameters:%s, PrivacyParameters:%v, AuthenticationProtocol:%s, PrivacyProtocol:%s",
sp.AuthoritativeEngineID,
sp.AuthoritativeEngineBoots,
sp.AuthoritativeEngineTime,
sp.UserName,
sp.AuthenticationParameters,
sp.PrivacyParameters,
sp.AuthenticationProtocol,
sp.PrivacyProtocol,
)
}

// Log logs security paramater information to the provided GoSNMP Logger
func (sp *UsmSecurityParameters) Log() {
sp.mu.Lock()
defer sp.mu.Unlock()
sp.Logger.Printf("SECURITY PARAMETERS:%+v", sp)
sp.Logger.Printf("SECURITY PARAMETERS:%s", sp.String())
}

// Copy method for UsmSecurityParameters used to copy a SnmpV3SecurityParameters without knowing it's implementation
Expand Down

0 comments on commit 2f729f9

Please sign in to comment.