Pomerium: proxy: Proxy configuration file preprocessed incorrectly, creating incorrect routes

[nareddyt]

Bug Description

When configuring a proxy locally with a policy.yaml file, some of the routes created are incorrect. This leads to the proxy routing requests incorrectly.

Environment

• Running the source locally. Latest commit to the master branch
• Windows 10 with Windows Subsystem for Linux

Impact

Minimal: I cannot test my fix to #129 with this bug in place. However, I believe I have a fix for this bug (below).

Root Cause

Specifically, I noticed that Proxy::routeConfigs is incorrectly initialized. In the image below, notice that two routes (with the configuration file given below) have the same memory address for the policy - highlighted in red. This should not occur, as each policy should be unique.

After some further digging, I noticed issues with memory addresses in this section of code. p.Handle(route.Source.Host, handler, &route), where the last argument is a pointer to route. However, the proxy must store a copy of the route for later requests outside of this function.

Proposed Fix

I will make a PR with the fix. The main modification would be here to store policies as a copy of the data, instead of pointers that do not exist after the main function exits.

Reproducibility

Policy Configuration

- from: hello1.homelab.tejunareddy.com
  to: http://localhost:8081
  allowed_users:
    - [email protected]
- from: hello2.homelab.tejunareddy.com
  to: http://localhost:8082
  allowed_users:
    - [email protected]
- from: hello3.homelab.tejunareddy.com
  to: http://localhost:8083
  allow_public_unauthenticated_access: true

Repro Steps

1. Start the server locally with this file
2. All requests to hello1.homelab.tejunareddy.com, hello2.homelab.tejunareddy.com, and hello3.homelab.tejunareddy.com are handled by the 3rd route, so they all redirect to http://localhost:8003.

Expected Behavior

It's expected that each route is handled by the correct proxy handler.

[desimone]

@nareddyt Thank you for the very well written bug report.

