Skip to content
This repository was archived by the owner on Jul 25, 2023. It is now read-only.
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fixed the bug in request_signer
  • Loading branch information
Maliheh committed Feb 20, 2019
commit c3b77624d29e479ac55729eca387f5134c67829f
45 changes: 29 additions & 16 deletions internal/devproxy/devproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const statusInvalidHost = 421

// DevProxy stores all the information associated with proxying the request.
type DevProxy struct {
redirectURL *url.URL // the url to receive requests at
// redirectURL *url.URL // the url to receive requests at
skipAuthPreflight bool
templates *template.Template
mux map[string]*route
Expand Down Expand Up @@ -104,15 +104,17 @@ func newUpstreamTransport(insecureSkipVerify bool) *upstreamTransport {

// ServeHTTP calls the upstream's ServeHTTP function.
func (u *UpstreamProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {

if u.requestSigner != nil {

u.requestSigner.Sign(r)
}

start := time.Now()
u.handler.ServeHTTP(w, r)
duration := time.Now().Sub(start)

fmt.Sprintf("service_name:%s, duation:%s", u.name, duration)
fmt.Printf("service_name:%s, duration:%s", u.name, duration)
}

func singleJoiningSlash(a, b string) string {
Expand Down Expand Up @@ -152,10 +154,6 @@ func NewReverseProxy(to *url.URL, config *UpstreamConfig) *httputil.ReverseProxy

proxy.Director = func(req *http.Request) {
req.Header.Add("X-Forwarded-Host", req.Host)
req.Header.Set("X-Forwarded-User", req.Header.Get("User"))
req.Header.Set("X-Forwarded-Email", req.Header.Get("Email"))
req.Header.Set("X-Forwarded-Groups", req.Header.Get("Groups"))
req.Header.Set("X-Forwarded-Access-Token", "")
dir(req)
req.Host = to.Host
}
Expand Down Expand Up @@ -185,10 +183,6 @@ func NewRewriteReverseProxy(route *RewriteRoute, config *UpstreamConfig) *httput
director := httputil.NewSingleHostReverseProxy(target).Director

req.Header.Add("X-Forwarded-Host", req.Host)
req.Header.Set("X-Forwarded-User", req.Header.Get("User"))
req.Header.Set("X-Forwarded-Email", req.Header.Get("Email"))
req.Header.Set("X-Forwarded-Groups", req.Header.Get("Groups"))
req.Header.Set("X-Forwarded-Access-Token", "")
director(req)
req.Host = target.Host
}
Expand All @@ -202,6 +196,7 @@ func NewReverseProxyHandler(reverseProxy *httputil.ReverseProxy, opts *Options,
handler: reverseProxy,
requestSigner: signer,
}

if config.SkipRequestSigning {
upstreamProxy.requestSigner = nil
}
Expand Down Expand Up @@ -255,18 +250,23 @@ func NewDevProxy(opts *Options, optFuncs ...func(*DevProxy) error) (*DevProxy, e
// Also build the `certs` static JSON-string which will be served from a public endpoint.
// The key published at this endpoint allows upstreams to decrypt the `Sso-Signature`
// header, and validate the integrity and authenticity of a request.

certs := make(map[string]string)
var requestSigner *RequestSigner
var err error
if len(opts.RequestSigningKey) > 0 {
requestSigner, err := NewRequestSigner(opts.RequestSigningKey)
requestSigner, err = NewRequestSigner(opts.RequestSigningKey)

if err != nil {
return nil, fmt.Errorf("could not build RequestSigner: %s", err)
}
id, key := requestSigner.PublicKey()
certs[id] = key

} else {
logger.Warn("Running DevProxy without signing key. Requests will not be signed.")
}

certsAsStr, err := json.MarshalIndent(certs, "", " ")
if err != nil {
return nil, fmt.Errorf("could not marshal public certs as JSON: %s", err)
Expand All @@ -277,7 +277,7 @@ func NewDevProxy(opts *Options, optFuncs ...func(*DevProxy) error) (*DevProxy, e
mux: make(map[string]*route),
regexRoutes: make([]*route, 0),

redirectURL: &url.URL{Path: "/oauth2/callback"},
// redirectURL: &url.URL{Path: "/oauth2/callback"},
templates: getTemplates(),
requestSigner: requestSigner,
publicCertsJSON: certsAsStr,
Expand All @@ -301,7 +301,7 @@ func NewDevProxy(opts *Options, optFuncs ...func(*DevProxy) error) (*DevProxy, e
handler, tags := NewReverseProxyHandler(reverseProxy, opts, upstreamConfig, requestSigner)
p.HandleRegex(route.FromRegex, handler, tags, upstreamConfig)
default:
return nil, fmt.Errorf("unkown route type")
return nil, fmt.Errorf("unknown route type")
}
}

Expand Down Expand Up @@ -342,6 +342,11 @@ func (p *DevProxy) RobotsTxt(rw http.ResponseWriter, _ *http.Request) {

// Favicon will proxy the request as usual
func (p *DevProxy) Favicon(rw http.ResponseWriter, req *http.Request) {
err := p.setProxyHeaders(rw, req)
if err != nil {
rw.WriteHeader(http.StatusNotFound)
return
}
rw.WriteHeader(http.StatusOK)
p.Proxy(rw, req)
}
Expand Down Expand Up @@ -391,10 +396,10 @@ func (p *DevProxy) isXMLHTTPRequest(req *http.Request) bool {
func (p *DevProxy) Proxy(rw http.ResponseWriter, req *http.Request) {

logger := log.NewLogEntry()
// start := time.Now()
logger.Info("Proxy...")
p.setProxyHeaders(rw, req)

// We have validated the users request and now proxy their request to the provided upstream.
logger.Info("Proxy...")
// We now proxy their request to the provided upstream.
route, ok := p.router(req)
if !ok {
p.UnknownHost(rw, req)
Expand Down Expand Up @@ -424,6 +429,14 @@ func (p *DevProxy) HandleRegex(regex *regexp.Regexp, handler http.Handler, tags
p.regexRoutes = append(p.regexRoutes, &route{regex: regex, handler: handler, upstreamConfig: upstreamConfig, tags: tags})
}

func (p *DevProxy) setProxyHeaders(rw http.ResponseWriter, req *http.Request) (err error) {
req.Header.Set("X-Forwarded-User", req.Header.Get("User"))
req.Header.Set("X-Forwarded-Email", req.Header.Get("Email"))
req.Header.Set("X-Forwarded-Groups", req.Header.Get("groups"))
// req.Header.set("X-Forwarded-Access-Token", "")
return nil
}

// router attempts to find a route for a request. If a route is successfully matched,
// it returns the route information and a bool value of `true`. If a route can not be matched,
//a nil value for the route and false bool value is returned.
Expand Down
10 changes: 5 additions & 5 deletions internal/devproxy/devproxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ func TestCerts(t *testing.T) {
opts.RequestSigningKey = string(requestSigningKey)
opts.Validate()

expectedPublicKey, err := ioutil.ReadFile("testdata/public_key.pub")
expectedPublicKey, err := ioutil.ReadFile("testdata/public_key.pem")
testutil.Assert(t, err == nil, "could not read public key from testdata: %s", err)

var keyHash []byte
Expand Down Expand Up @@ -798,7 +798,7 @@ func TestTimeoutHandler(t *testing.T) {
if res.StatusCode != tc.ExpectedStatusCode {
t.Errorf(" got=%v", res.StatusCode)
t.Errorf("want=%v", tc.ExpectedStatusCode)
t.Fatalf("got unexpcted status code")
t.Fatalf("got unexpected status code")
}

body, err := ioutil.ReadAll(res.Body)
Expand All @@ -809,7 +809,7 @@ func TestTimeoutHandler(t *testing.T) {
if string(body) != tc.ExpectedBody {
t.Errorf(" got=%q", body)
t.Errorf("want=%q", tc.ExpectedBody)
t.Fatalf("got unexpcted body")
t.Fatalf("got unexpected body")
}
})
}
Expand Down Expand Up @@ -868,7 +868,7 @@ func TestRewriteRoutingHandling(t *testing.T) {

upstreamHost, upstreamPort, err := net.SplitHostPort(parsedUpstreamURL.Host)
if err != nil {
t.Fatalf("expected to split host/hort err:%q", err)
t.Fatalf("expected to split host/port err:%q", err)
}

testCases := []struct {
Expand Down Expand Up @@ -902,7 +902,7 @@ func TestRewriteRoutingHandling(t *testing.T) {
ExpectedCode: statusInvalidHost,
},
{
Name: "it should match and replace using regex/template to find port in embeded domain",
Name: "it should match and replace using regex/template to find port in embedded domain",
TestHost: fmt.Sprintf("somedomain--%s", upstreamPort),
FromRegex: "somedomain--(.*)", // capture port
ToTemplate: fmt.Sprintf("%s:$1", upstreamHost), // add port to dest
Expand Down
9 changes: 3 additions & 6 deletions internal/devproxy/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@ import (
func GetActionTag(req *http.Request) string {
// only log metrics for these paths and actions
pathToAction := map[string]string{
"/favicon.ico": "favicon",
"/oauth2/sign_out": "sign_out",
"/oauth2/callback": "callback",
"/oauth2/auth": "auth",
"/ping": "ping",
"/robots.txt": "robots",
"/favicon.ico": "favicon",
"/ping": "ping",
"/robots.txt": "robots",
}
// get the action from the url path
path := req.URL.Path
Expand Down
18 changes: 14 additions & 4 deletions internal/devproxy/request_signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ var signedHeaders = []string{
"X-Forwarded-User",
"X-Forwarded-Email",
"X-Forwarded-Groups",
"Cookie",
}

// Name of the header used to transmit the signature computed for the request.
Expand Down Expand Up @@ -96,8 +95,8 @@ func NewRequestSigner(signingKeyPemStr string) (*RequestSigner, error) {
// <URL>
// <BODY>
// where:
// <HEADER.k> is the ','-joined concatenation of all header values of `signedHeaders[k]`; all
// other headers in the request are ignored,
// <HEADER.k> is the ','-joined concatenation of all header values of `signedHeaders[k]`; empty
// values such as '' and all other headers in the request are ignored,
// <URL> is the string "<PATH>(?<QUERY>)(#FRAGMENT)", where "?<QUERY>" and "#<FRAGMENT>" are
// ommitted if the associated components are absent from the request URL,
// <BODY> is the body of the Request (may be `nil`; e.g. for GET requests).
Expand All @@ -109,7 +108,8 @@ func mapRequestToHashInput(req *http.Request) (string, error) {

// Add signed headers.
for _, hdr := range signedHeaders {
if hdrValues := req.Header[hdr]; len(hdrValues) > 0 {
hdrValues := removeEmpty(req.Header[hdr])
if len(hdrValues) > 0 {
entries = append(entries, strings.Join(hdrValues, ","))
}
}
Expand Down Expand Up @@ -189,3 +189,13 @@ func (signer RequestSigner) Sign(req *http.Request) error {
func (signer RequestSigner) PublicKey() (string, string) {
return signer.publicKeyID, signer.publicKeyStr
}

func removeEmpty(s []string) []string {
r := []string{}
for _, str := range s {
if len(str) > 0 {
r = append(r, str)
}
}
return r
}
2 changes: 1 addition & 1 deletion internal/devproxy/request_signer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func TestSignatureRoundTripDecoding(t *testing.T) {
privateKey, err := ioutil.ReadFile("testdata/private_key.pem")
testutil.Assert(t, err == nil, "error reading private key from testdata")

publicKey, err := ioutil.ReadFile("testdata/public_key.pub")
publicKey, err := ioutil.ReadFile("testdata/public_key.pem")
testutil.Assert(t, err == nil, "error reading public key from testdata")

// Build the RequestSigner object used to generate the request signature header.
Expand Down
10 changes: 10 additions & 0 deletions internal/devproxy/testdata/.env
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export PORT=4888
export SCHEME=http
export HOST=http://localhost/
export UPSTREAM_CONFIGS=/path/to/upstream_configs.yml
export CLUSTER=sso-dev
export DEFAULT_UPSTREAM_TIMEOUT=10s
export TCP_WRITE_TIMEOUT=30s
export TCP_READ_TIMEOUT=30s
export REQUEST_LOGGING=true
export REQUEST_SIGNATURE_KEY=$(cat /path/to/devproxy/testdata/private_key.pem)
28 changes: 28 additions & 0 deletions internal/devproxy/testdata/private_key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCy38IQCH8QyeNF
s1zA0XuIyqnTcSfYZg0nPfB+K//pFy7tIOAwmR6th8NykrxFhEQDHKNCmLXt4j8V
FDHQZtGjUBHRmAXZW8NOQ0EI1vc/Dpt09sU40JQlXZZeL+9/7iAxEfSE3TQr1k7P
Xwxpjm9rsLSn7FoLnvXco0mc6+d2jjxf4cMgJIaQLKOd783KUQzLVEvBQJ05JnpI
2xMjS0q33ltMTMGF3QZQN9i4bZKgnItomKxTJbfxftO11FTNLB7og94sWmlThAY5
/UMjZaWYJ1g89+WUJ+KpVYyJsHPBBkaQG+NYazcLDyIowpzJ1WVkInysshpTqwT+
UPV4at+jAgMBAAECggEAX8lxK5LRMJVcLlwRZHQJekRE0yS6WKi1jHkfywEW5qRy
jatYQs4MXpLgN/+Z8IQWw6/XQXdznTLV4xzQXDBjPNhI4ntNTotUOBnNvsUW296f
ou/uxzDy1FuchU2YLGLBPGXIEko+gOcfhu74P6J1yi5zX6UyxxxVvtR2PCEb7yDw
m2881chwMblZ5Z8uyF++ajkK3/rqLk64w29+K4ZTDbTcCp5NtBYx2qSEU7yp12rc
qscUGqxG00Abx+osI3cUn0kOq7356LeR1rfA15yZwOb+s28QYp2WPlVB2hOiYXQv
+ttEOpt0x1QJhBAsFgwY173sD5w2MryRQb1RCwBvqQKBgQDeTdbRzxzAl83h/mAq
5I+pNEz57veAFVO+iby7TbZ/0w6q+QeT+bHF+TjGHiSlbtg3nd9NPrex2UjiN7ej
+DrxhsSLsP1ZfwDNv6f1Ii1HluJclUFSUNU/LntBjqqCJ959lniNp1y5+ZQ/j2Rf
+ZraVsHRB0itilFeAl5+n7CfxwKBgQDN/K+E1TCbp1inU60Lc9zeb8fqTEP6Mp36
qQ0Dp+KMLPJ0xQSXFq9ILr4hTJlBqfmTkfmQUcQuwercZ3LNQPbsuIg96bPW73R1
toXjokd6jUn5sJXCOE0RDumcJrL1VRf9RN1AmM4CgCc/adUMjws3pBc5R4An7UyU
ouRQhN+5RQKBgFOVTrzqM3RSX22mWAAomb9T09FxQQueeTM91IFUMdcTwwMTyP6h
Nm8qSmdrM/ojmBYpPKlteGHdQaMUse5rybXAJywiqs84ilPRyNPJOt8c4xVOZRYP
IG62Ck/W1VNErEnqBn+0OpAOP+g6ANJ5JfkL/6mZJIFjbT58g4z2e9FHAoGBAM3f
uBkd7lgTuLJ8Gh6xLVYQCJHuqZ49ytFE9qHpwK5zGdyFMSJE5OlS9mpXoXEUjkHk
iraoUlidLbwdlIr6XBCaGmku07SFXTNtOoIZpjEhV4c762HTXYsoCWos733uD2zt
z+iJEJVFOnTRtMK5kO+KjD+Oa9L8BCcmauTi+Ku1AoGAZBUzi95THA60hPXI0hm/
o0J5mfLkFPfhpUmDAMaEpv3bM4byA+IGXSZVc1IZO6cGoaeUHD2Yl1m9a5tv5rF+
FS9Ht+IgATvGojah+xxQy+kf6tRB9Hn4scyq+64AesXlDbWDEagomQ0hyV/JKSS6
LQatvnCmBd9omRT2uwYUo+o=
-----END PRIVATE KEY-----
8 changes: 8 additions & 0 deletions internal/devproxy/testdata/public_key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-----BEGIN RSA PUBLIC KEY-----
MIIBCgKCAQEAst/CEAh/EMnjRbNcwNF7iMqp03En2GYNJz3wfiv/6Rcu7SDgMJke
rYfDcpK8RYREAxyjQpi17eI/FRQx0GbRo1AR0ZgF2VvDTkNBCNb3Pw6bdPbFONCU
JV2WXi/vf+4gMRH0hN00K9ZOz18MaY5va7C0p+xaC5713KNJnOvndo48X+HDICSG
kCyjne/NylEMy1RLwUCdOSZ6SNsTI0tKt95bTEzBhd0GUDfYuG2SoJyLaJisUyW3
8X7TtdRUzSwe6IPeLFppU4QGOf1DI2WlmCdYPPfllCfiqVWMibBzwQZGkBvjWGs3
Cw8iKMKcydVlZCJ8rLIaU6sE/lD1eGrfowIDAQAB
-----END RSA PUBLIC KEY-----
8 changes: 5 additions & 3 deletions internal/devproxy/testdata/upstream_configs.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
- service: httpbin
- service: dev-shim
default:
from: httpbin.sso.localtest.me
to: http://httpheader.net
from: http://localhost:4888
to: http://localhost:4810
options:
skip_request_signing: false