From cc1a524636900577c5e5caa75423011a9cabf045 Mon Sep 17 00:00:00 2001 From: Anil Ambati Date: Thu, 21 Sep 2017 11:19:32 -0400 Subject: [PATCH] [FAB-6247] Sanitize debug messages This is a back port of changeset https://gerrit.hyperledger.org/r/c/13659/ to v1.0.2 Sanitize debug messages to filter out any sensitive information and updated password test for DB/LDAP Change-Id: I7eefe85f3268c9b18922585dc09df5e3f41e5470 Signed-off-by: Anil Ambati --- lib/ca.go | 2 +- lib/ca_test.go | 34 +++++++++++++ lib/caconfig.go | 38 ++++++++++++++- lib/ldap/client.go | 25 ++++++++++ lib/ldap/client_test.go | 47 ++++++++++++++---- scripts/fvt/passwordsInLog_test.sh | 78 +++++++++++++++++++++++++++--- 6 files changed, 206 insertions(+), 18 deletions(-) diff --git a/lib/ca.go b/lib/ca.go index 633cab1ea..c03a72174 100644 --- a/lib/ca.go +++ b/lib/ca.go @@ -539,7 +539,7 @@ func (ca *CA) initDB() error { return err } } - log.Infof("Initialized %s database at %s", db.Type, db.Datasource) + log.Infof("Initialized %s database", db.Type) return nil } diff --git a/lib/ca_test.go b/lib/ca_test.go index 6f1b4b36d..334ae0eab 100644 --- a/lib/ca_test.go +++ b/lib/ca_test.go @@ -17,6 +17,7 @@ package lib import ( "crypto/x509" + "fmt" "io/ioutil" "testing" @@ -179,3 +180,36 @@ func testValidMatchingKeys(cert *x509.Certificate, t *testing.T) { t.Error("Should have failed, public key and private key do not match") } } + +// Tests String method of CAConfigDB +func TestCAConfigDBStringer(t *testing.T) { + dbconfig := CAConfigDB{ + Type: "postgres", + Datasource: "dbname=mypostgres host=127.0.0.1 port=8888 user=admin password=admin sslmode=disable", + } + str := fmt.Sprintf("%+v", dbconfig) // String method of CAConfigDB is called here + t.Logf("Stringified postgres CAConfigDB: %s", str) + assert.Contains(t, str, "user=****", "Username is not masked in the datasource URL") + assert.Contains(t, str, "password=****", "Password is not masked in the datasource URL") + + dbconfig.Datasource = "dbname=mypostgres host=127.0.0.1 port=8888 password=admin sslmode=disable user=admin" + str = fmt.Sprintf("%+v", dbconfig) // String method of CAConfigDB is called here + t.Logf("Stringified postgres CAConfigDB: %s", str) + assert.Contains(t, str, "user=****", "Username is not masked in the datasource URL") + assert.Contains(t, str, "password=****", "Password is not masked in the datasource URL") + + dbconfig.Datasource = "dbname=cadb password=adminpwd host=127.0.0.1 port=8888 user=cadb sslmode=disable" + str = fmt.Sprintf("%+v", dbconfig) // String method of CAConfigDB is called here + t.Logf("Stringified postgres CAConfigDB: %s", str) + assert.Contains(t, str, "user=****", "Username is not masked in the datasource URL") + assert.Contains(t, str, "password=****", "Password is not masked in the datasource URL") + + dbconfig = CAConfigDB{ + Type: "mysql", + Datasource: "root:rootpw@tcp(localhost:8888)/mysqldb?parseTime=true", + } + str = fmt.Sprintf("%+v", dbconfig) + t.Logf("Stringified mysql CAConfigDB: %s", str) + assert.NotContains(t, str, "root", "Username is not masked in the datasource URL") + assert.NotContains(t, str, "rootpw", "Password is not masked in the datasource URL") +} diff --git a/lib/caconfig.go b/lib/caconfig.go index 0f711bb9b..15e41ced7 100644 --- a/lib/caconfig.go +++ b/lib/caconfig.go @@ -17,6 +17,9 @@ limitations under the License. package lib import ( + "regexp" + "strings" + "github.com/cloudflare/cfssl/config" "github.com/hyperledger/fabric-ca/api" "github.com/hyperledger/fabric-ca/lib/ldap" @@ -60,6 +63,10 @@ csr: ` ) +var ( + dbURLRegex = regexp.MustCompile("Datasource:\\s*(\\S+):(\\S+)@|Datasource:.*\\s(user=\\S+).*\\s(password=\\S+)|Datasource:.*\\s(password=\\S+).*\\s(user=\\S+)") +) + // CAConfig is the CA instance's config // The tags are recognized by the RegisterFlags function in fabric-ca/lib/util.go // and are as follows: @@ -97,6 +104,35 @@ type CAConfigDB struct { TLS tls.ClientTLSConfig } +// Implements Stringer interface for CAConfigDB +// Calls util.StructToString to convert the CAConfigDB struct to +// string and masks the password from the database URL. Returns +// resulting string. +func (c CAConfigDB) String() string { + str := util.StructToString(&c) + matches := dbURLRegex.FindStringSubmatch(str) + + // If there is a match, there should be three entries: 1 for + // the match and 6 for submatches (see dbURLRegex regular expression) + if len(matches) == 7 { + matchIdxs := dbURLRegex.FindStringSubmatchIndex(str) + substr := str[matchIdxs[0]:matchIdxs[1]] + for idx := 1; idx < len(matches); idx++ { + if matches[idx] != "" { + if strings.Index(matches[idx], "user=") == 0 { + substr = strings.Replace(substr, matches[idx], "user=****", 1) + } else if strings.Index(matches[idx], "password=") == 0 { + substr = strings.Replace(substr, matches[idx], "password=****", 1) + } else { + substr = strings.Replace(substr, matches[idx], "****", 1) + } + } + } + str = str[:matchIdxs[0]] + substr + str[matchIdxs[1]:len(str)] + } + return str +} + // CAConfigRegistry is the registry part of the server's config type CAConfigRegistry struct { MaxEnrollments int `def:"-1" help:"Maximum number of enrollments; valid if LDAP not enabled"` @@ -105,7 +141,7 @@ type CAConfigRegistry struct { // CAConfigIdentity is identity information in the server's config type CAConfigIdentity struct { - Name string + Name string `secret:"password"` Pass string `secret:"password"` Type string Affiliation string diff --git a/lib/ldap/client.go b/lib/ldap/client.go index f7f10462f..0902ae57b 100644 --- a/lib/ldap/client.go +++ b/lib/ldap/client.go @@ -21,12 +21,14 @@ import ( "fmt" "net" "net/url" + "regexp" "strconv" "strings" "github.com/cloudflare/cfssl/log" "github.com/hyperledger/fabric-ca/lib/spi" ctls "github.com/hyperledger/fabric-ca/lib/tls" + "github.com/hyperledger/fabric-ca/util" "github.com/hyperledger/fabric/bccsp" ldap "gopkg.in/ldap.v2" ) @@ -34,6 +36,7 @@ import ( var ( dnAttr = []string{"dn"} errNotSupported = errors.New("Not supported") + ldapURLRegex = regexp.MustCompile("ldaps*://(\\S+):(\\S+)@") ) // Config is the configuration object for this LDAP client @@ -45,6 +48,28 @@ type Config struct { TLS ctls.ClientTLSConfig } +// Implements Stringer interface for ldap.Config +// Calls util.StructToString to convert the Config struct to +// string and masks the password from the ldap URL. Returns +// resulting string. +func (c Config) String() string { + str := util.StructToString(&c) + matches := ldapURLRegex.FindStringSubmatch(str) + // If there is a match, there should be two entries: 1 for + // the match and 2 for submatches + if len(matches) == 3 { + matchIdxs := ldapURLRegex.FindStringSubmatchIndex(str) + substr := str[matchIdxs[0]:matchIdxs[1]] + for idx := 1; idx < len(matches); idx++ { + if matches[idx] != "" { + substr = strings.Replace(substr, matches[idx], "****", 1) + } + } + str = str[:matchIdxs[0]] + substr + str[matchIdxs[1]:len(str)] + } + return str +} + // NewClient creates an LDAP client func NewClient(cfg *Config, csp bccsp.BCCSP) (*Client, error) { log.Debugf("Creating new LDAP client for %+v", cfg) diff --git a/lib/ldap/client_test.go b/lib/ldap/client_test.go index 3bf4e7dab..467a3d771 100644 --- a/lib/ldap/client_test.go +++ b/lib/ldap/client_test.go @@ -19,6 +19,8 @@ package ldap import ( "fmt" "testing" + + "github.com/stretchr/testify/assert" ) func TestLDAP(t *testing.T) { @@ -40,7 +42,7 @@ func testLDAP(proto string, port int, t *testing.T) { host := "localhost" base := "dc=example,dc=org" url := fmt.Sprintf("%s://%s:%s@%s:%d/%s", proto, dn, pwd, host, port, base) - c, err := NewClient(&Config{URL: url}) + c, err := NewClient(&Config{URL: url}, nil) if err != nil { t.Errorf("ldap.NewClient failure: %s", err) return @@ -50,7 +52,7 @@ func testLDAP(proto string, port int, t *testing.T) { t.Errorf("ldap.Client.GetUser failure: %s", err) return } - err = user.Login("jsmithpw") + err = user.Login("jsmithpw", -1) if err != nil { t.Errorf("ldap.User.Login failure: %s", err) } @@ -58,7 +60,7 @@ func testLDAP(proto string, port int, t *testing.T) { if path == nil { t.Error("ldap.User.GetAffiliationPath is nil") } - err = user.Login("bogus") + err = user.Login("bogus", -1) if err == nil { t.Errorf("ldap.User.Login passed but should have failed") } @@ -70,19 +72,19 @@ func testLDAP(proto string, port int, t *testing.T) { } func testLDAPNegative(t *testing.T) { - _, err := NewClient(nil) + _, err := NewClient(nil, nil) if err == nil { t.Errorf("ldap.NewClient(nil) passed but should have failed") } - _, err = NewClient(&Config{URL: "bogus"}) + _, err = NewClient(&Config{URL: "bogus"}, nil) if err == nil { t.Errorf("ldap.NewClient(bogus) passed but should have failed") } - _, err = NewClient(&Config{URL: "ldaps://localhost"}) + _, err = NewClient(&Config{URL: "ldaps://localhost"}, nil) if err != nil { t.Errorf("ldap.NewClient(ldaps) failed: %s", err) } - _, err = NewClient(&Config{URL: "ldap://localhost:badport"}) + _, err = NewClient(&Config{URL: "ldap://localhost:badport"}, nil) if err == nil { t.Errorf("ldap.NewClient(badport) passed but should have failed") } @@ -96,7 +98,7 @@ func TestLDAPTLS(t *testing.T) { base := "dc=example,dc=org" port := 10636 url := fmt.Sprintf("%s://%s:%s@%s:%d/%s", proto, dn, pwd, host, port, base) - c, err := NewClient(&Config{URL: url}) + c, err := NewClient(&Config{URL: url}, nil) if err != nil { t.Errorf("ldap.NewClient failure: %s", err) return @@ -109,7 +111,7 @@ func TestLDAPTLS(t *testing.T) { t.Errorf("ldap.Client.GetUser failure: %s", err) return } - err = user.Login("jsmithpw") + err = user.Login("jsmithpw", -1) if err != nil { t.Errorf("ldap.User.Login failure: %s", err) } @@ -117,7 +119,7 @@ func TestLDAPTLS(t *testing.T) { if path == nil { t.Error("ldap.User.GetAffiliationPath is nil") } - err = user.Login("bogus") + err = user.Login("bogus", -1) if err == nil { t.Errorf("ldap.User.Login passed but should have failed") } @@ -127,3 +129,28 @@ func TestLDAPTLS(t *testing.T) { } t.Logf("email for user 'jsmith' is %s", email) } + +// Tests String method of ldap.Config +func TestLDAPConfigStringer(t *testing.T) { + ldapConfig := Config{ + Enabled: true, + URL: "ldap://admin:adminpwd@localhost:8888/users", + UserFilter: "(uid=%s)", + GroupFilter: "(memberUid=%s)", + } + str := fmt.Sprintf("%+v", ldapConfig) // String method of Config is called here + t.Logf("Stringified LDAP Config: %s", str) + assert.NotContains(t, str, "admin", "Username is not masked in the ldap URL") + assert.NotContains(t, str, "adminpwd", "Password is not masked in the ldap URL") + + ldapConfig = Config{ + Enabled: true, + URL: "ldaps://admin:adminpwd@localhost:8888/users", + UserFilter: "(uid=%s)", + GroupFilter: "(memberUid=%s)", + } + str = fmt.Sprintf("%+v", ldapConfig) + t.Logf("Stringified LDAP Config: %s", str) + assert.NotContains(t, str, "admin", "Username is not masked in the ldap URL") + assert.NotContains(t, str, "adminpwd", "Password is not masked in the ldap URL") +} diff --git a/scripts/fvt/passwordsInLog_test.sh b/scripts/fvt/passwordsInLog_test.sh index 7e168ca65..687e0a47c 100755 --- a/scripts/fvt/passwordsInLog_test.sh +++ b/scripts/fvt/passwordsInLog_test.sh @@ -5,14 +5,80 @@ # SPDX-License-Identifier: Apache-2.0 # +function checkPasswd() { + local pswd="$1" + local Type="$2" + : ${Type:="user"} + + set -f + # Extract password value(s) from logfile + case "$Type" in + user) passwd=$(egrep -o "Pass:[^[:space:]]+" $LOGFILE| awk -F':' '{print $2}') ;; + ldap) passwd=$(egrep -io "ldap.*@" $LOGFILE| awk -v FS=[:@] '{print $(NF-1)}') ;; + mysql) passwd=$(egrep -o "[a-z0-9*]+@tcp" $LOGFILE| awk -v FS=@ '{print $(NF-1)}') ;; + postgres) passwd=$(egrep -o "password=[^ ]+ " $LOGFILE| awk -F '=' '{print $2}') ;; + esac + + # Fail if password is empty + if [[ -z "$passwd" ]] ; then + ErrorMsg "Unable to extract password value(s)" + fi + + # Fail if password matches anything other than '*' + for p in $passwd; do + if ! [[ "$p" =~ \*+ ]]; then + ErrorMsg "Passwords were not masked in the log" + fi + done + + # ensure any string passed in doesn't appear anywhere in logfile + if [[ -n "$pswd" ]]; then + grep "$pswd" "$LOGFILE" && ErrorMsg "$pswd was not masked in the log" + fi + set +f +} + +function passWordSub() { + sed -i "/datasource:/ s/\(password=\)[[:alnum:]]\+\(.*\)/\1$PSWD\2/ + s/dc=com:$LDAP_PASSWD/dc=com:$PSWD/ + s/datasource:\(.*\)mysql@/datasource:\1$PSWD@/" $TESTDIR/runFabricCaFvt.yaml +} + RC=0 +TESTCASE="passwordsInLog" +TESTDIR="/tmp/$TESTCASE" +mkdir -p $TESTDIR + FABRIC_CA="$GOPATH/src/github.com/hyperledger/fabric-ca" SCRIPTDIR="$FABRIC_CA/scripts/fvt" . $SCRIPTDIR/fabric-ca_utils -fabric-ca-server init -b administrator:administratorpw -d &> /tmp/log.txt -grep "administratorpw" /tmp/log.txt &> /dev/null -if [ $? == 0 ]; then - ErrorMsg "Passwords were not masked in the log" -fi + +export CA_CFG_PATH="$TESTDIR" +export FABRIC_CA_SERVER_HOME="$TESTDIR" +LOGFILE=$FABRIC_CA_SERVER_HOME/log.txt + +USER=administrator +PSWD=thisIs_aLongUniquePasswordWith_aMinisculePossibilityOfBeingDuplicated + +# Test using bootstrap ID +fabric-ca-server init -b $USER:$PSWD -d 2>&1 | tee $LOGFILE +test ${PIPESTATUS[0]} -eq 0 && checkPasswd "$PSWD" || ErrorMsg "Init of CA failed" + +# Test using multiple IDs from pre-supplied config file +$SCRIPTDIR/fabric-ca_setup.sh -R; mkdir -p $TESTDIR +$SCRIPTDIR/fabric-ca_setup.sh -I -X -n1 -D 2>&1 | tee $LOGFILE +test ${PIPESTATUS[0]} -eq 0 && checkPasswd "$PSWD" || ErrorMsg "Init of CA failed" + +for server in ldap mysql postgres; do + $SCRIPTDIR/fabric-ca_setup.sh -R; mkdir -p $TESTDIR + case $server in + ldap) $SCRIPTDIR/fabric-ca_setup.sh -a -I -D > $LOGFILE 2>&1 ;; + *) $SCRIPTDIR/fabric-ca_setup.sh -I -D -d $server 2>&1 > $LOGFILE ;; + esac + passWordSub + $SCRIPTDIR/fabric-ca_setup.sh -S >> $LOGFILE 2>&1 + checkPasswd "$PSWD" $server +done + CleanUp $RC -exit $RC \ No newline at end of file +exit $RC