feat(enterprise): audit logs for alpha and zero #7295

aman-bansal · 2021-01-14T14:09:03Z

This PR enables the enterprise feature to enable audit logs. In a nutshell, we are auditing requests over the network. To start the cluster, all you need to do is dgraph alpha --audit "dir=/audit_dir". Audit logs have the flexibility to be compressed and encrypted. Dgraph audit tool was provided to decrypt old encrypted files.

The sample audit log will look like

{"level":"info","ts":1613635215.456009,"msg":"/removeNode","user":"","namespace":0,"server":"zero1:5080","client":"172.21.0.1:43100","req_type":"Http","req_body":"","query_param":{"group":["1"],"id":["3"]},"status":"OK"}

This change is

manishrjain

Think we should clean up how we do handling, particularly the GraphQL ones. We need a better way to do middleware. And then do this PR on top.

Reviewable status: 0 of 20 files reviewed, 6 unresolved discussions (waiting on @aman-bansal, @martinmr, @pawanrawal, and @vvbalaji-dgraph)

ee/audit/audit.go, line 1 at r1 (raw file):

package audit

license. Also add the build flags.

ee/audit/interceptor.go, line 1 at r1 (raw file):

package audit

add the license. and build flags.

ee/audit/interceptor.go, line 26 at r1 (raw file):

		skipApis := []string{"Heartbeat", "RaftMessage", "JoinCluster", "IsPeer", // raft server
			"StreamMembership", "UpdateMembership", "Oracle", // zero server
			"Check", "Watch", // health server

Make this a map.

ee/audit/interceptor.go, line 30 at r1 (raw file):

		for _, api := range skipApis {
			if strings.HasSuffix(method, api) {

Just lookup in map. Don't loop over it.

ee/audit/interceptor.go, line 87 at r1 (raw file):

		Endpoint:   info.FullMethod,
		ReqType:    Grpc,
		Req:        fmt.Sprintf("%v", req),

they have a text marshaller, so perhaps try with that.

x/jwt_helper.go, line 1 at r1 (raw file):

package x

license.

… logs

manishrjain

Get approval from @jarifibrahim as well.

Reviewed 1 of 20 files at r1, 32 of 32 files at r2.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @aman-bansal, @martinmr, @pawanrawal, and @vvbalaji-dgraph)

dgraph/cmd/alpha/run.go, line 201 at r2 (raw file):

	compress=true/false to enabled the compression of old audit logs (default behaviour is false).
	encrypt_file=enc/key/file enables the audit log encryption with the key path provided with the
	flag.

what's the extra dot here? Is that a space.

ee/audit/audit_ee.go, line 148 at r2 (raw file):

			if atomic.LoadUint32(&auditEnabled) != 1 {
				if auditor.log, err = x.InitLogger(conf.Dir, defaultAuditFilename, conf.EncryptBytes,

100 chars.

ee/audit/interceptor_ee.go, line 40 at r2 (raw file):

	info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
	skip := func(method string) bool {
		skipApis := map[string]bool{

this can be out as a var. Not sure how smart Go compiler is about this -- it should only init this once. Putting it in a var is easier on compiler.

ee/audit/interceptor_ee.go, line 70 at r2 (raw file):

	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		skip := func(method string) bool {
			skipEPs := map[string]bool{

same here.

ee/audit/run_ee.go, line 88 at r2 (raw file):

	defer file.Close()

	outfile, err := os.OpenFile(decryptCmd.Conf.GetString("out"), os.O_CREATE|os.O_WRONLY|os.O_TRUNC,

100 chars.

worker/config.go, line 105 at r2 (raw file):

		ad, err := filepath.Abs(opt.Audit.Dir)
		x.Check(err)
		x.AssertTruef(ad != pd, "Posting and Audit Directory cannot be the same ('%s').", opt.Audit.Dir)

100 chars.

x/flags.go, line 116 at r2 (raw file):

	return sf
}
func (sf *SuperFlag) Get(opt string) string {

GetString

x/flags.go, line 149 at r2 (raw file):

	return uint32(u)
}
func (sf *SuperFlag) GetString(opt string) string {

No need for this.

x/jwt_helper.go, line 2 at r2 (raw file):

/*
 * Copyright 2017-2021 Dgraph Labs, Inc. and Contributors

If this is code move, then OK with the years. Otherwise, if this is new code, then 2021.

jarifibrahim

Got some comments

Reviewed 17 of 17 files at r3.
Reviewable status: all files reviewed, 21 unresolved discussions (waiting on @aman-bansal, @martinmr, @pawanrawal, and @vvbalaji-dgraph)

dgraph/cmd/alpha/run.go, line 607 at r3 (raw file):

	ctype, clevel := x.ParseCompression(Alpha.Conf.GetString("badger.compression"))

	conf, err := audit.GetAuditConf(Alpha.Conf.GetString("audit"))

You can do the x.Check in the GetAuditConf function. We're crashing it anyway.

dgraph/cmd/alpha/run.go, line 727 at r3 (raw file):

	// Audit is enterprise feature.
	x.Check(audit.InitAuditorIfNecessary(opts.Audit, worker.EnterpriseEnabled))

Check the error inside the function. Don't need to return it.

dgraph/cmd/alpha/run.go, line 822 at r3 (raw file):

	glog.Infoln("adminCloser closed.")

	audit.Close()

This will stop the ticker but the trackIfEEValid goroutine will not stop

dgraph/cmd/zero/run.go, line 240 at r3 (raw file):

		ad, err := filepath.Abs(opts.audit.Dir)
		x.Check(err)
		x.AssertTruef(ad != wd, "WAL and Audit directory cannot be the same ('%s').", opts.audit.Dir)

100 chars

dgraph/cmd/zero/run.go, line 240 at r3 (raw file):

		ad, err := filepath.Abs(opts.audit.Dir)
		x.Check(err)
		x.AssertTruef(ad != wd, "WAL and Audit directory cannot be the same ('%s').", opts.audit.Dir)

Why don't we need this check in alpha?

ee/audit/audit_ee.go, line 27 at r3 (raw file):

const (
	defaultAuditConf     = "dir=;compress=false;encrypt-file="

Add space after each option.

ee/audit/interceptor.go, line 87 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

they have a text marshaller, so perhaps try with that.

@aman-bansal is this still valid?

ee/audit/run_ee.go, line 79 at r3 (raw file):

	x.Check(err)
	if key == nil {
		return errors.New("no encryption key provided")

Do you always need an encryption key?

x/flags.go, line 149 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

No need for this.

@aman-bansal do we need this?

aman-bansal

Reviewable status: 30 of 36 files reviewed, 21 unresolved discussions (waiting on @jarifibrahim, @manishrjain, @martinmr, @pawanrawal, and @vvbalaji-dgraph)

dgraph/cmd/alpha/run.go, line 201 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

what's the extra dot here? Is that a space.

Done.

dgraph/cmd/alpha/run.go, line 607 at r3 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

You can do the x.Check in the GetAuditConf function. We're crashing it anyway.

Done.

dgraph/cmd/alpha/run.go, line 727 at r3 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

Check the error inside the function. Don't need to return it.

Done.

dgraph/cmd/alpha/run.go, line 822 at r3 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

This will stop the ticker but the trackIfEEValid goroutine will not stop

Done.

dgraph/cmd/zero/run.go, line 240 at r3 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

Why don't we need this check in alpha?

alpha checks are in worker/config.go file.

dgraph/cmd/zero/run.go, line 240 at r3 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

100 chars

Done.

ee/audit/audit.go, line 1 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

license. Also add the build flags.

Done.

ee/audit/audit_ee.go, line 148 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

100 chars.

Done.

ee/audit/audit_ee.go, line 27 at r3 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

Add space after each option.

though not necessary. for better clarity i have fixed this

ee/audit/interceptor.go, line 26 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Make this a map.

Done.

ee/audit/interceptor.go, line 30 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Just lookup in map. Don't loop over it.

Done.

ee/audit/interceptor.go, line 87 at r1 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

@aman-bansal is this still valid?

No, we can use text marshaller because for text marshaller to work we need to know the type of interface. If works over proto.Message objects only.

ee/audit/interceptor_ee.go, line 40 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

this can be out as a var. Not sure how smart Go compiler is about this -- it should only init this once. Putting it in a var is easier on compiler.

Done.

ee/audit/interceptor_ee.go, line 70 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

same here.

Done.

ee/audit/run_ee.go, line 88 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

100 chars.

Done.

ee/audit/run_ee.go, line 79 at r3 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

Do you always need an encryption key?

yes. No benefit of this tool, if not for decrypting audit logs.

worker/config.go, line 105 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

100 chars.

Done.

x/flags.go, line 116 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

GetString

Done.

x/flags.go, line 149 at r2 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

@aman-bansal do we need this?

yes we have refactor Get() method to GetString() method

x/jwt_helper.go, line 1 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

license.

Done.

x/jwt_helper.go, line 2 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

If this is code move, then OK with the years. Otherwise, if this is new code, then 2021.

Done.

aman-bansal requested review from manishrjain, martinmr, pawanrawal and vvbalaji-dgraph as code owners January 14, 2021 14:09

github-actions bot added area/enterprise Related to proprietary features area/graphql Issues related to GraphQL support on Dgraph. area/live-loader Issues related to live loading. labels Jan 14, 2021

manishrjain suggested changes Jan 15, 2021

View reviewed changes

aman-bansal added 17 commits January 19, 2021 10:53

starting work on audit logs

6c8d67d

making audit logs to work with alpha

c77f3d0

making audit logs to work with zero too

6ba4f3b

go mod tidy

8f4b3bc

fixing minor things to clean

33aff9c

adding endpoint to grpc audit

d78651f

fixing alpha and zero default directories

8f33f61

fixing alpha and zero default directories

eb2b7c7

adding skip method for grpc audits

7515a1b

renaming rw

6e3d770

refactoring basic things

6706ac5

audit fix

72db976

making zero and alpha logs from the start itself.

97729e5

fixing zero init audit process

9b5c43e

adding licence file + build tags

d7adbf8

adding dgraph audit tool and logs encryption

0710595

adding logwriter to handle encryption and everything

40e0194

aman-bansal force-pushed the aman/audit_logs branch from f230d5a to 40e0194 Compare January 19, 2021 15:39

aman-bansal added 4 commits January 19, 2021 23:05

adding test cases to check requests are getting logged into the audit…

cdd76e8

… logs

adding interceptor ee version

87bea22

basic refactoring and log message truncate functionality

3248e2a

fixing auditing request handler

2ffcbfe

aman-bansal added 6 commits January 20, 2021 11:29

refactoring alpha and audit_ee

512f291

refactoring some error handling

6336d09

making audit conf more reliable

5b7c9f4

fixing goimports

7cd693d

Merge branch 'master' into aman/audit_logs

6373287

fixing merge conflicts

642c296

manishrjain approved these changes Jan 21, 2021

View reviewed changes

aman-bansal added 2 commits January 24, 2021 19:48

making log writer performant using buffered writer

d97d78d

fixing zero test case and comments

05ea358

jarifibrahim approved these changes Jan 25, 2021

View reviewed changes

gracefully closing all the go routines

d01d743

aman-bansal commented Jan 26, 2021

View reviewed changes

aman-bansal added 3 commits January 26, 2021 13:56

Merge branch 'master' into aman/audit_logs

2fa132d

fixing oss build

2498f6d

fixing failed test case

e35918c

aman-bansal merged commit be9ce74 into master Jan 26, 2021

aman-bansal deleted the aman/audit_logs branch January 26, 2021 13:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(enterprise): audit logs for alpha and zero #7295

feat(enterprise): audit logs for alpha and zero #7295

aman-bansal commented Jan 14, 2021 •

edited

Loading

manishrjain left a comment

manishrjain left a comment

jarifibrahim left a comment

aman-bansal left a comment

feat(enterprise): audit logs for alpha and zero #7295

feat(enterprise): audit logs for alpha and zero #7295

Conversation

aman-bansal commented Jan 14, 2021 • edited Loading

manishrjain left a comment

Choose a reason for hiding this comment

manishrjain left a comment

Choose a reason for hiding this comment

jarifibrahim left a comment

Choose a reason for hiding this comment

aman-bansal left a comment

Choose a reason for hiding this comment

aman-bansal commented Jan 14, 2021 •

edited

Loading