Skip to content

Support auditing chunked SQL Server packets #29228

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

Merged
gabrielcorado merged 8 commits into from
Aug 9, 2023
Merged

Support auditing chunked SQL Server packets #29228

Show file tree
Hide file tree
Changes from all 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 44 additions & 8 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 @@ -138,6 +139,10 @@ func (e *Engine) receiveFromClient(clientConn, serverConn io.ReadWriteCloser, cl
}()

msgFromClient := common.GetMessagesFromClientMetric(sessionCtx.Database)
// 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)
Expand All @@ -150,16 +155,26 @@ func (e *Engine) receiveFromClient(clientConn, serverConn io.ReadWriteCloser, cl
clientErrCh <- err
return
}

msgFromClient.Inc()

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
Copy Markdown
Contributor

@marcoandredinis marcoandredinis Jul 21, 2023

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
Copy Markdown
Contributor

@smallinsky smallinsky Jul 24, 2023

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
Copy Markdown
Contributor Author

@gabrielcorado gabrielcorado Jul 24, 2023

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
Copy Markdown
Contributor

@marcoandredinis marcoandredinis Aug 7, 2023

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 @@ -171,6 +186,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 because 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
163 changes: 150 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 @@ -94,8 +94,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 @@ -118,8 +118,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 @@ -144,15 +144,138 @@ 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",
DatabaseService: "dummy",
DatabaseURI: "uri",
DatabaseProtocol: "test",
},
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",
DatabaseService: "dummy",
DatabaseURI: "uri",
DatabaseProtocol: "test",
},
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",
DatabaseService: "dummy",
DatabaseURI: "uri",
DatabaseProtocol: "test",
},
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",
DatabaseService: "dummy",
DatabaseURI: "uri",
DatabaseProtocol: "test",
},
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",
DatabaseService: "dummy",
DatabaseURI: "uri",
DatabaseProtocol: "test",
},
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",
DatabaseService: "dummy",
DatabaseURI: "uri",
DatabaseProtocol: "test",
},
Metadata: events.Metadata{
Type: libevents.DatabaseSessionSQLServerRPCRequestEvent,
Code: libevents.SQLServerRPCRequestCode,
},
Parameters: []string{"select 1"},
Procname: "Sp_ExecuteSql",
}),
},
},
}

for _, tc := range tests {
Expand All @@ -170,8 +293,11 @@ func TestHandleConnectionAuditEvents(t *testing.T) {
})
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 @@ -208,6 +334,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.
Comment thread
gabrielcorado marked this conversation as 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
Loading