Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Malicious user can access one server forever by forging cookie #107

Open
onestraw opened this issue Dec 15, 2017 · 6 comments
Open

Malicious user can access one server forever by forging cookie #107

onestraw opened this issue Dec 15, 2017 · 6 comments

Comments

@onestraw
Copy link
Contributor

Malicious user can access the specific server forever by setting backend server in cookie, it's very dangerous if the project is used in API gateway.

I will show how to reproduce the issue:

gateway code

package main

import (
	"net/http"
	"net/url"

	"github.com/vulcand/oxy/forward"
	"github.com/vulcand/oxy/roundrobin"
)

func main() {
	fwd, _ := forward.New()

	ss := roundrobin.NewStickySession("zzz")
	lb, err := roundrobin.New(fwd, roundrobin.EnableStickySession(ss))
	if err != nil {
		panic(err)
	}

	url1, _ := url.Parse("http://127.0.0.1:8001")
	url2, _ := url.Parse("http://127.0.0.1:8002")

	err = lb.UpsertServer(url1, roundrobin.Weight(1))
	err = lb.UpsertServer(url2, roundrobin.Weight(2))

	s := &http.Server{
		Addr:    ":8000",
		Handler: lb,
	}
	panic(s.ListenAndServe())
}

exploit

  • get one cookie

curl -i http://127.0.0.1:8000

    Set-Cookie: zzz=http://127.0.0.1:8001; Path=/
  • forge an request that access 8001 port forever

curl -H "Cookie: zzz=http://127.0.0.1:8001; Path=/" http://127.0.0.1:8000

even without Path

curl -H "Cookie: zzz=http://127.0.0.1:8001" http://127.0.0.1:8000

Do you think we should enhance it?

@emilevauge
Copy link
Contributor

@onestraw there is an issue indeed, but I don't agree with:

Malicious user can access the specific server forever by setting backend server in cookie

The fact that the server IP is visible in the cookie is the issue, as we shouldn't be able to see this info. But even if a hash were used instead of the ip, a user could forge a request to access this specific server, indeed, this is the whole point of sticky session right? But in this case, he couldn't guess other servers' IP.

@onestraw
Copy link
Contributor Author

Yes, we shouldn't disclose backend server to client. And we should also avoid giving client possibility to select backend server.

Cookie is used to map the backend server, simple hash can not solve the problem, LB should has some checking method, check some unique data for each client.

If we hash the combination of client address (ip:port, http.Request.RemoteAddr) and backend server (after LB select), the hash value is used as cookie. Client A access the server, GW generates the cookie related the user's address, which is unique. The second time A access the server, GW lookup the map table or try to combine A's address with each server and compare the hash value, ... Client B cannot use A's Cookie as they have different address.

@emilevauge
Copy link
Contributor

@onestraw I really don't get the issue here. IP is not something we can trust (multiple users can be behind a proxy and have the same IP). Do you have any example of those checks being done on other projects ?

@onestraw
Copy link
Contributor Author

@emilevauge Yes, we cannot trust client IP, it's just increase the difficult to control of accessing backend server. Currently I don't have verified it, but I will work more on this, and give updates.

@emilevauge
Copy link
Contributor

emilevauge commented Dec 15, 2017

@onestraw to be clear, I think it would be a mistake to introduce this check. You should not lose your cookie (and be disconnected) when your IP changes.

@onestraw
Copy link
Contributor Author

onestraw commented Dec 19, 2017

@emilevauge You're right.
I have checked Tengine session sticky module, which is based on Nginx, it has an option using server name's md5 value as cookie. We can hide the server IP at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants