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.

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

T864 - Separate log file for unparsed queries #295

Merged
merged 16 commits into from
Dec 27, 2018
38 changes: 22 additions & 16 deletions acra-censor/acra-censor_configuration_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ limitations under the License.
package acracensor

import (
"github.com/cossacklabs/acra/acra-censor/common"
"github.com/cossacklabs/acra/acra-censor/handlers"
"gopkg.in/yaml.v2"
"strings"
)

// Query handlers' names.
const (
BlacklistConfigStr = "blacklist"
WhitelistConfigStr = "whitelist"
QueryCaptureConfigStr = "query_capture"
QueryIgnoreConfigStr = "query_ignore"
BlacklistConfigStr = "blacklist"
WhitelistConfigStr = "whitelist"
QueryIgnoreConfigStr = "query_ignore"
)

// Config shows handlers configuration: queries, tables, patterns
Expand All @@ -37,9 +37,10 @@ type Config struct {
Queries []string
Tables []string
Patterns []string
Filepath string
}
IgnoreParseError bool `yaml:"ignore_parse_error"`
IgnoreParseError bool `yaml:"ignore_parse_error"`
ParseErrorsLog string `yaml:"parse_errors_log"`
CaptureLog string `yaml:"capture_log"`
}

// LoadConfiguration loads configuration of AcraCensor
Expand All @@ -50,6 +51,21 @@ func (acraCensor *AcraCensor) LoadConfiguration(configuration []byte) error {
return err
}
acraCensor.ignoreParseError = censorConfiguration.IgnoreParseError
if !strings.EqualFold(censorConfiguration.CaptureLog, "") {
queryWriter, err := common.NewFileQueryWriter(censorConfiguration.CaptureLog)
if err != nil {
return err
}
acraCensor.parsedQueriesWriter = queryWriter
}
if !strings.EqualFold(censorConfiguration.ParseErrorsLog, "") {
queryWriter, err := common.NewFileQueryWriter(censorConfiguration.ParseErrorsLog)
if err != nil {
return err
}
acraCensor.unparsedQueriesWriter = queryWriter
}

