From 480cca1d6cefc075dd262fb05e155408b8e02e0c Mon Sep 17 00:00:00 2001 From: Clark Li Date: Sun, 27 Feb 2022 14:22:40 +1030 Subject: [PATCH] Allow rotating secrets from ConnConfig - Remember the identified rotating secret index - Encrypt using the identified rotating secret --- conn.go | 71 +++++++++++++++-- conn_test.go | 212 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 275 insertions(+), 8 deletions(-) create mode 100644 conn_test.go diff --git a/conn.go b/conn.go index 7d07899..e9ed2ef 100644 --- a/conn.go +++ b/conn.go @@ -43,6 +43,7 @@ var ( errSessionNotFound = errors.New("session not found or timed out") errUnexpectedEOF = errors.New("unexpected EOF") errPacketQueueFull = errors.New("packet queue full") + errInvalidSecret = errors.New("invalid secret") ) // doneContext allows a done channel to be used as a context.Context @@ -89,6 +90,30 @@ func crypt(p, key []byte) { } } +// Check if a payload can be unmarshalled +func checkPayload(p []byte, c * ConnConfig) error { + if hdrType > len(p) - 1 { + return errors.New("Invalid packet length") + } + switch p[hdrType] { + case sessTypeAuthen: + as := new(AuthenStart) + err := as.unmarshal(p[hdrLen:]) + return err + case sessTypeAuthor: + as := new(AuthorRequest) + err := as.unmarshal(p[hdrLen:]) + return err + case sessTypeAcct: + as := new(AcctRequest) + err := as.unmarshal(p[hdrLen:]) + return err + default: + return errors.New("invalid session type") + } + return nil +} + // a packet can be marshalled to and from raw bytes type packet interface { marshal([]byte) ([]byte, error) // appends the encoded packet to the provided slice @@ -109,6 +134,7 @@ type session struct { in chan []byte // Buffered channel for incoming raw packet c *conn // Connection for session done chan struct{} // close channel to close session + rotatingSecretIndex int // Index of the identified rotating secrets mu sync.Mutex // Guards the following err error // last seen error @@ -180,7 +206,26 @@ func (s *session) readPacket(ctx context.Context) ([]byte, error) { return p, errInvalidSeqNo } - crypt(p, s.c.Secret) + if s.c.RotatingSecrets == nil { + crypt(p, s.c.Secret) + } else if s.rotatingSecretIndex >= 0 { + crypt(p, s.c.RotatingSecrets[s.rotatingSecretIndex]) + } else { + for index, secret := range s.c.RotatingSecrets { + crypt(p, secret) + err := checkPayload(p, &s.c.ConnConfig) + if err == nil { + s.rotatingSecretIndex = index + break + } else { + // Recover the packet to original + crypt(p, secret) + } + } + if s.rotatingSecretIndex < 0 { + return p, errInvalidSecret + } + } return p, nil } @@ -197,7 +242,14 @@ func (s *session) writePacket(ctx context.Context, p []byte) error { // set body size binary.BigEndian.PutUint32(p[hdrBodyLen:], uint32(len(p)-hdrLen)) - crypt(p, s.c.Secret) + + if s.c.RotatingSecrets == nil { + crypt(p, s.c.Secret) + } else if s.rotatingSecretIndex >= 0 { + crypt(p, s.c.RotatingSecrets[s.rotatingSecretIndex]) + } else { + return errors.New("No valid secret is found for encryption") + } wr := writeRequest{p: p, ec: make(chan error, 1)} if deadline, ok := ctx.Deadline(); ok { @@ -226,6 +278,7 @@ func newSession(c *conn, id uint32) *session { s := &session{id: id, c: c} s.in = make(chan []byte, 1) s.done = make(chan struct{}) + s.rotatingSecretIndex = -1 return s } @@ -260,12 +313,14 @@ type sessRequest struct { // // Timeout's are ignored if zero. type ConnConfig struct { - Mux bool // Allow sessions to be multiplexed over a single connection - LegacyMux bool // Allow session multiplexing without setting the single-connection header flag - Secret []byte // Shared secret key - IdleTimeout time.Duration // Time before closing an idle multiplexed connection with no sessions - ReadTimeout time.Duration // Maximum time to read a packet (not including waiting for first byte) - WriteTimeout time.Duration // Maximum time to write a packet + Mux bool // Allow sessions to be multiplexed over a single connection + LegacyMux bool // Allow session multiplexing without setting the single-connection header flag + Secret []byte // Shared secret key + RotatingSecrets [][]byte // Rotating shared secret keys. This is necessary when the new secret is still being rolled out. + // When it is set @ref Secret is discard + IdleTimeout time.Duration // Time before closing an idle multiplexed connection with no sessions + ReadTimeout time.Duration // Maximum time to read a packet (not including waiting for first byte) + WriteTimeout time.Duration // Maximum time to write a packet // Optional function to log errors. If not defined log.Print will be used. Log func(v ...interface{}) diff --git a/conn_test.go b/conn_test.go new file mode 100644 index 0000000..1b99df8 --- /dev/null +++ b/conn_test.go @@ -0,0 +1,212 @@ +package tacplus + +import ( + "context" + "encoding/binary" + "errors" + "fmt" + "reflect" + "testing" +) + +var payloadTestsTypes = []byte { + sessTypeAuthen, + sessTypeAuthor, + sessTypeAcct, +} + +var payloadTests = []packet{ + &AuthenStart{ + Action: AuthenActionSendAuth, + PrivLvl: 23, + AuthenType: AuthenTypeARAP, + AuthenService: AuthenServiceX25, + User: "fred", + Port: "tty00", + RemAddr: "10.1.2.3", + Data: []byte{0, 1, 2, 3, 0, 1}, + }, + &AuthorRequest{ + AuthenMethod: AuthenMethodKRB4, + PrivLvl: 99, + AuthenType: AuthenTypeMSCHAP, + AuthenService: AuthenServiceFWProxy, + User: "fred", + Port: "tty00", + RemAddr: "10.0.0.1", + Arg: []string{"protocol=ip", "timeout=1"}, + }, + &AcctRequest{ + Flags: AcctFlagMore, + AuthenMethod: AuthenMethodEnable, + PrivLvl: 15, + AuthenType: AuthenTypeCHAP, + AuthenService: AuthenServicePT, + User: "joe", + Port: "port23", + RemAddr: "192.168.1.1", + Arg: []string{"a=b", "c=d"}, + }, +} + +func connTestLog(v ...interface{}) { + if len(v) == 0 { + return + } + err, ok := v[0].(error) + if !ok { + err = errors.New(fmt.Sprint(v...)) + } + if err != nil { + fmt.Println(err) + } +} + +func TestCheckPayload(t *testing.T) { + for index, p := range payloadTests { + tp := reflect.Indirect(reflect.ValueOf(p)).Type() // get type + b := make([]byte, 128) + b, err := p.marshal(b[:hdrLen]) + if err != nil { + t.Error("marshal of", tp.Name(), p, "failed:", err) + continue + } + binary.BigEndian.PutUint32(b[hdrBodyLen:], uint32(len(b)-hdrLen)) + b[hdrType] = payloadTestsTypes[index] + // Encrypt then decret using the same secret + crypt(b, []byte("secret")) + crypt(b, []byte("secret")) + c := ConnConfig { + Log: connTestLog, + } + err = checkPayload(b, &c) + if err != nil { + t.Error("checkPayload of", tp.Name(), p, "failed:", err) + } + // Encrypt then decret using the wrong secret + crypt(b, []byte("secret")) + crypt(b, []byte("wrong_secret")) + err = checkPayload(b, &c) + if err == nil { + t.Error("checkPayload should have failed ", tp.Name(), p, ":", err) + } + } +} + +func TestReadPacket(t *testing.T) { + for index, p := range payloadTests { + tp := reflect.Indirect(reflect.ValueOf(p)).Type() // get type + b := make([]byte, 128) + b, err := p.marshal(b[:hdrLen]) + if err != nil { + t.Error("marshal of", tp.Name(), p, "failed:", err) + continue + } + binary.BigEndian.PutUint32(b[hdrBodyLen:], uint32(len(b)-hdrLen)) + b[hdrType] = payloadTestsTypes[index] + b[hdrSeqNo] = 1 + ctx := context.Background() + { + crypt(b, []byte("secret")) + c := conn { + ConnConfig: ConnConfig { + Log: connTestLog, + RotatingSecrets: [][]byte{[]byte("secret"), []byte("wrong_secret")}, + }, + } + s := newSession(&c, 42) + // Write the packet to the receive channel of session + s.in <- b + b, err = s.readPacket(ctx) + if err != nil { + t.Error("readPacket of", tp.Name(), p, "failed:", err) + } + if s.rotatingSecretIndex != 0 { + t.Error("readPacket uses wrong index", s.rotatingSecretIndex) + } + } + + // secret not in index 0 + { + crypt(b, []byte("secret")) + c := conn { + ConnConfig: ConnConfig { + Log: connTestLog, + RotatingSecrets: [][]byte{[]byte("wrong_secret"), []byte("secret")}, + }, + } + s := newSession(&c, 42) + // Write the packet to the receive channel of session + s.in <- b + b, err = s.readPacket(ctx) + if err != nil { + t.Error("readPacket of", tp.Name(), p, "failed:", err) + } + if s.rotatingSecretIndex != 1 { + t.Error("readPacket uses wrong index", s.rotatingSecretIndex) + } + } + // secret not found + { + crypt(b, []byte("secret")) + c := conn { + ConnConfig: ConnConfig { + Log: connTestLog, + RotatingSecrets: [][]byte{[]byte("wrong_secret")}, + }, + } + s := newSession(&c, 42) + // Write the packet to the receive channel of session + s.in <- b + b, err = s.readPacket(ctx) + if err == nil { + t.Error("readPacket of", tp.Name(), p, "should have failed:", err) + } + if s.rotatingSecretIndex != -1 { + t.Error("readPacket shouldn't have updated index", s.rotatingSecretIndex) + } + } + } +} + +func TestWritePacket(t *testing.T) { + b := make([]byte, 128) + b[hdrSeqNo + 1] = 1 + b[hdrSeqNo + 2] = 2 + ctx := context.Background() + c := conn { + ConnConfig: ConnConfig { + Log: connTestLog, + RotatingSecrets: [][]byte{[]byte("wrong_secret"), []byte("secret")}, + }, + wc : make(chan writeRequest), + } + s := newSession(&c, 42) + + err := s.writePacket(ctx, b) + if err == nil { + t.Error("writePacket of", b, " should have failed:", err) + } + + // Set up a valid rottaing secret index + s.rotatingSecretIndex = 1 + // Create a null function for packet write + go func (c *conn) { + for { + select { + case req := <-c.wc: + req.ec <- nil + } + } + } (&c) + err = s.writePacket(ctx, b) + if err != nil { + t.Error("writePacket of", b, " failed:", err) + } + // Verify that the packet can be decrypted + crypt(b, []byte("secret")) + // Don't try to check the headers + if b[hdrSeqNo + 1] != 1 || b[hdrSeqNo + 2] != 2 { + t.Error("Failed to decrypt packet") + } +}