Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support auditing chunked SQL Server packets #29228

Merged
merged 8 commits into from
Aug 9, 2023
Merged

Support auditing chunked SQL Server packets #29228

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
256bc4e
feat(sqlserver): parse chunked packets
gabrielcorado Jul 17, 2023
0a14a31
chore(sqlserver): fix typo
gabrielcorado Jul 20, 2023
b6ac9aa
fix(sqlserver): assert final packet on flag bit
gabrielcorado Jul 20, 2023
2d866f7
refactor(sqlserver): return chunks data using Bytes()
gabrielcorado Jul 21, 2023
615d838
chore(sqlserver): update comparision logic
gabrielcorado Jul 21, 2023
af6be1e
chore(sqlserver): typo
gabrielcorado Jul 24, 2023
522fd8c
Merge branch 'master' into gabrielcorado/audit-large-sql-server-packet
gabrielcorado Aug 8, 2023
43a2842
Merge branch 'master' into gabrielcorado/audit-large-sql-server-packet
gabrielcorado Aug 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 45 additions & 7 deletions lib/srv/db/sqlserver/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package sqlserver

import (
"bytes"
"context"
"io"
"net"
Expand Down Expand Up @@ -132,6 +133,11 @@ func (e *Engine) receiveFromClient(clientConn, serverConn io.ReadWriteCloser, cl
e.Log.Debug("Stop receiving from client.")
close(clientErrCh)
}()

// initialPacketHeader and chunkData are used to accumulate chunked packets
// to build a single packet with full contents for auditing.
var initialPacketHeader protocol.PacketHeader
var chunkData bytes.Buffer
for {
p, err := protocol.ReadPacket(clientConn)
if err != nil {
Expand All @@ -144,13 +150,24 @@ func (e *Engine) receiveFromClient(clientConn, serverConn io.ReadWriteCloser, cl
return
}

sqlPacket, err := protocol.ToSQLPacket(p)
switch {
case err != nil:
e.Log.WithError(err).Errorf("Failed to parse SQLServer packet.")
e.emitMalformedPacket(e.Context, sessionCtx, p)
default:
e.auditPacket(e.Context, sessionCtx, sqlPacket)
// Audit events are going to be emitted only on final messages, this way
// the packet parsing can be complete and provide the query/RPC
// contents.
if protocol.IsFinalPacket(p) {
sqlPacket, err := e.toSQLPacket(initialPacketHeader, p, &chunkData)
switch {
case err != nil:
e.Log.WithError(err).Errorf("Failed to parse SQLServer packet.")
e.emitMalformedPacket(e.Context, sessionCtx, p)
default:
e.auditPacket(e.Context, sessionCtx, sqlPacket)
}
} else {
if chunkData.Len() == 0 {
initialPacketHeader = p.Header()
}

chunkData.Write(p.Data())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this cause a DoS if a large enough packet was crafted?
Should we add a (configurable?) max size for the chunk data?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no good limit value that can be set. We can consider using https://learn.microsoft.com/en-us/sql/sql-server/maximum-capacity-specifications-for-sql-server?view=sql-server-ver16#:~:text=Bytes%20per%20varchar(max)%2C%20varbinary(max)%2C%20xml%2C%20text%2C%20or%20image%20column but this is a limit for single connection a client can establish many db conn and try enforce teleport db agent to run out of memory.
I wonder I we can add a debug/info log in case of handling large package and audit this operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't set a limit that will generate a valid packet because the RPC calls can be composed of multiple parameters. Although I prefer having predictable memory consumption, I don't know how we should handle those partial packets. We could stick with the current behavior of generating db.session.malformed_packet if a packet exceeds a threshold, but in that case, that could be used to bypass audit logs too.

For me, the best solution is to be able to parse those requests partially, so we can decide to split them into multiple audit logs. But this would require custom packet parsing (other than what we have from the go-mssqldb driver); I think it might not be worth doing it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be ok to return an error if the packet exceeds a couple of MBs.
Or, just truncate the packet (is it possible?) and add a sentinel flag indicating that this is a truncated message

For reference, AWS CloudTrail has a max of 256KB for their log message size.
https://docs.aws.amazon.com/awscloudtrail/latest/userguide/WhatIsCloudTrail-Limits.html

}

_, err = serverConn.Write(p.Bytes())
Expand All @@ -162,6 +179,27 @@ func (e *Engine) receiveFromClient(clientConn, serverConn io.ReadWriteCloser, cl
}
}

// toSQLPacket Parses a regular (self-contained) or chunked packet into an SQL
// packet (used for auditing).
func (e *Engine) toSQLPacket(header protocol.PacketHeader, packet *protocol.BasicPacket, chunks *bytes.Buffer) (protocol.Packet, error) {
if chunks.Len() > 0 {
defer chunks.Reset()
chunks.Write(packet.Data())
// We're safe to "read" chunk using `Bytes()` function beacuse the
// packet processing copies the packet contents.
packetData := chunks.Bytes()

var err error
// The final chucked packet header must be the first packet header.
packet, err = protocol.NewBasicPacket(header, packetData)
if err != nil {
return nil, trace.Wrap(err)
}
}

return protocol.ToSQLPacket(packet)
}

// receiveFromServer relays protocol messages received from MySQL database
// to MySQL client.
func (e *Engine) receiveFromServer(serverConn, clientConn io.ReadWriteCloser, serverErrCh chan<- error) {
Expand Down
145 changes: 132 additions & 13 deletions lib/srv/db/sqlserver/engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ func TestHandleConnectionAuditEvents(t *testing.T) {
}

tests := []struct {
name string
packet []byte
checks []check
name string
packets [][]byte
checks []check
}{
{
name: "rpc request procedure",
packet: fixtures.RPCClientRequest,
name: "rpc request procedure",
packets: [][]byte{fixtures.GenerateCustomRPCCallPacket("foo3")},
checks: []check{
hasNoErr(),
hasAuditEventCode(libevents.DatabaseSessionStartCode),
Expand All @@ -91,8 +91,8 @@ func TestHandleConnectionAuditEvents(t *testing.T) {
},
},
{
name: "rpc request param",
packet: fixtures.RPCClientRequestParam,
name: "rpc request param",
packets: [][]byte{fixtures.GenerateExecuteSQLRPCPacket("select @@version")},
checks: []check{
hasNoErr(),
hasAuditEventCode(libevents.DatabaseSessionStartCode),
Expand All @@ -112,8 +112,8 @@ func TestHandleConnectionAuditEvents(t *testing.T) {
},
},
{
name: "sql batch",
packet: fixtures.SQLBatch,
name: "sql batch",
packets: [][]byte{fixtures.GenerateBatchQueryPacket("\nselect 'foo' as 'bar'\n ")},
checks: []check{
hasNoErr(),
hasAuditEventCode(libevents.DatabaseSessionStartCode),
Expand All @@ -135,15 +135,120 @@ func TestHandleConnectionAuditEvents(t *testing.T) {
},
},
{
name: "malformed packet",
packet: fixtures.MalformedPacketTest,
name: "malformed packet",
packets: [][]byte{fixtures.MalformedPacketTest},
checks: []check{
hasNoErr(),
hasAuditEventCode(libevents.DatabaseSessionStartCode),
hasAuditEventCode(libevents.DatabaseSessionEndCode),
hasAuditEventCode(libevents.DatabaseSessionMalformedPacketCode),
},
},
{
name: "sql batch chunked packets",
packets: fixtures.GenerateBatchQueryChunkedPacket(5, "select 'foo' as 'bar'"),
checks: []check{
hasNoErr(),
hasAuditEventCode(libevents.DatabaseSessionStartCode),
hasAuditEventCode(libevents.DatabaseSessionEndCode),
hasAuditEvent(1, &events.DatabaseSessionQuery{
DatabaseMetadata: events.DatabaseMetadata{
DatabaseUser: "sa",
DatabaseType: "self-hosted",
},
Metadata: events.Metadata{
Type: libevents.DatabaseSessionQueryEvent,
Code: libevents.DatabaseSessionQueryCode,
},
DatabaseQuery: "select 'foo' as 'bar'",
Status: events.Status{
Success: true,
},
}),
},
},
{
name: "rpc request param chunked",
packets: fixtures.GenerateExecuteSQLRPCChunkedPacket(5, "select @@version"),
checks: []check{
hasNoErr(),
hasAuditEventCode(libevents.DatabaseSessionStartCode),
hasAuditEventCode(libevents.DatabaseSessionEndCode),
hasAuditEvent(1, &events.SQLServerRPCRequest{
DatabaseMetadata: events.DatabaseMetadata{
DatabaseUser: "sa",
DatabaseType: "self-hosted",
},
Metadata: events.Metadata{
Type: libevents.DatabaseSessionSQLServerRPCRequestEvent,
Code: libevents.SQLServerRPCRequestCode,
},
Parameters: []string{"select @@version"},
Procname: "Sp_ExecuteSql",
}),
},
},
{
name: "intercalated chunked messages",
packets: intercalateChunkedPacketMessages(
fixtures.GenerateExecuteSQLRPCChunkedPacket(5, "select @@version"),
fixtures.GenerateExecuteSQLRPCPacket("select 1"),
2,
),
checks: []check{
hasNoErr(),
hasAuditEventCode(libevents.DatabaseSessionStartCode),
hasAuditEventCode(libevents.DatabaseSessionEndCode),
hasAuditEvent(1, &events.SQLServerRPCRequest{
DatabaseMetadata: events.DatabaseMetadata{
DatabaseUser: "sa",
DatabaseType: "self-hosted",
},
Metadata: events.Metadata{
Type: libevents.DatabaseSessionSQLServerRPCRequestEvent,
Code: libevents.SQLServerRPCRequestCode,
},
Parameters: []string{"select @@version"},
Procname: "Sp_ExecuteSql",
}),
hasAuditEvent(2, &events.SQLServerRPCRequest{
DatabaseMetadata: events.DatabaseMetadata{
DatabaseUser: "sa",
DatabaseType: "self-hosted",
},
Metadata: events.Metadata{
Type: libevents.DatabaseSessionSQLServerRPCRequestEvent,
Code: libevents.SQLServerRPCRequestCode,
},
Parameters: []string{"select 1"},
Procname: "Sp_ExecuteSql",
}),
hasAuditEvent(3, &events.SQLServerRPCRequest{
DatabaseMetadata: events.DatabaseMetadata{
DatabaseUser: "sa",
DatabaseType: "self-hosted",
},
Metadata: events.Metadata{
Type: libevents.DatabaseSessionSQLServerRPCRequestEvent,
Code: libevents.SQLServerRPCRequestCode,
},
Parameters: []string{"select @@version"},
Procname: "Sp_ExecuteSql",
}),
hasAuditEvent(4, &events.SQLServerRPCRequest{
DatabaseMetadata: events.DatabaseMetadata{
DatabaseUser: "sa",
DatabaseType: "self-hosted",
},
Metadata: events.Metadata{
Type: libevents.DatabaseSessionSQLServerRPCRequestEvent,
Code: libevents.SQLServerRPCRequestCode,
},
Parameters: []string{"select 1"},
Procname: "Sp_ExecuteSql",
}),
},
},
}

for _, tc := range tests {
Expand All @@ -152,8 +257,11 @@ func TestHandleConnectionAuditEvents(t *testing.T) {
_, err := b.Write(fixtures.Login7)
require.NoError(t, err)

_, err = b.Write(tc.packet)
require.NoError(t, err)
for _, packet := range tc.packets {
_, err = b.Write(packet)
require.NoError(t, err)
}

emitterMock := &eventstest.MockRecorderEmitter{}
audit, err := common.NewAudit(common.AuditConfig{
Emitter: emitterMock,
Expand Down Expand Up @@ -189,6 +297,17 @@ func TestHandleConnectionAuditEvents(t *testing.T) {
}
}

// intercalateChunkedPacketMessages intercalates a chunked packet with a regular packet a specified number of times.
func intercalateChunkedPacketMessages(chunkedPacket [][]byte, regularPacket []byte, repeat int) [][]byte {
var result [][]byte
for i := 0; i < repeat; i++ {
result = append(result, chunkedPacket...)
result = append(result, regularPacket)
}

return result
}

type mockConn struct {
net.Conn
buff bytes.Buffer
Expand Down
5 changes: 3 additions & 2 deletions lib/srv/db/sqlserver/protocol/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ const (
// packetHeaderSize is the size of the protocol packet header.
packetHeaderSize = 8

// packetStatusLast indicates that the packet is the last in the request.
packetStatusLast uint8 = 0x01
// PacketStatusLast indicates that the packet is the last in the request.
gabrielcorado marked this conversation as resolved.
Show resolved Hide resolved
// https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-tds/ce398f9a-7d47-4ede-8f36-9dd6fc21ca43
PacketStatusLast uint8 = 0x01

preLoginOptionVersion = 0x00
preLoginOptionEncryption = 0x01
Expand Down