for _, handlerConfiguration := range censorConfiguration.Handlers {
switch handlerConfiguration.Handler {
case WhitelistConfigStr:
Expand Down Expand Up @@ -78,16 +94,6 @@ func (acraCensor *AcraCensor) LoadConfiguration(configuration []byte) error {
}
acraCensor.AddHandler(blacklistHandler)
break
case QueryCaptureConfigStr:
if strings.EqualFold(handlerConfiguration.Filepath, "") {
break
}
queryCaptureHandler, err := handlers.NewQueryCaptureHandler(handlerConfiguration.Filepath)
if err != nil {
return err
}
acraCensor.AddHandler(queryCaptureHandler)
break
case QueryIgnoreConfigStr:
queryIgnoreHandler := handlers.NewQueryIgnoreHandler()
queryIgnoreHandler.AddQueries(handlerConfiguration.Queries)
Expand Down
75 changes: 52 additions & 23 deletions acra-censor/acra-censor_implementation.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,30 @@ package acracensor

import (
"github.com/cossacklabs/acra/acra-censor/common"
"github.com/cossacklabs/acra/acra-censor/handlers"
log "github.com/sirupsen/logrus"
"strings"
"time"
)

// ServiceName to use in logs
const ServiceName = "acra-censor"

// AcraCensor describes censor data: query handler, logger and reaction on parsing errors.
type AcraCensor struct {
handlers []QueryHandlerInterface
ignoreParseError bool
logger *log.Entry
handlers []QueryHandlerInterface
ignoreParseError bool
parsedQueriesWriter *common.QueryWriter
unparsedQueriesWriter *common.QueryWriter
logger *log.Entry
}

// NewAcraCensor creates new censor object.
func NewAcraCensor() *AcraCensor {
acraCensor := &AcraCensor{}
acraCensor.logger = log.WithField("service", ServiceName)
acraCensor.ignoreParseError = false
acraCensor.parsedQueriesWriter = nil
acraCensor.unparsedQueriesWriter = nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can skip it because its already nil

return acraCensor
}

Expand Down Expand Up @@ -65,37 +70,25 @@ func (acraCensor *AcraCensor) ReleaseAll() {

// HandleQuery processes every query through each handler.
func (acraCensor *AcraCensor) HandleQuery(rawQuery string) error {
if len(acraCensor.handlers) == 0 {
if len(acraCensor.handlers) == 0 && acraCensor.parsedQueriesWriter == nil && acraCensor.unparsedQueriesWriter == nil {
// no handlers, AcraCensor won't work
return nil
}
normalizedQuery, queryWithHiddenValues, parsedQuery, err := common.HandleRawSQLQuery(rawQuery)
// Unparsed query handling
if err == common.ErrQuerySyntaxError {
acraCensor.logger.WithError(err).Warning("Failed to parse input query")
acraCensor.saveUnparsedQuery(rawQuery)
if acraCensor.ignoreParseError {
acraCensor.logger.Infof("Unparsed query has been allowed")
acraCensor.logger.Infoln("Unparsed query has been allowed")
return nil
}
acraCensor.logger.Errorf("Unparsed query has been forbidden")
acraCensor.logger.Errorln("Unparsed query has been forbidden")
return err
}

// Parsed query handling
acraCensor.saveParsedQuery(queryWithHiddenValues)
for _, handler := range acraCensor.handlers {
// in QueryCapture Handler we use only redacted queries
if queryCaptureHandler, ok := handler.(*handlers.QueryCaptureHandler); ok {
queryCaptureHandler.CheckQuery(queryWithHiddenValues, parsedQuery)
continue
}
// in QueryIgnore Handler we use only raw queries
if queryIgnoreHandler, ok := handler.(*handlers.QueryIgnoreHandler); ok {
continueHandling, _ := queryIgnoreHandler.CheckQuery(rawQuery, parsedQuery)
if continueHandling {
continue
} else {
break
}
}
// remained handlers operate
continueHandling, err := handler.CheckQuery(normalizedQuery, parsedQuery)
if err != nil {
acraCensor.logger.Errorf("Forbidden query: '%s'", queryWithHiddenValues)
Expand All @@ -110,3 +103,39 @@ func (acraCensor *AcraCensor) HandleQuery(rawQuery string) error {
acraCensor.logger.Infof("Allowed query: '%s'", queryWithHiddenValues)
return nil
}

// GetLoggingTimeout returns current timeout of censor's logging process
func (acraCensor *AcraCensor) GetLoggingTimeout() time.Duration {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we doesn't need such methods because they will not called anywhere and logging timeout we set once at first censor configurations with some constant and it will not changed during acra-server run. so I think we don't need extend AcraCensorInterface. anyway it's specific settings for handler, not censor at all.

return acraCensor.parsedQueriesWriter.GetSerializationTimeout()
}

// SetLoggingTimeout sets timeout of censor's logging process
func (acraCensor *AcraCensor) SetLoggingTimeout(duration time.Duration) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

save as above

acraCensor.parsedQueriesWriter.SetSerializationTimeout(duration)
acraCensor.unparsedQueriesWriter.SetSerializationTimeout(duration)
}

func (acraCensor *AcraCensor) saveUnparsedQuery(query string) {
if acraCensor.unparsedQueriesWriter != nil {
saveQuery(acraCensor.unparsedQueriesWriter, query)
Copy link
Collaborator

Choose a reason for hiding this comment

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

imho better to use unparsedQueriesWriter.WriteQuery(query) or move saveQuery to file with this handler

}
}

func (acraCensor *AcraCensor) saveParsedQuery(query string) {
if acraCensor.parsedQueriesWriter != nil {
saveQuery(acraCensor.parsedQueriesWriter, query)
}
}

func saveQuery(writer *common.QueryWriter, query string) {
//skip already captured queries
for _, capturedQuery := range writer.Queries {
Copy link
Collaborator

Choose a reason for hiding this comment

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

it is logic of QueryWriter, not censor. better to move this logic to query writer and add some method like handler.WriteQuery(query string) where it will check is this query exists or not and create new QueryInfo and append or not

after moving we will not need writer.Queries as public field and change to private

if strings.EqualFold(capturedQuery.RawQuery, query) {
return
}
}
queryInfo := &common.QueryInfo{}
queryInfo.RawQuery = query
queryInfo.IsForbidden = false
writer.Queries = append(writer.Queries, queryInfo)
}
7 changes: 6 additions & 1 deletion acra-censor/acra-censor_interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ limitations under the License.
// https://github.com/cossacklabs/acra/wiki/AcraCensor
package acracensor

import "github.com/cossacklabs/acra/sqlparser"
import (
"github.com/cossacklabs/acra/sqlparser"
"time"
)

// QueryHandlerInterface describes what actions are available for queries.
type QueryHandlerInterface interface {
Expand All @@ -36,4 +39,6 @@ type AcraCensorInterface interface {
AddHandler(handler QueryHandlerInterface)
RemoveHandler(handler QueryHandlerInterface)
ReleaseAll()
GetLoggingTimeout() time.Duration
Copy link
Collaborator

Choose a reason for hiding this comment

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

imho we don't need it

SetLoggingTimeout(duration time.Duration)
}
Loading