From a4e900148a4b0346d2f4705f865a653beecdf8ea Mon Sep 17 00:00:00 2001 From: Pantelis Karamolegkos Date: Wed, 16 Feb 2022 23:07:26 +0200 Subject: [PATCH 1/6] Hashing web credentials --- server/middleware.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/middleware.go b/server/middleware.go index 5c3ffa4b6f..7195a10979 100644 --- a/server/middleware.go +++ b/server/middleware.go @@ -14,6 +14,7 @@ package server import ( + "crypto/md5" "net/http" "github.com/runatlantis/atlantis/server/logging" @@ -52,13 +53,13 @@ func (l *RequestLogger) ServeHTTP(rw http.ResponseWriter, r *http.Request, next user, pass, ok := r.BasicAuth() if ok { r.SetBasicAuth(user, pass) - l.logger.Debug("user: %s / pass: %s >> url: %s", user, pass, r.URL.RequestURI()) + l.logger.Debug("user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user), md5.Sum([]byte(pass), r.URL.RequestURI()) if user == l.WebUsername && pass == l.WebPassword { - l.logger.Debug("[VALID] user: %s / pass: %s >> url: %s", user, pass, r.URL.RequestURI()) + l.logger.Debug("[VALID] user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user), md5.Sum([]byte(pass), r.URL.RequestURI()) allowed = true } else { allowed = false - l.logger.Info("[INVALID] user: %s / pass: %s >> url: %s", user, pass, r.URL.RequestURI()) + l.logger.Info("[INVALID] user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user), md5.Sum([]byte(pass), r.URL.RequestURI()) } } } From 11261017d37b04ec5a48c95c0e2272ab582c1c23 Mon Sep 17 00:00:00 2001 From: Pantelis Karamolegkos Date: Wed, 16 Feb 2022 23:26:10 +0200 Subject: [PATCH 2/6] Fix syntactic error --- server/middleware.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/server/middleware.go b/server/middleware.go index 7195a10979..e9e7d3b767 100644 --- a/server/middleware.go +++ b/server/middleware.go @@ -53,13 +53,16 @@ func (l *RequestLogger) ServeHTTP(rw http.ResponseWriter, r *http.Request, next user, pass, ok := r.BasicAuth() if ok { r.SetBasicAuth(user, pass) - l.logger.Debug("user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user), md5.Sum([]byte(pass), r.URL.RequestURI()) + l.logger.Debug("user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user)), + md5.Sum([]byte(pass)), r.URL.RequestURI()) if user == l.WebUsername && pass == l.WebPassword { - l.logger.Debug("[VALID] user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user), md5.Sum([]byte(pass), r.URL.RequestURI()) + l.logger.Debug("[VALID] user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user)), + md5.Sum([]byte(pass)), r.URL.RequestURI()) allowed = true } else { allowed = false - l.logger.Info("[INVALID] user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user), md5.Sum([]byte(pass), r.URL.RequestURI()) + l.logger.Info("[INVALID] user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user)), + md5.Sum([]byte(pass)), r.URL.RequestURI()) } } } From 80ef892135768189fae536c6145dc2b109012631 Mon Sep 17 00:00:00 2001 From: Pantelis Karamolegkos Date: Wed, 16 Feb 2022 23:50:29 +0200 Subject: [PATCH 3/6] Using strong cryptogrpahic primitive --- server/middleware.go | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/server/middleware.go b/server/middleware.go index e9e7d3b767..1478f7c6ac 100644 --- a/server/middleware.go +++ b/server/middleware.go @@ -14,11 +14,11 @@ package server import ( - "crypto/md5" "net/http" "github.com/runatlantis/atlantis/server/logging" "github.com/urfave/negroni" + "golang.org/x/crypto/bcrypt" ) // NewRequestLogger creates a RequestLogger. @@ -31,6 +31,8 @@ func NewRequestLogger(s *Server) *RequestLogger { } } +const redacted = "[REDACTED]" + // RequestLogger logs requests and their response codes. // as well as handle the basicauth on the requests type RequestLogger struct { @@ -43,6 +45,10 @@ type RequestLogger struct { // ServeHTTP implements the middleware function. It logs all requests at DEBUG level. func (l *RequestLogger) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) { l.logger.Debug("%s %s – from %s", r.Method, r.URL.RequestURI(), r.RemoteAddr) + var ( + hashUser, hashPass []byte + err error + ) allowed := false if !l.WebAuthentication || r.URL.Path == "/events" || @@ -51,18 +57,25 @@ func (l *RequestLogger) ServeHTTP(rw http.ResponseWriter, r *http.Request, next allowed = true } else { user, pass, ok := r.BasicAuth() + hashUser, err = hashData([]byte(user)) + if err != nil { + l.logger.Debug("unable to hash username, defaulting to %s", redacted) + hashUser = []byte(redacted) + } + hashPass, err = hashData([]byte(pass)) + if err != nil { + l.logger.Debug("unable to hash password, defaulting to %s", redacted) + hashPass = []byte(redacted) + } if ok { r.SetBasicAuth(user, pass) - l.logger.Debug("user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user)), - md5.Sum([]byte(pass)), r.URL.RequestURI()) + l.logger.Debug("user(hash): %s / pass(hash): %s >> url: %s", string(hashUser), string(hashPass), r.URL.RequestURI()) if user == l.WebUsername && pass == l.WebPassword { - l.logger.Debug("[VALID] user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user)), - md5.Sum([]byte(pass)), r.URL.RequestURI()) + l.logger.Debug("[VALID] user(hash): %s / pass(hash): %s >> url: %s", string(hashUser), string(hashPass), r.URL.RequestURI()) allowed = true } else { allowed = false - l.logger.Info("[INVALID] user(md5): %s / pass(md5): %s >> url: %s", md5.Sum([]byte(user)), - md5.Sum([]byte(pass)), r.URL.RequestURI()) + l.logger.Info("[INVALID] user(hash): %s / pass(hash): %s >> url: %s", string(hashUser), string(hashPass), r.URL.RequestURI()) } } } @@ -74,3 +87,11 @@ func (l *RequestLogger) ServeHTTP(rw http.ResponseWriter, r *http.Request, next } l.logger.Debug("%s %s – respond HTTP %d", r.Method, r.URL.RequestURI(), rw.(negroni.ResponseWriter).Status()) } + +func hashData(data []byte) ([]byte, error) { + hashed, err := bcrypt.GenerateFromPassword(data, bcrypt.DefaultCost) + if err != nil { + return nil, err + } + return hashed, nil +} From 6b9d411b77792a8c30f15efdaf41e53988c35b72 Mon Sep 17 00:00:00 2001 From: Pantelis Karamolegkos Date: Fri, 18 Feb 2022 00:15:02 +0200 Subject: [PATCH 4/6] Entirely removing credentials --- server/middleware.go | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/server/middleware.go b/server/middleware.go index 1478f7c6ac..4ac0d08109 100644 --- a/server/middleware.go +++ b/server/middleware.go @@ -18,7 +18,6 @@ import ( "github.com/runatlantis/atlantis/server/logging" "github.com/urfave/negroni" - "golang.org/x/crypto/bcrypt" ) // NewRequestLogger creates a RequestLogger. @@ -45,10 +44,6 @@ type RequestLogger struct { // ServeHTTP implements the middleware function. It logs all requests at DEBUG level. func (l *RequestLogger) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) { l.logger.Debug("%s %s – from %s", r.Method, r.URL.RequestURI(), r.RemoteAddr) - var ( - hashUser, hashPass []byte - err error - ) allowed := false if !l.WebAuthentication || r.URL.Path == "/events" || @@ -57,25 +52,14 @@ func (l *RequestLogger) ServeHTTP(rw http.ResponseWriter, r *http.Request, next allowed = true } else { user, pass, ok := r.BasicAuth() - hashUser, err = hashData([]byte(user)) - if err != nil { - l.logger.Debug("unable to hash username, defaulting to %s", redacted) - hashUser = []byte(redacted) - } - hashPass, err = hashData([]byte(pass)) - if err != nil { - l.logger.Debug("unable to hash password, defaulting to %s", redacted) - hashPass = []byte(redacted) - } if ok { r.SetBasicAuth(user, pass) - l.logger.Debug("user(hash): %s / pass(hash): %s >> url: %s", string(hashUser), string(hashPass), r.URL.RequestURI()) if user == l.WebUsername && pass == l.WebPassword { - l.logger.Debug("[VALID] user(hash): %s / pass(hash): %s >> url: %s", string(hashUser), string(hashPass), r.URL.RequestURI()) + l.logger.Debug("[VALID] log in: %s >> url: %s", r.URL.RequestURI()) allowed = true } else { allowed = false - l.logger.Info("[INVALID] user(hash): %s / pass(hash): %s >> url: %s", string(hashUser), string(hashPass), r.URL.RequestURI()) + l.logger.Info("[INVALID] log in attempt: %s >> url: %s", r.URL.RequestURI()) } } } @@ -87,11 +71,3 @@ func (l *RequestLogger) ServeHTTP(rw http.ResponseWriter, r *http.Request, next } l.logger.Debug("%s %s – respond HTTP %d", r.Method, r.URL.RequestURI(), rw.(negroni.ResponseWriter).Status()) } - -func hashData(data []byte) ([]byte, error) { - hashed, err := bcrypt.GenerateFromPassword(data, bcrypt.DefaultCost) - if err != nil { - return nil, err - } - return hashed, nil -} From f0c549b507e3950af9e501bae769bd1516af4d36 Mon Sep 17 00:00:00 2001 From: Pantelis Karamolegkos Date: Fri, 18 Feb 2022 00:16:36 +0200 Subject: [PATCH 5/6] Removing unused constant --- server/middleware.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/middleware.go b/server/middleware.go index 4ac0d08109..1cdbf89337 100644 --- a/server/middleware.go +++ b/server/middleware.go @@ -30,8 +30,6 @@ func NewRequestLogger(s *Server) *RequestLogger { } } -const redacted = "[REDACTED]" - // RequestLogger logs requests and their response codes. // as well as handle the basicauth on the requests type RequestLogger struct { From 119c2a5246c3e29a3bb8f26256754e52856a6898 Mon Sep 17 00:00:00 2001 From: Pantelis Karamolegkos Date: Fri, 18 Feb 2022 00:17:45 +0200 Subject: [PATCH 6/6] Removing redundant formatting parameters --- server/middleware.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/middleware.go b/server/middleware.go index 1cdbf89337..089394f770 100644 --- a/server/middleware.go +++ b/server/middleware.go @@ -53,11 +53,11 @@ func (l *RequestLogger) ServeHTTP(rw http.ResponseWriter, r *http.Request, next if ok { r.SetBasicAuth(user, pass) if user == l.WebUsername && pass == l.WebPassword { - l.logger.Debug("[VALID] log in: %s >> url: %s", r.URL.RequestURI()) + l.logger.Debug("[VALID] log in: >> url: %s", r.URL.RequestURI()) allowed = true } else { allowed = false - l.logger.Info("[INVALID] log in attempt: %s >> url: %s", r.URL.RequestURI()) + l.logger.Info("[INVALID] log in attempt: >> url: %s", r.URL.RequestURI()) } } }