This is super odd -- what you are showing makes complete sense and I am seeing what you are seeing in relationship to the pointer being re-used but I am not able to reproduce what you are seeing (all requests being redirected to the last policy route.

log.Info().Str("src", route.Source.Host).Str("dst", route.Destination.Host).Msgf("proxy: new route: %p", &route)

func (p *Proxy) policy(r *http.Request) (*policy.Policy, bool) {
    config, ok := p.routeConfigs[r.Host]
    if ok {
+       log.Info().Str("host", r.Host).Msgf("config*: %p config.policy: %p", config, config.policy)
        return config.policy, true
    }
    return nil, false
}

httpbin.corp.beyondperimeter.com

4:24PM INF config*: 0xc00028e4a0 
+config.policy: 0xc0002ec360 config.policy={  
   "AllowedDomains":[  
      "pomerium.io"
   ],
   "AllowedEmails":null,
   "AllowedGroups":null,
   "CORSAllowPreflight":false,
   "Destination":{  
      "ForceQuery":false,
      "Fragment":"",
      "Host":"localhost:8080",
      "Opaque":"",
      "Path":"",
      "RawPath":"",
      "RawQuery":"",
      "Scheme":"http",
      "User":null
   },
+   "From":"hello.corp.beyondperimeter.com",
   "Source":{  
      "ForceQuery":false,
      "Fragment":"",
+      "Host":"hello.corp.beyondperimeter.com",
      "Opaque":"",
      "Path":"",
      "RawPath":"",
      "RawQuery":"",
      "Scheme":"https",
      "User":null
   },
   "To":"http://localhost:8080",
   "UpstreamTimeout":0
+} host=httpbin.corp.beyondperimeter.com

hello.corp.beyondperimeter.com

4:24PM INF config*: 0xc00028e700 config.policy: 
+0xc0002ec360 config.policy=
{  
   "AllowedDomains":[  
      "pomerium.io"
   ],
   "AllowedEmails":null,
   "AllowedGroups":null,
   "CORSAllowPreflight":false,
   "Destination":{  
      "ForceQuery":false,
      "Fragment":"",
      "Host":"localhost:8080",
      "Opaque":"",
      "Path":"",
      "RawPath":"",
      "RawQuery":"",
      "Scheme":"http",
      "User":null
   },
+   "From":"hello.corp.beyondperimeter.com",
   "Source":{  
      "ForceQuery":false,
      "Fragment":"",
      "Host":"hello.corp.beyondperimeter.com",
      "Opaque":"",
      "Path":"",
      "RawPath":"",
      "RawQuery":"",
      "Scheme":"https",
      "User":null
   },
   "To":"http://localhost:8080",
   "UpstreamTimeout":0
+} host=hello.corp.beyondperimeter.com

policy yaml
```policy.yaml

• from: httpbin.corp.beyondperimeter.com
  to: http://localhost:8000
  allowed_domains:
  
  
  
  • pomerium.io
  
  
• from: hello.corp.beyondperimeter.com
  to: http://localhost:8080
  allowed_domains:
  
  
  
  • pomerium.io
  
  

```bash
curl 'https://httpbin.corp.beyondperimeter.com/get' \
-H 'cookie: _pomerium=snip' \
--compressed  

```json
{
"args": {},
"headers": {
"Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,/;q=0.8,application/signed-exchange;v=b3",
"Accept-Encoding": "gzip, deflate, br",
"Accept-Language": "en-US,en;q=0.9",
"Cache-Control": "no-cache",
"Cookie": "",
"Dnt": "1",
"Host": "localhost:8000",
"Pragma": "no-cache",
"Referer": "https://httpbin.corp.beyondperimeter.com/",
"Upgrade-Insecure-Requests": "1",
"User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.157 Safari/537.36",
"X-Forwarded-Host": "httpbin.corp.beyondperimeter.com",
"X-Pomerium-Authenticated-User-Email": "[email protected]",
"X-Pomerium-Authenticated-User-Groups": "[email protected],[email protected],[email protected]",
"X-Pomerium-Authenticated-User-Id": "111432655977273150308",
"X-Pomerium-Jwt-Assertion": "eyJhbGciOiJFUzI1NiIsImtpZCI6IiIsInR5cCI6IkpXVCJ9.eyJhdWQiOlsiaHR0cGJpbi5jb3JwLmJleW9uZHBlcmltZXRlci5jb20iXSwiZW1haWwiOiJiZGRAcG9tZXJpdW0uaW8iLCJleHAiOjE1NTg0ODAwMjIsImdyb3VwcyI6ImFkbWluc0Bwb21lcml1bS5pbyxldmVyeW9uZUBwb21lcml1bS5pbyxzdXBwb3J0QHBvbWVyaXVtLmlvIiwiaWF0IjoxNTU4NDc5OTYyLCJpc3MiOiJwb21lcml1bS1wcm94eSIsIm5iZiI6MTU1ODQ3OTkwMiwic3ViIjoiMTExNDMyNjU1OTc3MjczMTUwMzA4In0.59yqdje6HLXh6JXhOaoy1SiVMc4yny-wxpe9X3_YPCv0B_aQKgXoDZ0swKbY2EWeg6ShCOeFNqZPSKo0X3wAoQ"
},
"origin": "192.168.1.1",
"url": "http://httpbin.corp.beyondperimeter.com/get"
}

```bash
curl 'https://hello.corp.beyondperimeter.com' \
-H 'cookie: _pomerium=snip' \
--compressed  

Hello, world!
Version: 2.0.0
Hostname: e14371a6b5de

where the last argument is a pointer to route. However, the proxy must store a copy of the route for later requests outside of this function.

Just so I understand, this issue only occurs if we want to modify a route after the fact and this only works today because we iterate over the list of policies but the pointer keeps getting moved forward.

[desimone]

tl;dr --Ok. I think I've got it. Routes work today by a happy accident but this is still very much a bug. Thank you @nareddyt

[nareddyt]

No problem! Yeah I found it odd because I was not able to reproduce this bug with the latest docker image, but it does occur when running from source code on my machine.

Just curious, do you have any guesses on why this bug hasn't caused problems beforehand? Let me know if you need me to provide any other logs like you did above.

[desimone]

I don't know why it is working differently on your machine vs docker (windows? that would be too weird!).

My best guess for why this ws working as intended is that the handler map is still being built so router() is still going to find an entry. Authentication and authorization are still being enforced because they are still being properly loaded in those services.

However, I speculate CORS is not working as intended. It's a newer feature but, as I write this, I remember I wasn't actually able to reproduce working CORS configuration until I used the contributors policy file which ... makes sense now looking back on it. I had several policies, the first of which was CORS enabled. His was the last.