From f7df4c8dbde1d242a2603b9af354ecd267eb536a Mon Sep 17 00:00:00 2001 From: Vixentael Date: Fri, 13 Jul 2018 18:27:11 +0300 Subject: [PATCH 1/4] fixing http handling --- cmd/acra-connector/acra-connector.go | 17 ++++++++++------- cmd/acra-reader/http-api/decryptor.go | 9 ++++----- configs/acra-reader.yaml | 2 +- network/utils.go | 6 ++++++ 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/cmd/acra-connector/acra-connector.go b/cmd/acra-connector/acra-connector.go index af66064c6..5ffcc9b8b 100644 --- a/cmd/acra-connector/acra-connector.go +++ b/cmd/acra-connector/acra-connector.go @@ -195,7 +195,7 @@ func main() { if *acraServerHost == "" && *acraServerConnectionString == "" { log.WithField(logging.FieldKeyEventCode, logging.EventCodeErrorWrongConfiguration). - Errorln("Configuration error: you must pass acraserver_connection_host or acra_connection_string parameter") + Errorln("Configuration error: you must pass acraserver_connection_host or acraserver_connection_string parameter") os.Exit(1) } if *acraServerHost != "" { @@ -240,11 +240,6 @@ func main() { os.Exit(1) } - if *verbose { - logging.SetLogLevel(logging.LOG_VERBOSE) - } else { - logging.SetLogLevel(logging.LOG_DISCARD) - } if runtime.GOOS != "linux" { *disableUserCheck = true } @@ -267,7 +262,7 @@ func main() { os.Exit(1) } - log.Debugf("Start listening connections") + log.Infof("Ready: start listening connections") config := &Config{KeyStore: keyStore, KeysDir: *keysDir, ClientId: []byte(*clientId), AcraServerConnectionString: *acraServerConnectionString, ConnectionString: *connectionString, AcraServerId: []byte(*acraServerId), disableUserCheck: *disableUserCheck} listener, err := network.Listen(*connectionString) if err != nil { @@ -346,6 +341,14 @@ func main() { } log.Infof("Start listening connection %s", *connectionString) + + if *verbose { + logging.SetLogLevel(logging.LOG_VERBOSE) + } else { + log.Infof("Disabling future logs.. Set -v to see logs") + logging.SetLogLevel(logging.LOG_DISCARD) + } + for { connection, err := listener.Accept() if err != nil { diff --git a/cmd/acra-reader/http-api/decryptor.go b/cmd/acra-reader/http-api/decryptor.go index 526c1d615..b208deda6 100644 --- a/cmd/acra-reader/http-api/decryptor.go +++ b/cmd/acra-reader/http-api/decryptor.go @@ -7,7 +7,6 @@ import ( "net/http" "net" "bytes" - "encoding/binary" "github.com/cossacklabs/acra/logging" "strings" "os" @@ -27,14 +26,13 @@ func NewHTTPConnectionsDecryptor(keystorage keystore.KeyStore) (*HTTPConnections func (decryptor *HTTPConnectionsDecryptor) SendResponseAndCloseConnection(logger *log.Entry, response *http.Response, connection net.Conn) { - buf := new(bytes.Buffer) - err := binary.Write(buf, binary.BigEndian, response) + r, err := ioutil.ReadAll(response.Body) if err != nil { logger.WithError(err).WithField(logging.FieldKeyEventCode, logging.EventCodeErrorReaderCantReturnResponse). - Warningln("Can't convert response to binary") + Warningln("Can't convert response to binary %s", response) } else { - _, err = connection.Write(buf.Bytes()) + _, err = connection.Write(r) if err != nil { logger.WithError(err).WithField(logging.FieldKeyEventCode, logging.EventCodeErrorReaderCantReturnResponse). Warningln("Can't write response to HTTP request") @@ -46,6 +44,7 @@ func (decryptor *HTTPConnectionsDecryptor) SendResponseAndCloseConnection(logger logger.WithError(err).WithField(logging.FieldKeyEventCode, logging.EventCodeErrorReaderCantCloseConnection). Warningln("Can't close connection of HTTP request") } + logger.Infoln("Closed connection") } diff --git a/configs/acra-reader.yaml b/configs/acra-reader.yaml index 8cd770ca0..06afcfd0e 100644 --- a/configs/acra-reader.yaml +++ b/configs/acra-reader.yaml @@ -14,7 +14,7 @@ incoming_connection_close_timeout: 10 incoming_connection_grpc_string: tcp://0.0.0.0:9696/ # Connection string for HTTP transport like http://x.x.x.x:yyyy -incoming_connection_http_string: tcp://0.0.0.0:9595/ +incoming_connection_http_string: http://0.0.0.0:9595/ # Folder from which will be loaded keys keys_dir: .acrakeys diff --git a/network/utils.go b/network/utils.go index 3fcb0ffa8..ad5d4b2e8 100644 --- a/network/utils.go +++ b/network/utils.go @@ -7,6 +7,8 @@ import ( url_ "net/url" "os" "strings" + "reflect" + log "github.com/sirupsen/logrus" ) const ( @@ -92,7 +94,11 @@ func GetConnectionDescriptor(connection net.Conn) (uintptr, error) { file, err = connection.(*net.TCPConn).File() case *net.UnixConn: file, err = connection.(*net.UnixConn).File() + case *secureSessionConnection: + // get decription of own connection + return GetConnectionDescriptor(connection.(*secureSessionConnection).Conn) default: + log.Errorf("Unsupported connection type: %s", reflect.TypeOf(connection)) return 0, ErrUnsupportedConnectionType } if err != nil { From 50468292896dd155e52a180dc389d1a52ac66ecc Mon Sep 17 00:00:00 2001 From: Vixentael Date: Fri, 13 Jul 2018 20:13:32 +0300 Subject: [PATCH 2/4] close connection, remove buffers for http response --- cmd/acra-reader/http-api/decryptor.go | 21 ++++++++++++--------- cmd/acra-reader/server.go | 10 +++++++++- network/utils.go | 2 +- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/cmd/acra-reader/http-api/decryptor.go b/cmd/acra-reader/http-api/decryptor.go index b208deda6..2ba97b2dd 100644 --- a/cmd/acra-reader/http-api/decryptor.go +++ b/cmd/acra-reader/http-api/decryptor.go @@ -14,6 +14,7 @@ import ( "github.com/cossacklabs/themis/gothemis/keys" log "github.com/sirupsen/logrus" "fmt" + "io" ) type HTTPConnectionsDecryptor struct { @@ -27,6 +28,8 @@ func NewHTTPConnectionsDecryptor(keystorage keystore.KeyStore) (*HTTPConnections func (decryptor *HTTPConnectionsDecryptor) SendResponseAndCloseConnection(logger *log.Entry, response *http.Response, connection net.Conn) { r, err := ioutil.ReadAll(response.Body) + io.Copy(ioutil.Discard, response.Body) + response.Body.Close() if err != nil { logger.WithError(err).WithField(logging.FieldKeyEventCode, logging.EventCodeErrorReaderCantReturnResponse). @@ -39,12 +42,12 @@ func (decryptor *HTTPConnectionsDecryptor) SendResponseAndCloseConnection(logger } } - err = connection.Close() - if err != nil { - logger.WithError(err).WithField(logging.FieldKeyEventCode, logging.EventCodeErrorReaderCantCloseConnection). - Warningln("Can't close connection of HTTP request") - } - logger.Infoln("Closed connection") + //err = connection.Close() + //if err != nil { + // logger.WithError(err).WithField(logging.FieldKeyEventCode, logging.EventCodeErrorReaderCantCloseConnection). + // Warningln("Can't close connection of HTTP request") + //} + //logger.Infoln("Closed connection") } @@ -123,7 +126,7 @@ func (decryptor *HTTPConnectionsDecryptor) ParseRequestPrepareResponse(logger *l response := emptyResponseWithStatus(request, http.StatusOK) response.Header.Set("Content-Type", "application/octet-stream") - response.Body = ioutil.NopCloser(bytes.NewBuffer(decryptedStruct)) + response.Body = ioutil.NopCloser(bytes.NewReader(decryptedStruct)) response.ContentLength = int64(len(decryptedStruct)) return response default: @@ -183,8 +186,8 @@ func emptyResponseWithStatus(request *http.Request, status int) *http.Response { func responseWithMessage(request *http.Request, status int, body string) *http.Response { response := emptyResponseWithStatus(request, status) response.Header.Set("Content-Type", "text/plain") - response.Body = ioutil.NopCloser(bytes.NewBufferString(body)) - response.ContentLength = int64(len(body)) + response.Body = ioutil.NopCloser(bytes.NewReader([]byte(body))) + response.ContentLength = int64(len([]byte(body))) return response } diff --git a/cmd/acra-reader/server.go b/cmd/acra-reader/server.go index e28652c55..228707682 100644 --- a/cmd/acra-reader/server.go +++ b/cmd/acra-reader/server.go @@ -107,6 +107,14 @@ func (server *ReaderServer) HandleConnectionString(parentContext context.Context logger.WithError(err).Errorln("can't add connection to connection manager") return } + defer func () { + logger.Debugln("Closing connection") + err := wrappedConnection.Close() + if err != nil { + logger.WithError(err).Errorln("Can't close wrapped connection") + } + logger.Infoln("Connection closed") + }() processingFunc(parentContext, clientId, wrappedConnection) if err := server.connectionManager.RemoveConnection(wrappedConnection); err != nil { logger.WithError(err).Errorln("can't remove connection from connection manager") @@ -119,7 +127,7 @@ func (server *ReaderServer) HandleConnectionString(parentContext context.Context case <-parentContext.Done(): log.WithError(parentContext.Err()).Debugln("Exit from handling connection string. Close all connections") case outErr = <-errCh: - log.WithError(err).Errorln("error on accepting new connections") + log.WithError(err).Errorln("Error on accepting new connections") } return outErr diff --git a/network/utils.go b/network/utils.go index ad5d4b2e8..ad7254167 100644 --- a/network/utils.go +++ b/network/utils.go @@ -95,7 +95,7 @@ func GetConnectionDescriptor(connection net.Conn) (uintptr, error) { case *net.UnixConn: file, err = connection.(*net.UnixConn).File() case *secureSessionConnection: - // get decription of own connection + // get description of own connection return GetConnectionDescriptor(connection.(*secureSessionConnection).Conn) default: log.Errorf("Unsupported connection type: %s", reflect.TypeOf(connection)) From f2fe133413c950c4f1e86283cbd92b42f14d5beb Mon Sep 17 00:00:00 2001 From: Vixentael Date: Mon, 16 Jul 2018 14:18:41 +0300 Subject: [PATCH 3/4] add more logs to decryptor --- cmd/acra-reader/http-api/decryptor.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/acra-reader/http-api/decryptor.go b/cmd/acra-reader/http-api/decryptor.go index 3b1ab438d..4af649eda 100644 --- a/cmd/acra-reader/http-api/decryptor.go +++ b/cmd/acra-reader/http-api/decryptor.go @@ -7,7 +7,6 @@ import ( "github.com/cossacklabs/themis/gothemis/keys" log "github.com/sirupsen/logrus" "fmt" - "io" "net/http" "strings" "os" @@ -27,12 +26,15 @@ func NewHTTPConnectionsDecryptor(keystorage keystore.KeyStore) (*HTTPConnections func (decryptor *HTTPConnectionsDecryptor) SendResponse(logger *log.Entry, response *http.Response, connection net.Conn) { r, err := ioutil.ReadAll(response.Body) - io.Copy(ioutil.Discard, response.Body) - response.Body.Close() + if err != nil { + logger.WithError(err).WithField(logging.FieldKeyEventCode, logging.EventCodeErrorReaderCantReturnResponse). + Warningln("Can't read response body") + } + err = response.Body.Close() if err != nil { logger.WithError(err).WithField(logging.FieldKeyEventCode, logging.EventCodeErrorReaderCantReturnResponse). - Warningln("Can't convert response to binary %s", response) + Warningln("Can't convert response to binary") } else { _, err = connection.Write(r) if err != nil { From 8da3c0a49c38370e4ba64259126788b965737d3a Mon Sep 17 00:00:00 2001 From: Vixentael Date: Mon, 16 Jul 2018 15:58:27 +0300 Subject: [PATCH 4/4] move defer to the top --- cmd/acra-reader/server.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cmd/acra-reader/server.go b/cmd/acra-reader/server.go index b1bbfb6bb..69b22a80b 100644 --- a/cmd/acra-reader/server.go +++ b/cmd/acra-reader/server.go @@ -112,10 +112,6 @@ func (server *ReaderServer) HandleConnectionString(parentContext context.Context logging.SetLoggerToContext(parentContext, logger) go func() { - if err := server.connectionManager.AddConnection(wrappedConnection); err != nil { - logger.WithError(err).Errorln("Can't add connection to connection manager") - return - } defer func () { err := wrappedConnection.Close() if err != nil { @@ -124,6 +120,11 @@ func (server *ReaderServer) HandleConnectionString(parentContext context.Context logger.Infoln("Connection closed") }() + if err := server.connectionManager.AddConnection(wrappedConnection); err != nil { + logger.WithError(err).Errorln("Can't add connection to connection manager") + return + } + processingFunc(parentContext, clientId, wrappedConnection) if err := server.connectionManager.RemoveConnection(wrappedConnection); err != nil {