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

fix remove LDAPPassword from audit logs #9773

Merged
merged 1 commit into from Jun 5, 2020

Conversation

harshavardhana
Copy link
Member

@harshavardhana harshavardhana commented Jun 5, 2020

Description

fix remove LDAPPassword from audit logs

Motivation and Context

The previous fix for #9707 was not correct,
fix this properly passing the right
filter keys to be filtered from the audit
log output.

How to test this PR?

Run the webhook.go

package main

import (
	"fmt"
	"io/ioutil"
	"log"
	"net/http"
)

func hello(w http.ResponseWriter, r *http.Request) {
	if r.URL.Path != "/" {
		http.Error(w, "404 not found.", http.StatusNotFound)
		return
	}

	fmt.Println("Authorization header", r.Header.Get("Authorization"))
	switch r.Method {
	case "POST":
		body, err := ioutil.ReadAll(r.Body)
		if err != nil {
			fmt.Fprintf(w, "ReadError() err: %v", err)
			return
		}
		fmt.Println(string(body))
	default:
		fmt.Fprintf(w, "Sorry, only POST methods are supported.")
	}
}

func main() {
	http.HandleFunc("/", hello)

	if err := http.ListenAndServe(":9056", nil); err != nil {
		log.Fatal(err)
	}
}

Configure the server to send audit logs to this webhook server

~ MINIO_AUDIT_WEBHOOK_ENABLE=on MINIO_AUDIT_WEBHOOK_ENDPOINT=http://localhost:9056 minio server ~/test

And send curl request

~ curl --data-urlencode "" "http://localhost:9000/?Action=AssumeRoleWithLDAPIdentity&LDAPUsername=foouser&LDAPPassword=foouserpassword&Version=2011-06-15" 

Observe on the webhook console that LDAPPassword shouldn't be displayed

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Documentation needed
  • Unit tests needed
  • Functional tests needed (If yes, add mint PR # here: )

the previous fix for minio#9707 was not correct,
fix this properly passing the right filter
keys to be filtered from the audit
log output.

Fixes minio#9767
Copy link
Contributor

@poornas poornas left a comment

Choose a reason for hiding this comment

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

tested,LGTM

Copy link
Contributor

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

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

LGTM

@harshavardhana harshavardhana merged commit d93bdea into minio:master Jun 5, 2020
@harshavardhana harshavardhana deleted the remove-ldap branch June 5, 2020 05:08
@timhughes
Copy link
Contributor

I can confirm that this is working and is no longer sending the users password to the audit webhook.

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants