Skip to content
This repository has been archived by the owner on Jul 25, 2023. It is now read-only.

Commit

Permalink
fixed the bug in request_signer
Browse files Browse the repository at this point in the history
  • Loading branch information
Maliheh committed Feb 20, 2019
1 parent 6546833 commit c3b7762
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 35 deletions.
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


0 comments on commit c3b7762

Please sign in to comment.