contrib icon indicating copy to clipboard operation
contrib copied to clipboard

How to get client's real ip?

Open bangbaew opened this issue 1 year ago • 18 comments

image

When make a remote request to my Gofiber endpoint, it gives http.client_ip = 10.8.11.189, which is container's local ip, but in Rust version of opentelemetry, used with Actix Web, it gives my real public ip out there, how can I make Gofiber's otel show public client ip?

bangbaew avatar Mar 13 '23 12:03 bangbaew

@bangbaew Is the rust version also running inside Docker?

gaby avatar Mar 13 '23 12:03 gaby

Found the issue. We are using the ClientIP from the context here: https://github.com/gofiber/contrib/blob/main/otelfiber/semconv.go#L59

We need to add support for X-Forwarded-For.

Related issue: https://github.com/open-telemetry/opentelemetry-go/issues/2282

I do think this should probably be fixed in Fiber instead of the middleware. Someone reported a similar issue when using c.IP() a few days ago on discord.

gaby avatar Mar 13 '23 12:03 gaby

@bangbaew Is the rust version also running inside Docker?

It's running inside a container, same network as the Gofiber app. This is the Rust library i use: https://github.com/OutThereLabs/actix-web-opentelemetry

bangbaew avatar Mar 13 '23 13:03 bangbaew

@bangbaew Is the rust version also running inside Docker?

It's running inside a container, same network as the Gofiber app. This is the Rust library i use: https://github.com/OutThereLabs/actix-web-opentelemetry

Yeah, this is a Fiber bug.

gaby avatar Mar 13 '23 13:03 gaby

We can probably solve this by using this: https://docs.gofiber.io/api/ctx#ips

gaby avatar Mar 13 '23 13:03 gaby

@bangbaew Is the rust version also running inside Docker?

It's running inside a container, same network as the Gofiber app. This is the Rust library i use: https://github.com/OutThereLabs/actix-web-opentelemetry

Yeah, this is a Fiber bug.

Yeah, the log IPs on the terminal as well, they all are local IPs, and I don't think they're any useful. image

bangbaew avatar Mar 13 '23 13:03 bangbaew

@bangbaew Is the rust version also running inside Docker?

It's running inside a container, same network as the Gofiber app. This is the Rust library i use: https://github.com/OutThereLabs/actix-web-opentelemetry

Yeah, this is a Fiber bug.

Yeah, the log IPs on the terminal as well, they all are local IPs, and I don't think they're any useful. image

Those are expected since thats your IP inside the container. They only way to get the real IP in the logs is by parsing the Forwarded headers, it should be the first one in the List.

In one of your routes log ctx.IPs()

gaby avatar Mar 13 '23 13:03 gaby

https://github.com/gofiber/contrib/blob/bae3c8cc2db1c8cfcf747203af34f0ffa6a96bd9/otelfiber/semconv.go#L59-L62

https://github.com/gofiber/fiber/blob/634f163e3f6292e658e61d0dd9e3c475d87b5d54/ctx.go#L699-L701

https://docs.gofiber.io/next/api/fiber#config image

did you configure this header ? otherwise the fiber app can not determine the real ip

@gaby maybe we should extend the doc for these cases (ip method)

ReneWerner87 avatar Mar 13 '23 13:03 ReneWerner87

https://github.com/gofiber/fiber/blob/master/ctx_test.go#L1265

ReneWerner87 avatar Mar 13 '23 13:03 ReneWerner87

https://github.com/gofiber/contrib/blob/bae3c8cc2db1c8cfcf747203af34f0ffa6a96bd9/otelfiber/semconv.go#L59-L62

https://github.com/gofiber/fiber/blob/634f163e3f6292e658e61d0dd9e3c475d87b5d54/ctx.go#L699-L701

https://docs.gofiber.io/next/api/fiber#config image

did you configure this header ? otherwise the fiber app can not determine the real ip

@gaby maybe we should extend the doc for these cases (ip method)

Agree, it's a bit confusing. From a otelfiber perspective using c.IPs() may be better since opentelemetry will auto-parse the list and only use the first IP which is the real client IP.

gaby avatar Mar 13 '23 13:03 gaby

@bangbaew have you ever tested what you get when you configure the header of the proxy (mostly forwarded-for ) in your fiber app ?

ReneWerner87 avatar Mar 14 '23 07:03 ReneWerner87

@bangbaew have you ever tested what you get when you configure the header of the proxy (mostly forwarded-for ) in your fiber app ?

If you mean have I tried logging from C.IPs() and c.GetReqHeaders(), I've tried them and the real IPs are shown in the fmt.Println, they both echo the X-Forwarded-For If I send a request over Kong Gateway endpoint, it will log this

"X-Forwarded-For": "{my real public ip}, 10.8.26.4",
"X-Real-Ip": "10.8.26.4"

The 10.8.26.4 is Kong instance's IP.

If I send a request directly, it will log this

"X-Forwarded-For": "{my real public ip}",
"X-Real-Ip": "{my real public ip}"

but both of them will log this in Jaeger UI

http.client_ip 10.8.51.49

You can see that the http.client_ip in Jaeger UI is the fiber instance's local ip, not even the forwarded IPs.

But I don't know how to configure the header of the proxy in my fiber app.

bangbaew avatar Mar 14 '23 07:03 bangbaew

But I don't know how to configure the header of the proxy in my fiber app.

@bangbaew like this

app := fiber.New(fiber.Config{
	ProxyHeader: fiber.HeaderXForwardedFor,
})

https://docs.gofiber.io/next/api/fiber#config image

ReneWerner87 avatar Mar 14 '23 08:03 ReneWerner87

But I don't know how to configure the header of the proxy in my fiber app.

@bangbaew like this

app := fiber.New(fiber.Config{
	ProxyHeader: fiber.HeaderXForwardedFor,
})

https://docs.gofiber.io/next/api/fiber#config image

Thanks a lot! it shows the X-Forwarded-For IPs now, with both public IP and API Gateway's IP, can I make it record only the first value?

bangbaew avatar Mar 14 '23 08:03 bangbaew

do not think so, I would have to research

in any case, we should expand the documentation

@bangbaew you can do that, you know best where you searched for the solution of the problem

maybe in the examples and as a hint in the readme https://github.com/gofiber/contrib/tree/main/otelfiber#readme

ReneWerner87 avatar Mar 14 '23 08:03 ReneWerner87

https://github.com/gofiber/fiber/commit/0dee42a57cd76d7922a753d437894fa214819a63

https://docs.gofiber.io/next/api/ctx#ip

ReneWerner87 avatar Mar 14 '23 10:03 ReneWerner87

@bangbaew opentelemetry says they only take the first value. Has that been the case for you after adding the header?

gaby avatar Mar 15 '23 13:03 gaby

maybe we can change the middleware and cut away the second value which comes back through the header

ReneWerner87 avatar Mar 15 '23 14:03 ReneWerner87