From 3ac449e2a67805284992c1070579b35c03bbd81d Mon Sep 17 00:00:00 2001 From: Waldz Date: Thu, 23 Apr 2020 20:48:23 +0300 Subject: [PATCH 1/2] Expose server's middleware for incoming clients control --- openvpn/middlewares/server/auth/middleware.go | 144 +++++++++--------- .../server/auth/middleware_test.go | 127 +++++++-------- .../server/credentials/middleware.go | 78 ++++++++++ .../server/credentials/middleware_test.go | 93 +++++++++++ openvpn/middlewares/server/event.go | 11 +- .../middlewares/server/filter/middleware.go | 53 ++----- .../server/filter/middleware_test.go | 35 +---- 7 files changed, 330 insertions(+), 211 deletions(-) create mode 100644 openvpn/middlewares/server/credentials/middleware.go create mode 100644 openvpn/middlewares/server/credentials/middleware_test.go diff --git a/openvpn/middlewares/server/auth/middleware.go b/openvpn/middlewares/server/auth/middleware.go index 6efa12a..b6c1186 100644 --- a/openvpn/middlewares/server/auth/middleware.go +++ b/openvpn/middlewares/server/auth/middleware.go @@ -19,41 +19,89 @@ package auth import ( "strings" + "sync" "github.com/mysteriumnetwork/go-openvpn/openvpn/log" "github.com/mysteriumnetwork/go-openvpn/openvpn/management" "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server" ) -type middleware struct { - // TODO: consider implementing event channel to communicate required callbacks - credentialsValidator CredentialsValidator - commandWriter management.CommandWriter - currentEvent server.ClientEvent -} +// ClientEventCallback is called when state of each Openvp client changes. +type ClientEventCallback func(event server.ClientEvent) + +// Middleware is able to process client auth events, exposes client control API. +// +// The OpenVPN server should have been started with the +// --management-client-auth directive so that it will ask the management +// interface to approve client connections. +type Middleware struct { + commandWriter management.CommandWriter + currentEvent server.ClientEvent -// CredentialsValidator callback checks given auth primitives (i.e. customer identity signature / node's sessionId) -type CredentialsValidator func(clientID int, username, password string) (bool, error) + listenersMu sync.RWMutex + listeners []ClientEventCallback +} -// NewMiddleware creates server user_auth challenge authentication middleware -func NewMiddleware(credentialsValidator CredentialsValidator) *middleware { - return &middleware{ - credentialsValidator: credentialsValidator, - commandWriter: nil, - currentEvent: server.UndefinedEvent, +// NewMiddleware creates new instance of Middleware. +func NewMiddleware(listeners ...ClientEventCallback) *Middleware { + return &Middleware{ + commandWriter: nil, + currentEvent: server.UndefinedEvent(), + listeners: listeners, } } -func (m *middleware) Start(commandWriter management.CommandWriter) error { +// ClientsSubscribe subscribes to Openvpn clients states. +func (m *Middleware) ClientsSubscribe(callback ClientEventCallback) { + m.listenersMu.Lock() + defer m.listenersMu.Unlock() + + m.listeners = append(m.listeners, callback) +} + +// ClientAccept client control which allows authorisation (for CONNECT or REAUTH state). +func (m *Middleware) ClientAccept(clientID, keyID int) error { + _, err := m.commandWriter.SingleLineCommand("client-auth-nt %d %d", clientID, keyID) + return err +} + +// ClientDeny client control which forbids authorisation (for CONNECT or REAUTH state). +func (m *Middleware) ClientDeny(clientID, keyID int, message string) error { + _, err := m.commandWriter.SingleLineCommand("client-deny %d %d", clientID, keyID, message) + return err +} + +// ClientDenyWithMessage client control which forbids authorisation with reason message (for CONNECT or REAUTH state). +func (m *Middleware) ClientDenyWithMessage(clientID, keyID int, message string) error { + _, err := m.commandWriter.SingleLineCommand("client-deny %d %d %s", clientID, keyID, message) + return err +} + +// ClientKill client control which stops established connection (for ESTABLISHED state). +func (m *Middleware) ClientKill(clientID int) error { + _, err := m.commandWriter.SingleLineCommand("client-kill %d", clientID) + return err +} + +// ClientKillWithMessage client control which stops established connection with reason message (for ESTABLISHED state). +func (m *Middleware) ClientKillWithMessage(clientID int, message string) error { + _, err := m.commandWriter.SingleLineCommand("client-kill %d %s", clientID, message) + return err +} + +// Start starts the middleware. +func (m *Middleware) Start(commandWriter management.CommandWriter) error { m.commandWriter = commandWriter return nil } -func (m *middleware) Stop(commandWriter management.CommandWriter) error { +// Stop stops the middleware. +func (m *Middleware) Stop(_ management.CommandWriter) error { return nil } -func (m *middleware) ConsumeLine(line string) (bool, error) { +// ConsumeLine handles the given openvpn management line. +func (m *Middleware) ConsumeLine(line string) (bool, error) { if !strings.HasPrefix(line, ">CLIENT:") { return false, nil } @@ -98,69 +146,29 @@ func (m *middleware) ConsumeLine(line string) (bool, error) { return true, nil } -func (m *middleware) startOfEvent(eventType server.ClientEventType, clientID int, keyID int) { +func (m *Middleware) startOfEvent(eventType server.ClientEventType, clientID int, keyID int) { m.currentEvent.EventType = eventType m.currentEvent.ClientID = clientID m.currentEvent.ClientKey = keyID } -func (m *middleware) addEnvVar(key string, val string) { +func (m *Middleware) addEnvVar(key string, val string) { m.currentEvent.Env[key] = val } -func (m *middleware) endOfEvent() { - m.handleClientEvent(m.currentEvent) - m.reset() -} +func (m *Middleware) endOfEvent() { + m.listenersMu.RLock() + defer m.listenersMu.RUnlock() -func (m *middleware) reset() { - m.currentEvent = server.UndefinedEvent -} - -func (m *middleware) handleClientEvent(event server.ClientEvent) { - switch event.EventType { - case server.Connect, server.Reauth: - username := event.Env["username"] - password := event.Env["password"] - err := m.authenticateClient(event.ClientID, event.ClientKey, username, password) - if err != nil { - log.Error("Unable to authenticate client:", err) + if m.listeners != nil { + for _, subscription := range m.listeners { + subscription(m.currentEvent) } - case server.Established: - log.Info("Client with ID:", event.ClientID, "connection established successfully") - case server.Disconnect: - log.Info("Client with ID:", event.ClientID, "disconnected") - // NOTE: do not cleanup session after disconnect event risen by transport itself - // cleanup session only by user's intent - } -} - -func (m *middleware) authenticateClient(clientID, clientKey int, username, password string) error { - - if username == "" || password == "" { - return denyClientAuthWithMessage(m.commandWriter, clientID, clientKey, "missing username or password") } - log.Info("Authenticating user:", username, "clientID:", clientID, "clientKey:", clientKey) - - authenticated, err := m.credentialsValidator(clientID, username, password) - if err != nil { - log.Error("Authentication error:", err) - return denyClientAuthWithMessage(m.commandWriter, clientID, clientKey, "internal error") - } - - if authenticated { - return approveClient(m.commandWriter, clientID, clientKey) - } - return denyClientAuthWithMessage(m.commandWriter, clientID, clientKey, "wrong username or password") -} - -func approveClient(commandWriter management.CommandWriter, clientID, keyID int) error { - _, err := commandWriter.SingleLineCommand("client-auth-nt %d %d", clientID, keyID) - return err + m.reset() } -func denyClientAuthWithMessage(commandWriter management.CommandWriter, clientID, keyID int, message string) error { - _, err := commandWriter.SingleLineCommand("client-deny %d %d %s", clientID, keyID, message) - return err +func (m *Middleware) reset() { + m.currentEvent = server.UndefinedEvent() } diff --git a/openvpn/middlewares/server/auth/middleware_test.go b/openvpn/middlewares/server/auth/middleware_test.go index b1c2d0b..04005b2 100644 --- a/openvpn/middlewares/server/auth/middleware_test.go +++ b/openvpn/middlewares/server/auth/middleware_test.go @@ -21,29 +21,10 @@ import ( "testing" "github.com/mysteriumnetwork/go-openvpn/openvpn/management" + "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server" "github.com/stretchr/testify/assert" ) -type fakeAuthenticatorStub struct { - called bool - username string - password string - authenticated bool -} - -func (f *fakeAuthenticatorStub) fakeAuthenticator(clientID int, username, password string) (bool, error) { - f.called = true - f.username = username - f.password = password - return f.authenticated, nil -} - -func Test_Factory(t *testing.T) { - fas := fakeAuthenticatorStub{} - middleware := NewMiddleware(fas.fakeAuthenticator) - assert.NotNil(t, middleware) -} - func Test_ConsumeLineSkips(t *testing.T) { var tests = []struct { line string @@ -53,8 +34,7 @@ func Test_ConsumeLineSkips(t *testing.T) { {">PASSWORD"}, {">USERNAME"}, } - fas := fakeAuthenticatorStub{} - middleware := NewMiddleware(fas.fakeAuthenticator) + middleware := NewMiddleware() for _, test := range tests { consumed, err := middleware.ConsumeLine(test.line) @@ -73,8 +53,7 @@ func Test_ConsumeLineTakes(t *testing.T) { {">CLIENT:ENV,username=username"}, } - fas := fakeAuthenticatorStub{} - middleware := NewMiddleware(fas.fakeAuthenticator) + middleware := NewMiddleware() mockConnection := &management.MockConnection{} middleware.Start(mockConnection) @@ -85,48 +64,33 @@ func Test_ConsumeLineTakes(t *testing.T) { } } -func Test_ConsumeLineAuthState(t *testing.T) { +func Test_ConsumeLineShouldNotTriggerClientState(t *testing.T) { var tests = []struct { line string }{ + {">CLIENT:ENV,password=12341234"}, + {">CLIENT:ENV,username=username"}, {">CLIENT:REAUTH,0,0"}, {">CLIENT:CONNECT,0,0"}, } for _, test := range tests { - fas := fakeAuthenticatorStub{} - middleware := NewMiddleware(fas.fakeAuthenticator) - mockConnection := &management.MockConnection{} - middleware.Start(mockConnection) - - consumed, err := middleware.ConsumeLine(test.line) - assert.NoError(t, err, test.line) - assert.True(t, consumed, test.line) - } -} - -func Test_ConsumeLineNotAuthState(t *testing.T) { - var tests = []struct { - line string - }{ - {">CLIENT:ENV,password=12341234"}, - {">CLIENT:ENV,username=username"}, - } + var receivedEvent *server.ClientEvent + middleware := NewMiddleware(func(e server.ClientEvent) { + receivedEvent = &e + }) - for _, test := range tests { - fas := fakeAuthenticatorStub{} - middleware := NewMiddleware(fas.fakeAuthenticator) mockConnection := &management.MockConnection{} middleware.Start(mockConnection) consumed, err := middleware.ConsumeLine(test.line) assert.NoError(t, err, test.line) assert.True(t, consumed, test.line) - assert.False(t, fas.called) + assert.Nil(t, receivedEvent) } } -func Test_ConsumeLineAuthTrueChecker(t *testing.T) { +func Test_ConsumeLineShouldTriggerClientStateAfterReceivingEnvironment(t *testing.T) { var tests = []struct { line string }{ @@ -135,9 +99,12 @@ func Test_ConsumeLineAuthTrueChecker(t *testing.T) { {">CLIENT:ENV,username=username1"}, {">CLIENT:ENV,END"}, } - fas := fakeAuthenticatorStub{} - fas.authenticated = true - middleware := NewMiddleware(fas.fakeAuthenticator) + + var receivedEvent server.ClientEvent + middleware := NewMiddleware(func(e server.ClientEvent) { + receivedEvent = e + }) + mockConnection := &management.MockConnection{} middleware.Start(mockConnection) @@ -146,31 +113,49 @@ func Test_ConsumeLineAuthTrueChecker(t *testing.T) { assert.NoError(t, err, test.line) assert.True(t, consumed, test.line) } - assert.True(t, fas.called) - assert.Equal(t, "username1", fas.username) - assert.Equal(t, "12341234", fas.password) - assert.Equal(t, "client-auth-nt 1 2", mockConnection.LastLine) + assert.Equal( + t, + server.ClientEvent{ + EventType: server.Connect, + ClientID: 1, + ClientKey: 2, + Env: map[string]string{ + "username": "username1", + "password": "12341234", + }, + }, + receivedEvent, + ) } -func Test_ConsumeLineAuthFalseChecker(t *testing.T) { - var tests = []struct { - line string - }{ - {">CLIENT:CONNECT,3,4"}, - {">CLIENT:ENV,username=bad"}, - {">CLIENT:ENV,password=12341234"}, - {">CLIENT:ENV,END"}, +func Test_ConsumeLinesAcceptsClientIdsAntKeysWithSeveralDigits(t *testing.T) { + var tests = []string{ + ">CLIENT:CONNECT,115,23", + ">CLIENT:ENV,END", } - fas := fakeAuthenticatorStub{} - fas.authenticated = false - middleware := NewMiddleware(fas.fakeAuthenticator) + + var receivedEvent server.ClientEvent + middleware := NewMiddleware(func(e server.ClientEvent) { + receivedEvent = e + }) + mockConnection := &management.MockConnection{} middleware.Start(mockConnection) - for _, test := range tests { - consumed, err := middleware.ConsumeLine(test.line) - assert.NoError(t, err, test.line) - assert.True(t, consumed, test.line) + for _, testLine := range tests { + consumed, err := middleware.ConsumeLine(testLine) + assert.NoError(t, err, testLine) + assert.Equal(t, true, consumed, testLine) } - assert.Equal(t, "client-deny 3 4 wrong username or password", mockConnection.LastLine) + + assert.Equal( + t, + server.ClientEvent{ + EventType: server.Connect, + ClientID: 115, + ClientKey: 23, + Env: map[string]string{}, + }, + receivedEvent, + ) } diff --git a/openvpn/middlewares/server/credentials/middleware.go b/openvpn/middlewares/server/credentials/middleware.go new file mode 100644 index 0000000..20a752c --- /dev/null +++ b/openvpn/middlewares/server/credentials/middleware.go @@ -0,0 +1,78 @@ +/* + * Copyright (C) 2020 The "MysteriumNetwork/go-openvpn" Authors. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package credentials + +import ( + "github.com/mysteriumnetwork/go-openvpn/openvpn/log" + "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server" + "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server/auth" +) + +// Middleware is able to process authorize incoming clients by given credentials validator callback. +type Middleware struct { + *auth.Middleware + + validator Validator +} + +// Validator callback checks given auth primitives. +type Validator func(clientID int, username, password string) (bool, error) + +// NewMiddleware creates server user_auth challenge authentication Middleware +func NewMiddleware(validator Validator) *Middleware { + m := &Middleware{ + validator: validator, + } + m.Middleware = auth.NewMiddleware(m.handleClientEvent) + return m +} + +func (m *Middleware) handleClientEvent(event server.ClientEvent) { + switch event.EventType { + case server.Connect, server.Reauth: + username := event.Env["username"] + password := event.Env["password"] + err := m.authenticateClient(event.ClientID, event.ClientKey, username, password) + if err != nil { + log.Error("Unable to authenticate client:", err) + } + case server.Established: + log.Info("Client with ID:", event.ClientID, "connection established successfully") + case server.Disconnect: + log.Info("Client with ID:", event.ClientID, "disconnected") + } +} + +func (m *Middleware) authenticateClient(clientID, clientKey int, username, password string) error { + log.Info("Authenticating user:", username, "clientID:", clientID, "clientKey:", clientKey) + if username == "" || password == "" { + return m.ClientDenyWithMessage(clientID, clientKey, "missing username or password") + } + + authenticated, err := m.validator(clientID, username, password) + if err != nil { + log.Error("Authentication error:", err) + return m.ClientDenyWithMessage(clientID, clientKey, "internal error") + } + + if !authenticated { + return m.ClientDenyWithMessage(clientID, clientKey, "wrong username or password") + } + + return m.ClientAccept(clientID, clientKey) +} diff --git a/openvpn/middlewares/server/credentials/middleware_test.go b/openvpn/middlewares/server/credentials/middleware_test.go new file mode 100644 index 0000000..0a4a2e3 --- /dev/null +++ b/openvpn/middlewares/server/credentials/middleware_test.go @@ -0,0 +1,93 @@ +/* + * Copyright (C) 2020 The "MysteriumNetwork/go-openvpn" Authors. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package credentials + +import ( + "testing" + + "github.com/mysteriumnetwork/go-openvpn/openvpn/management" + "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server" + "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server/auth" + "github.com/stretchr/testify/assert" +) + +type fakeValidator struct { + called bool +} + +func (f *fakeValidator) authenticateClient(clientID int, username, password string) (bool, error) { + f.called = true + return username == "username1" && password == "12341234", nil +} + +func mockAuthMiddleware(connection *management.MockConnection) *auth.Middleware { + m := auth.NewMiddleware() + m.Start(connection) + return m +} + +func Test_Factory(t *testing.T) { + fas := fakeValidator{} + middleware := NewMiddleware(fas.authenticateClient) + assert.NotNil(t, middleware) +} + +func Test_ConsumeLineAuthTrueChecker(t *testing.T) { + fas := fakeValidator{} + + mockConnection := &management.MockConnection{} + middleware := NewMiddleware(fas.authenticateClient) + middleware.Start(mockConnection) + + middleware.handleClientEvent(server.ClientEvent{ + EventType: server.Connect, + ClientID: 3, + ClientKey: 4, + Env: map[string]string{ + "username": "username1", + "password": "12341234", + }, + }) + assert.Equal(t, "client-auth-nt 3 4", mockConnection.LastLine) +} + +func Test_ConsumeLineAuthFalseChecker(t *testing.T) { + fas := fakeValidator{} + + mockConnection := &management.MockConnection{} + middleware := NewMiddleware(fas.authenticateClient) + middleware.Start(mockConnection) + + middleware.handleClientEvent(server.ClientEvent{ + EventType: server.Connect, + ClientID: 3, + ClientKey: 4, + }) + assert.Equal(t, "client-deny 3 4 missing username or password", mockConnection.LastLine) + + middleware.handleClientEvent(server.ClientEvent{ + EventType: server.Connect, + ClientID: 3, + ClientKey: 4, + Env: map[string]string{ + "username": "username1", + "password": "wrong", + }, + }) + assert.Equal(t, "client-deny 3 4 wrong username or password", mockConnection.LastLine) +} diff --git a/openvpn/middlewares/server/event.go b/openvpn/middlewares/server/event.go index 8ea3de4..2580bc0 100644 --- a/openvpn/middlewares/server/event.go +++ b/openvpn/middlewares/server/event.go @@ -47,8 +47,11 @@ type ClientEvent struct { } // UndefinedEvent is an empty OpenVPN management client event. -var UndefinedEvent = ClientEvent{ - ClientID: Undefined, - ClientKey: Undefined, - Env: make(map[string]string), +// Note: can't be a var, because `ClientEvent.Env` variable is being overwritten as it's a reference to same memory. +func UndefinedEvent() ClientEvent { + return ClientEvent{ + ClientID: Undefined, + ClientKey: Undefined, + Env: make(map[string]string), + } } diff --git a/openvpn/middlewares/server/filter/middleware.go b/openvpn/middlewares/server/filter/middleware.go index dc5da53..bedf42d 100644 --- a/openvpn/middlewares/server/filter/middleware.go +++ b/openvpn/middlewares/server/filter/middleware.go @@ -20,11 +20,11 @@ package filter import ( "bytes" "html/template" - "strings" "github.com/mysteriumnetwork/go-openvpn/openvpn/log" "github.com/mysteriumnetwork/go-openvpn/openvpn/management" "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server" + "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server/auth" ) const filterLANTemplate = `client-pf {{.ClientID}} @@ -42,59 +42,34 @@ END var filterLAN = template.Must(template.New("filter_lan").Parse(filterLANTemplate)) +// Exposes API to control client's packet filtering. +// +// The OpenVPN server should have been started with the +// --management-client-pf directive so that it will require that +// VPN tunnel packets sent or received by client instances must +// conform to that client's packet filter configuration. type middleware struct { + *auth.Middleware + commandWriter management.CommandWriter - currentEvent server.ClientEvent allow []string block []string } -// NewMiddleware creates server user_auth challenge authentication middleware +// NewMiddleware creates new instance of middleware func NewMiddleware(allow, block []string) *middleware { - return &middleware{ + m := &middleware{ commandWriter: nil, allow: allow, block: block, } + m.Middleware = auth.NewMiddleware(m.handleClientEvent) + return m } func (m *middleware) Start(commandWriter management.CommandWriter) error { m.commandWriter = commandWriter - return nil -} - -func (m *middleware) Stop(commandWriter management.CommandWriter) error { - return nil -} - -func (m *middleware) ConsumeLine(line string) (bool, error) { - if !strings.HasPrefix(line, ">CLIENT:") { - return false, nil - } - - clientLine := strings.TrimPrefix(line, ">CLIENT:") - - eventType, eventData, err := server.ParseClientEvent(clientLine) - if err != nil { - return true, err - } - - switch eventType { - case server.Connect, server.Reauth: - clientID, _, err := server.ParseIDAndKey(eventData) - if err != nil { - return true, err - } - - m.currentEvent.EventType = eventType - m.currentEvent.ClientID = clientID - case server.Env: - if strings.ToLower(eventData) == "end" { - m.handleClientEvent(m.currentEvent) - } - } - - return true, nil + return m.Middleware.Start(commandWriter) } func (m *middleware) handleClientEvent(event server.ClientEvent) { diff --git a/openvpn/middlewares/server/filter/middleware_test.go b/openvpn/middlewares/server/filter/middleware_test.go index 167c882..6002359 100644 --- a/openvpn/middlewares/server/filter/middleware_test.go +++ b/openvpn/middlewares/server/filter/middleware_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/mysteriumnetwork/go-openvpn/openvpn/management" + "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server" "github.com/stretchr/testify/assert" ) @@ -63,17 +64,11 @@ END ) func Test_EmptyPacketFilterForEmptySubnets(t *testing.T) { - middleware := NewMiddleware(nil, nil) mockConnection := &management.MockConnection{} + middleware := NewMiddleware(nil, nil) middleware.Start(mockConnection) - consumed, err := middleware.ConsumeLine(">CLIENT:CONNECT,0,0") - assert.NoError(t, err) - assert.True(t, consumed) - consumed, err = middleware.ConsumeLine(">CLIENT:ENV,END") - assert.NoError(t, err) - assert.True(t, consumed) - + middleware.handleClientEvent(server.ClientEvent{EventType: server.Connect, ClientID: 0}) assert.Equal(t, emptyFilter, mockConnection.WrittenLines[0]) } @@ -82,13 +77,7 @@ func Test_AllowPacketFilterForAllowSubnets(t *testing.T) { mockConnection := &management.MockConnection{} middleware.Start(mockConnection) - consumed, err := middleware.ConsumeLine(">CLIENT:CONNECT,0,0") - assert.NoError(t, err) - assert.True(t, consumed) - consumed, err = middleware.ConsumeLine(">CLIENT:ENV,END") - assert.NoError(t, err) - assert.True(t, consumed) - + middleware.handleClientEvent(server.ClientEvent{EventType: server.Connect, ClientID: 0}) assert.Equal(t, allowFilter, mockConnection.WrittenLines[0]) } @@ -97,13 +86,7 @@ func Test_BlockPacketFilterForBlockSubnets(t *testing.T) { mockConnection := &management.MockConnection{} middleware.Start(mockConnection) - consumed, err := middleware.ConsumeLine(">CLIENT:CONNECT,0,0") - assert.NoError(t, err) - assert.True(t, consumed) - consumed, err = middleware.ConsumeLine(">CLIENT:ENV,END") - assert.NoError(t, err) - assert.True(t, consumed) - + middleware.handleClientEvent(server.ClientEvent{EventType: server.Connect, ClientID: 0}) assert.Equal(t, blockFilter, mockConnection.WrittenLines[0]) } @@ -112,12 +95,6 @@ func Test_BothPacketFilterForBothSubnets(t *testing.T) { mockConnection := &management.MockConnection{} middleware.Start(mockConnection) - consumed, err := middleware.ConsumeLine(">CLIENT:CONNECT,0,0") - assert.NoError(t, err) - assert.True(t, consumed) - consumed, err = middleware.ConsumeLine(">CLIENT:ENV,END") - assert.NoError(t, err) - assert.True(t, consumed) - + middleware.handleClientEvent(server.ClientEvent{EventType: server.Connect, ClientID: 0}) assert.Equal(t, bothFilter, mockConnection.WrittenLines[0]) } From ae7446984e47461839f1d8c2b37c8fa1356a6b6b Mon Sep 17 00:00:00 2001 From: Valdas Petrulis <2713911+Waldz@users.noreply.github.com> Date: Wed, 29 Apr 2020 12:38:37 +0300 Subject: [PATCH 2/2] Factory improvements --- openvpn/middlewares/server/auth/middleware.go | 26 ++++++++----------- .../server/credentials/middleware.go | 7 +++-- .../server/credentials/middleware_test.go | 7 ----- .../middlewares/server/filter/middleware.go | 8 +++--- 4 files changed, 17 insertions(+), 31 deletions(-) diff --git a/openvpn/middlewares/server/auth/middleware.go b/openvpn/middlewares/server/auth/middleware.go index b6c1186..6e3a641 100644 --- a/openvpn/middlewares/server/auth/middleware.go +++ b/openvpn/middlewares/server/auth/middleware.go @@ -26,10 +26,10 @@ import ( "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server" ) -// ClientEventCallback is called when state of each Openvp client changes. +// ClientEventCallback is called when state of each OpenVPN client changes. type ClientEventCallback func(event server.ClientEvent) -// Middleware is able to process client auth events, exposes client control API. +// Middleware is able to subscribe to client status events, exposes client control API. // // The OpenVPN server should have been started with the // --management-client-auth directive so that it will ask the management @@ -45,9 +45,8 @@ type Middleware struct { // NewMiddleware creates new instance of Middleware. func NewMiddleware(listeners ...ClientEventCallback) *Middleware { return &Middleware{ - commandWriter: nil, - currentEvent: server.UndefinedEvent(), - listeners: listeners, + currentEvent: server.UndefinedEvent(), + listeners: listeners, } } @@ -59,31 +58,31 @@ func (m *Middleware) ClientsSubscribe(callback ClientEventCallback) { m.listeners = append(m.listeners, callback) } -// ClientAccept client control which allows authorisation (for CONNECT or REAUTH state). +// ClientAccept is a client control which allows authorization (for CONNECT or REAUTH state). func (m *Middleware) ClientAccept(clientID, keyID int) error { _, err := m.commandWriter.SingleLineCommand("client-auth-nt %d %d", clientID, keyID) return err } -// ClientDeny client control which forbids authorisation (for CONNECT or REAUTH state). +// ClientDeny is a client control which forbids authorization (for CONNECT or REAUTH state). func (m *Middleware) ClientDeny(clientID, keyID int, message string) error { _, err := m.commandWriter.SingleLineCommand("client-deny %d %d", clientID, keyID, message) return err } -// ClientDenyWithMessage client control which forbids authorisation with reason message (for CONNECT or REAUTH state). +// ClientDenyWithMessage is a client control which forbids authorization with reason message (for CONNECT or REAUTH state). func (m *Middleware) ClientDenyWithMessage(clientID, keyID int, message string) error { _, err := m.commandWriter.SingleLineCommand("client-deny %d %d %s", clientID, keyID, message) return err } -// ClientKill client control which stops established connection (for ESTABLISHED state). +// ClientKill is a client control which stops established connection (for ESTABLISHED state). func (m *Middleware) ClientKill(clientID int) error { _, err := m.commandWriter.SingleLineCommand("client-kill %d", clientID) return err } -// ClientKillWithMessage client control which stops established connection with reason message (for ESTABLISHED state). +// ClientKillWithMessage is a client control which stops established connection with reason message (for ESTABLISHED state). func (m *Middleware) ClientKillWithMessage(clientID int, message string) error { _, err := m.commandWriter.SingleLineCommand("client-kill %d %s", clientID, message) return err @@ -160,12 +159,9 @@ func (m *Middleware) endOfEvent() { m.listenersMu.RLock() defer m.listenersMu.RUnlock() - if m.listeners != nil { - for _, subscription := range m.listeners { - subscription(m.currentEvent) - } + for _, subscription := range m.listeners { + subscription(m.currentEvent) } - m.reset() } diff --git a/openvpn/middlewares/server/credentials/middleware.go b/openvpn/middlewares/server/credentials/middleware.go index 20a752c..c613803 100644 --- a/openvpn/middlewares/server/credentials/middleware.go +++ b/openvpn/middlewares/server/credentials/middleware.go @@ -23,7 +23,7 @@ import ( "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server/auth" ) -// Middleware is able to process authorize incoming clients by given credentials validator callback. +// Middleware is able to authorize incoming clients by given credentials validator callback. type Middleware struct { *auth.Middleware @@ -35,10 +35,9 @@ type Validator func(clientID int, username, password string) (bool, error) // NewMiddleware creates server user_auth challenge authentication Middleware func NewMiddleware(validator Validator) *Middleware { - m := &Middleware{ - validator: validator, - } + m := new(Middleware) m.Middleware = auth.NewMiddleware(m.handleClientEvent) + m.validator = validator return m } diff --git a/openvpn/middlewares/server/credentials/middleware_test.go b/openvpn/middlewares/server/credentials/middleware_test.go index 0a4a2e3..b38ee8c 100644 --- a/openvpn/middlewares/server/credentials/middleware_test.go +++ b/openvpn/middlewares/server/credentials/middleware_test.go @@ -22,7 +22,6 @@ import ( "github.com/mysteriumnetwork/go-openvpn/openvpn/management" "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server" - "github.com/mysteriumnetwork/go-openvpn/openvpn/middlewares/server/auth" "github.com/stretchr/testify/assert" ) @@ -35,12 +34,6 @@ func (f *fakeValidator) authenticateClient(clientID int, username, password stri return username == "username1" && password == "12341234", nil } -func mockAuthMiddleware(connection *management.MockConnection) *auth.Middleware { - m := auth.NewMiddleware() - m.Start(connection) - return m -} - func Test_Factory(t *testing.T) { fas := fakeValidator{} middleware := NewMiddleware(fas.authenticateClient) diff --git a/openvpn/middlewares/server/filter/middleware.go b/openvpn/middlewares/server/filter/middleware.go index bedf42d..b0fdd1c 100644 --- a/openvpn/middlewares/server/filter/middleware.go +++ b/openvpn/middlewares/server/filter/middleware.go @@ -58,12 +58,10 @@ type middleware struct { // NewMiddleware creates new instance of middleware func NewMiddleware(allow, block []string) *middleware { - m := &middleware{ - commandWriter: nil, - allow: allow, - block: block, - } + m := new(middleware) m.Middleware = auth.NewMiddleware(m.handleClientEvent) + m.allow = allow + m.block = block return m }