From 44bd8c758f6db096dccd901378f418a14dac5a03 Mon Sep 17 00:00:00 2001 From: Mygod Date: Wed, 26 Apr 2023 16:35:55 -0400 Subject: [PATCH 01/10] Fix multiauth --- caddyfile.go | 14 ++++++++------ common_test.go | 30 +++++++++++++++++------------- forwardproxy.go | 22 +++++++--------------- 3 files changed, 32 insertions(+), 34 deletions(-) diff --git a/caddyfile.go b/caddyfile.go index 4124d11..057df5a 100644 --- a/caddyfile.go +++ b/caddyfile.go @@ -1,6 +1,7 @@ package forwardproxy import ( + "encoding/base64" "log" "strconv" "strings" @@ -45,13 +46,14 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if strings.Contains(args[0], ":") { return d.Err("character ':' in usernames is not allowed") } - // TODO: Support multiple basicauths. - // TODO: Actually, just try to use Caddy 2's existing basicauth module. - if h.BasicauthUser != "" || h.BasicauthPass != "" { - return d.Err("Multi-user basicauth is not supported") + if h.AuthCredentials == nil { + h.AuthCredentials = [][]byte{} } - h.BasicauthUser = args[0] - h.BasicauthPass = args[1] + // base64-encode credentials + buf := make([]byte, base64.StdEncoding.EncodedLen(len(args[0])+1+len(args[1]))) + base64.StdEncoding.Encode(buf, []byte(args[0]+":"+args[1])) + h.AuthCredentials = append(h.AuthCredentials, buf) + h.AuthRequired = true case "hosts": if len(args) == 0 { return d.ArgErr() diff --git a/common_test.go b/common_test.go index b8bad5f..c9da928 100644 --- a/common_test.go +++ b/common_test.go @@ -3,6 +3,7 @@ package forwardproxy import ( "context" "crypto/tls" + "encoding/base64" "encoding/hex" "encoding/json" "fmt" @@ -190,15 +191,18 @@ func TestMain(m *testing.M) { }, } + buf := make([]byte, base64.StdEncoding.EncodedLen(9)) + base64.StdEncoding.Encode(buf, []byte("test:pass")) + caddyForwardProxyAuth = caddyTestServer{ addr: "127.0.0.1:4891", root: "./test/forwardproxy", tls: true, proxyHandler: &Handler{ - PACPath: defaultPACPath, - ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, - BasicauthUser: "test", - BasicauthPass: "pass", + PACPath: defaultPACPath, + ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, + AuthCredentials: [][]byte{buf}, + AuthRequired: true, }, } @@ -206,10 +210,10 @@ func TestMain(m *testing.M) { addr: "127.0.69.73:6973", root: "./test/forwardproxy", proxyHandler: &Handler{ - PACPath: defaultPACPath, - ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, - BasicauthUser: "test", - BasicauthPass: "pass", + PACPath: defaultPACPath, + ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, + AuthCredentials: [][]byte{buf}, + AuthRequired: true, }, } @@ -221,8 +225,8 @@ func TestMain(m *testing.M) { PACPath: "/superhiddenfile.pac", ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, ProbeResistance: &ProbeResistance{Domain: "test.localhost"}, - BasicauthUser: "test", - BasicauthPass: "pass", + AuthCredentials: [][]byte{buf}, + AuthRequired: true, }, httpRedirPort: "8880", } @@ -249,9 +253,9 @@ func TestMain(m *testing.M) { root: "./test/upstreamingproxy", tls: true, proxyHandler: &Handler{ - Upstream: "https://test:pass@127.0.0.1:4891", - BasicauthUser: "upstreamtest", - BasicauthPass: "upstreampass", + Upstream: "https://test:pass@127.0.0.1:4891", + AuthCredentials: [][]byte{buf}, + AuthRequired: true, }, } diff --git a/forwardproxy.go b/forwardproxy.go index 45ec450..4309cb8 100644 --- a/forwardproxy.go +++ b/forwardproxy.go @@ -91,10 +91,10 @@ type Handler struct { aclRules []aclRule // TODO: temporary/deprecated - we should try to reuse existing authentication modules instead! - BasicauthUser string `json:"auth_user_deprecated,omitempty"` - BasicauthPass string `json:"auth_pass_deprecated,omitempty"` - authRequired bool - authCredentials [][]byte // slice with base64-encoded credentials + BasicauthUser string `json:"auth_user_deprecated,omitempty"` + BasicauthPass string `json:"auth_pass_deprecated,omitempty"` + AuthRequired bool `json:"auth_required,omitempty"` + AuthCredentials [][]byte `json:"auth_credentials,omitempty"` // slice with base64-encoded credentials } // CaddyModule returns the Caddy module information. @@ -120,14 +120,6 @@ func (h *Handler) Provision(ctx caddy.Context) error { TLSHandshakeTimeout: 10 * time.Second, } - // TODO: temporary, in an effort to get the tests to pass - if h.BasicauthUser != "" && h.BasicauthPass != "" { - basicAuthBuf := make([]byte, base64.StdEncoding.EncodedLen(len(h.BasicauthUser)+1+len(h.BasicauthPass))) - base64.StdEncoding.Encode(basicAuthBuf, []byte(h.BasicauthUser+":"+h.BasicauthPass)) - h.authRequired = true - h.authCredentials = [][]byte{basicAuthBuf} - } - // access control lists for _, rule := range h.ACL { for _, subj := range rule.Subjects { @@ -155,7 +147,7 @@ func (h *Handler) Provision(ctx caddy.Context) error { h.aclRules = append(h.aclRules, &aclAllRule{allow: true}) if h.ProbeResistance != nil { - if !h.authRequired { + if !h.AuthRequired { return fmt.Errorf("probe resistance requires authentication") } if len(h.ProbeResistance.Domain) > 0 { @@ -237,7 +229,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht } var authErr error - if h.authRequired { + if h.AuthRequired { authErr = h.checkCredentials(r) } if h.ProbeResistance != nil && len(h.ProbeResistance.Domain) > 0 && reqHost == h.ProbeResistance.Domain { @@ -418,7 +410,7 @@ func (h Handler) checkCredentials(r *http.Request) error { if strings.ToLower(pa[0]) != "basic" { return errors.New("Auth type is not supported") } - for _, creds := range h.authCredentials { + for _, creds := range h.AuthCredentials { if subtle.ConstantTimeCompare(creds, []byte(pa[1])) == 1 { // Please do not consider this to be timing-attack-safe code. Simple equality is almost // mindlessly substituted with constant time algo and there ARE known issues with this code, From 51a9bb545bac520540aca4872c9702f3b63483aa Mon Sep 17 00:00:00 2001 From: Mygod Date: Wed, 26 Apr 2023 16:42:19 -0400 Subject: [PATCH 02/10] Remove useless fields --- forwardproxy.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/forwardproxy.go b/forwardproxy.go index 4309cb8..cf8672f 100644 --- a/forwardproxy.go +++ b/forwardproxy.go @@ -91,8 +91,6 @@ type Handler struct { aclRules []aclRule // TODO: temporary/deprecated - we should try to reuse existing authentication modules instead! - BasicauthUser string `json:"auth_user_deprecated,omitempty"` - BasicauthPass string `json:"auth_pass_deprecated,omitempty"` AuthRequired bool `json:"auth_required,omitempty"` AuthCredentials [][]byte `json:"auth_credentials,omitempty"` // slice with base64-encoded credentials } From 035199efd11dc69e136c5742fb236e0c9bf647d5 Mon Sep 17 00:00:00 2001 From: Mygod Date: Thu, 27 Apr 2023 22:01:42 -0400 Subject: [PATCH 03/10] Add support for printing user id in log --- forwardproxy.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/forwardproxy.go b/forwardproxy.go index cf8672f..a967824 100644 --- a/forwardproxy.go +++ b/forwardproxy.go @@ -410,6 +410,11 @@ func (h Handler) checkCredentials(r *http.Request) error { } for _, creds := range h.AuthCredentials { if subtle.ConstantTimeCompare(creds, []byte(pa[1])) == 1 { + repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) + buf := make([]byte, base64.StdEncoding.DecodedLen(len(creds))) + _, _ = base64.StdEncoding.Decode(buf, creds) // should not err ever since we are decoding a known good input + cred := string(buf) + repl.Set("http.auth.user.id", cred[:strings.IndexByte(cred, ':')]) // Please do not consider this to be timing-attack-safe code. Simple equality is almost // mindlessly substituted with constant time algo and there ARE known issues with this code, // e.g. size of smallest credentials is guessable. TODO: protect from all the attacks! Hash? From 4e71e857d9bcfac8d7dd28874e9885371080125f Mon Sep 17 00:00:00 2001 From: Mygod Date: Wed, 19 Jul 2023 23:43:13 -0400 Subject: [PATCH 04/10] Log invalid user as well --- forwardproxy.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/forwardproxy.go b/forwardproxy.go index 8422851..d1aa93d 100644 --- a/forwardproxy.go +++ b/forwardproxy.go @@ -35,6 +35,7 @@ import ( "strings" "sync" "time" + "unicode/utf8" caddy "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" @@ -421,6 +422,18 @@ func (h Handler) checkCredentials(r *http.Request) error { return nil } } + buf := make([]byte, base64.StdEncoding.DecodedLen(len([]byte(pa[1])))) + n, err := base64.StdEncoding.Decode(buf, []byte(pa[1])) + if err == nil && utf8.Valid(buf[:n]) { + cred := string(buf[:n]) + i := strings.IndexByte(cred, ':') + repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) + if i >= 0 { + repl.Set("http.auth.user.id", "invalid:"+cred[:i]) + } else { + repl.Set("http.auth.user.id", "invalidformat:"+cred) + } + } return errors.New("invalid credentials") } From 6a4bb51efcfff2f033ba5afc8af5a767792a8616 Mon Sep 17 00:00:00 2001 From: Mygod Date: Fri, 21 Jul 2023 13:57:03 -0400 Subject: [PATCH 05/10] Refine error user --- forwardproxy.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/forwardproxy.go b/forwardproxy.go index d1aa93d..d27befc 100644 --- a/forwardproxy.go +++ b/forwardproxy.go @@ -422,17 +422,23 @@ func (h Handler) checkCredentials(r *http.Request) error { return nil } } + repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) buf := make([]byte, base64.StdEncoding.DecodedLen(len([]byte(pa[1])))) n, err := base64.StdEncoding.Decode(buf, []byte(pa[1])) - if err == nil && utf8.Valid(buf[:n]) { + if err != nil { + repl.Set("http.auth.user.id", "invalidbase64:"+err.Error()) + return err + } + if utf8.Valid(buf[:n]) { cred := string(buf[:n]) i := strings.IndexByte(cred, ':') - repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) if i >= 0 { repl.Set("http.auth.user.id", "invalid:"+cred[:i]) } else { repl.Set("http.auth.user.id", "invalidformat:"+cred) } + } else { + repl.Set("http.auth.user.id", "invalid::") } return errors.New("invalid credentials") } From cd93f0ed5136fee53cf123f19e97e569e1225c4e Mon Sep 17 00:00:00 2001 From: Mygod Date: Thu, 2 Nov 2023 14:55:11 -0400 Subject: [PATCH 06/10] Fix upstream creds in test --- common_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/common_test.go b/common_test.go index e687cde..313691f 100644 --- a/common_test.go +++ b/common_test.go @@ -212,13 +212,16 @@ func TestMain(m *testing.M) { }, } + upstreamBuf := make([]byte, base64.StdEncoding.EncodedLen(9)) + base64.StdEncoding.Encode(upstreamBuf, []byte("upstreamtest:upstreampass")) + caddyHTTPForwardProxyAuth = caddyTestServer{ addr: "127.0.69.73:6973", root: "./test/forwardproxy", proxyHandler: &Handler{ PACPath: defaultPACPath, ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, - AuthCredentials: [][]byte{buf}, + AuthCredentials: [][]byte{upstreamBuf}, AuthRequired: true, }, } From 611a43963a4703fc7d8920499de707a40114a70b Mon Sep 17 00:00:00 2001 From: Mygod Date: Thu, 2 Nov 2023 14:57:39 -0400 Subject: [PATCH 07/10] Fix encoded len --- common_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common_test.go b/common_test.go index 313691f..4300b5c 100644 --- a/common_test.go +++ b/common_test.go @@ -212,7 +212,7 @@ func TestMain(m *testing.M) { }, } - upstreamBuf := make([]byte, base64.StdEncoding.EncodedLen(9)) + upstreamBuf := make([]byte, base64.StdEncoding.EncodedLen(25)) base64.StdEncoding.Encode(upstreamBuf, []byte("upstreamtest:upstreampass")) caddyHTTPForwardProxyAuth = caddyTestServer{ From 9976c57edb0c47a2101f01d74a5f5f993bf2f644 Mon Sep 17 00:00:00 2001 From: Mygod Date: Thu, 2 Nov 2023 15:03:05 -0400 Subject: [PATCH 08/10] Oops --- common_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/common_test.go b/common_test.go index 4300b5c..97bd277 100644 --- a/common_test.go +++ b/common_test.go @@ -212,16 +212,13 @@ func TestMain(m *testing.M) { }, } - upstreamBuf := make([]byte, base64.StdEncoding.EncodedLen(25)) - base64.StdEncoding.Encode(upstreamBuf, []byte("upstreamtest:upstreampass")) - caddyHTTPForwardProxyAuth = caddyTestServer{ addr: "127.0.69.73:6973", root: "./test/forwardproxy", proxyHandler: &Handler{ PACPath: defaultPACPath, ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, - AuthCredentials: [][]byte{upstreamBuf}, + AuthCredentials: [][]byte{buf}, AuthRequired: true, }, } @@ -257,13 +254,16 @@ func TestMain(m *testing.M) { root: "./test/index", } + upstreamBuf := make([]byte, base64.StdEncoding.EncodedLen(25)) + base64.StdEncoding.Encode(upstreamBuf, []byte("upstreamtest:upstreampass")) + caddyAuthedUpstreamEnter = caddyTestServer{ addr: "127.0.65.25:6585", root: "./test/upstreamingproxy", tls: true, proxyHandler: &Handler{ Upstream: "https://test:pass@127.0.0.1:4891", - AuthCredentials: [][]byte{buf}, + AuthCredentials: [][]byte{upstreamBuf}, AuthRequired: true, }, } From 226c444d94efd61da5f5ab24eb8c123c36e263d9 Mon Sep 17 00:00:00 2001 From: Mygod Date: Sun, 5 Nov 2023 10:01:10 -0500 Subject: [PATCH 09/10] Remove duplicate code --- caddyfile.go | 13 +++++++++---- common_test.go | 15 ++++----------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/caddyfile.go b/caddyfile.go index bf0fcf7..87dde9e 100644 --- a/caddyfile.go +++ b/caddyfile.go @@ -22,6 +22,14 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) return &fp, err } +// EncodeAuthCredentials base64-encode credentials +func EncodeAuthCredentials(user, pass string) (result []byte) { + raw := []byte(user + ":" + pass) + result = make([]byte, base64.StdEncoding.EncodedLen(len(raw))) + base64.StdEncoding.Encode(result, raw) + return +} + // UnmarshalCaddyfile unmarshals Caddyfile tokens into h. func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if !d.Next() { @@ -49,10 +57,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if h.AuthCredentials == nil { h.AuthCredentials = [][]byte{} } - // base64-encode credentials - buf := make([]byte, base64.StdEncoding.EncodedLen(len(args[0])+1+len(args[1]))) - base64.StdEncoding.Encode(buf, []byte(args[0]+":"+args[1])) - h.AuthCredentials = append(h.AuthCredentials, buf) + h.AuthCredentials = append(h.AuthCredentials, EncodeAuthCredentials(args[0], args[1])) h.AuthRequired = true case "hosts": if len(args) == 0 { diff --git a/common_test.go b/common_test.go index 97bd277..18ce832 100644 --- a/common_test.go +++ b/common_test.go @@ -3,7 +3,6 @@ package forwardproxy import ( "context" "crypto/tls" - "encoding/base64" "encoding/hex" "encoding/json" "fmt" @@ -197,9 +196,6 @@ func TestMain(m *testing.M) { }, } - buf := make([]byte, base64.StdEncoding.EncodedLen(9)) - base64.StdEncoding.Encode(buf, []byte("test:pass")) - caddyForwardProxyAuth = caddyTestServer{ addr: "127.0.0.1:4891", root: "./test/forwardproxy", @@ -207,7 +203,7 @@ func TestMain(m *testing.M) { proxyHandler: &Handler{ PACPath: defaultPACPath, ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, - AuthCredentials: [][]byte{buf}, + AuthCredentials: [][]byte{EncodeAuthCredentials("test", "pass")}, AuthRequired: true, }, } @@ -218,7 +214,7 @@ func TestMain(m *testing.M) { proxyHandler: &Handler{ PACPath: defaultPACPath, ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, - AuthCredentials: [][]byte{buf}, + AuthCredentials: [][]byte{EncodeAuthCredentials("test", "pass")}, AuthRequired: true, }, } @@ -231,7 +227,7 @@ func TestMain(m *testing.M) { PACPath: "/superhiddenfile.pac", ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, ProbeResistance: &ProbeResistance{Domain: "test.localhost"}, - AuthCredentials: [][]byte{buf}, + AuthCredentials: [][]byte{EncodeAuthCredentials("test", "pass")}, AuthRequired: true, }, httpRedirPort: "8880", @@ -254,16 +250,13 @@ func TestMain(m *testing.M) { root: "./test/index", } - upstreamBuf := make([]byte, base64.StdEncoding.EncodedLen(25)) - base64.StdEncoding.Encode(upstreamBuf, []byte("upstreamtest:upstreampass")) - caddyAuthedUpstreamEnter = caddyTestServer{ addr: "127.0.65.25:6585", root: "./test/upstreamingproxy", tls: true, proxyHandler: &Handler{ Upstream: "https://test:pass@127.0.0.1:4891", - AuthCredentials: [][]byte{upstreamBuf}, + AuthCredentials: [][]byte{EncodeAuthCredentials("upstreamtest", "upstreampass")}, AuthRequired: true, }, } From 5b0898c8e8608898fece2f0382b020ef0dcead71 Mon Sep 17 00:00:00 2001 From: Mygod Date: Sun, 5 Nov 2023 10:04:09 -0500 Subject: [PATCH 10/10] Remove useless field --- caddyfile.go | 1 - common_test.go | 4 ---- forwardproxy.go | 5 ++--- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/caddyfile.go b/caddyfile.go index 87dde9e..239b5c7 100644 --- a/caddyfile.go +++ b/caddyfile.go @@ -58,7 +58,6 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { h.AuthCredentials = [][]byte{} } h.AuthCredentials = append(h.AuthCredentials, EncodeAuthCredentials(args[0], args[1])) - h.AuthRequired = true case "hosts": if len(args) == 0 { return d.ArgErr() diff --git a/common_test.go b/common_test.go index 18ce832..4880d3b 100644 --- a/common_test.go +++ b/common_test.go @@ -204,7 +204,6 @@ func TestMain(m *testing.M) { PACPath: defaultPACPath, ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, AuthCredentials: [][]byte{EncodeAuthCredentials("test", "pass")}, - AuthRequired: true, }, } @@ -215,7 +214,6 @@ func TestMain(m *testing.M) { PACPath: defaultPACPath, ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, AuthCredentials: [][]byte{EncodeAuthCredentials("test", "pass")}, - AuthRequired: true, }, } @@ -228,7 +226,6 @@ func TestMain(m *testing.M) { ACL: []ACLRule{{Subjects: []string{"all"}, Allow: true}}, ProbeResistance: &ProbeResistance{Domain: "test.localhost"}, AuthCredentials: [][]byte{EncodeAuthCredentials("test", "pass")}, - AuthRequired: true, }, httpRedirPort: "8880", } @@ -257,7 +254,6 @@ func TestMain(m *testing.M) { proxyHandler: &Handler{ Upstream: "https://test:pass@127.0.0.1:4891", AuthCredentials: [][]byte{EncodeAuthCredentials("upstreamtest", "upstreampass")}, - AuthRequired: true, }, } diff --git a/forwardproxy.go b/forwardproxy.go index c2f4982..218f205 100644 --- a/forwardproxy.go +++ b/forwardproxy.go @@ -92,7 +92,6 @@ type Handler struct { aclRules []aclRule // TODO: temporary/deprecated - we should try to reuse existing authentication modules instead! - AuthRequired bool `json:"auth_required,omitempty"` AuthCredentials [][]byte `json:"auth_credentials,omitempty"` // slice with base64-encoded credentials } @@ -146,7 +145,7 @@ func (h *Handler) Provision(ctx caddy.Context) error { h.aclRules = append(h.aclRules, &aclAllRule{allow: true}) if h.ProbeResistance != nil { - if !h.AuthRequired { + if h.AuthCredentials == nil { return fmt.Errorf("probe resistance requires authentication") } if len(h.ProbeResistance.Domain) > 0 { @@ -228,7 +227,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht } var authErr error - if h.AuthRequired { + if h.AuthCredentials != nil { authErr = h.checkCredentials(r) } if h.ProbeResistance != nil && len(h.ProbeResistance.Domain) > 0 && reqHost == h.ProbeResistance.Domain {