Implement Proxy protocol for the gateway, linksharing, and authservice

[amwolff]

Background

What is the problem/pain point?

Gateway, Link Sharing, and Auth Service don't have the ability to see IP addresses associated with the incoming traffic when they terminate TLS and are behind a load balancer.

Who is impacted?

-

What is the impact?

-

Why now?

-

Requirements

User Story

-

Acceptance Criteria

-

Success Metrics

-

[pwilloughby]

It would be nice to listen on a new additional port for proxy protocol to help with the migration

[storj-gerrit[bot]]

Change pkg/httpserver: support PROXY protocol mentions this issue.

[jewharton]

I've decided to use the pires/go-proxyproto library to add support for PROXY protocol requests.

Dependency review results:

• Check whether licenses are compatible with our project. No GPL, LGPL. storj/uplink shouldn't have a dependency on Apache v2, because it should be GPL compatible. Try to stick to BSD/MIT.
  • The library has an Apache 2.0 license, which is compatible with gateway-mt.
• See whether it easily compiles on all platforms.
  • It compiles easily on Windows and Linux. I'm not sure about macOS since I don't have access to it.
• Does the code expect an existance of an os? e.g. WASM may not have a filesystem.
  • No.
• See whether it uses cgo, unsafe or reflect.
  • The library does not use the cgo or unsafe packages. It only uses reflect in tests.
• Run our linter suite on them. Are there any significant issues that haven't been fixed?
  • In certain circumstances, the library discards errors that occur when attempting to close the connection with a client. [1], [2], [3]
• Read/skim every line in the new dependency:
  • Would you be able to maintain the code?
  • Yes.
  • Would you be able to do a critical fix under a day?
  • It depends on the nature of the issue I'd be fixing. For example, I would be able to quickly patch a nil pointer dereference. However, investigating a situation where the library silently exhibits unexpected behavior would take longer.
  • How idiomatic is the code?
  • Based on my Go experience, I found the code to be idiomatic.
  • Does it have code that is really hard to follow?
  • I didn't consider the code to be very hard to follow.
  • Does it have global variables/state (either private or public)?
  • No, the package does not maintain a global state.
  • Does it use global logging? Is it customizable? (i.e. can you supply different logger for different tests)
  • Yes, logging is customizable.
  • Does the code have comprehensive testsuite?
  • Yes.
  • If it contains parsing, does it have fuzzing?
  • The library contains code for parsing PROXY protocol headers, but no fuzzers are set up for testing this functionality.
  • Does the project have vetters and linters setup for PR-s?
  • Yes.
  • Are they following semantic versioning?
  • Yes.
  • Ideally there are two independent reviewers.
  • At the moment, I am the only reviewer.
• Are there any critical issues or PR-s open?
  • There are no critical, unaddressed issues or PRs open.
• Do the maintainers seem responsive? Are there more than one maintainer?
  • The library has a single maintainer who is moderately responsive, addressing issues and PRs within a few months of their submission.
• Check how much it affects storj/uplink size.
  • N/A

[halkyon]

Here's my review on https://github.com/pires/go-proxyproto. It looks usable to me.

• Check whether licenses are compatible with our project. No GPL, LGPL. storj/uplink shouldn't have a dependency on Apache v2, because it should be GPL compatible. Try to stick to BSD/MIT.
  • Apache 2.0 license, which is fine for storj/edge.
• See whether it easily compiles on all platforms.
  • I ran a binaries build on the change that includes the dependency on all platforms we support and it compiled fine. (make binaries in the storj/edge repo)
• Does the code expect an existance of an os? e.g. WASM may not have a filesystem.
  • No.
• See whether it uses cgo, unsafe or reflect.
  • No use of it. Reflect only in tests.
• Run our linter suite on them. Are there any significant issues that haven't been fixed?
  • There are some errors being ignored, e.g. https://github.com/pires/go-proxyproto/blob/main/v2.go#L169 and https://github.com/pires/go-proxyproto/blob/main/protocol.go#L74, but I don't think it's critical.
• Read/skim every line in the new dependency:
  • Would you be able to maintain the code?
  • Yes.
  • Would you be able to do a critical fix under a day?
  • I think so, it doesn't seem overly complicated.
  • How idiomatic is the code?
  • Looks pretty good to me.
  • Does it have code that is really hard to follow?
  • No.
  • Does it have global variables/state (either private or public)?
  • No, apart from the usual "default timeout" variables, for example: https://github.com/pires/go-proxyproto/blob/main/protocol.go#L16
  • Does it use global logging? Is it customizable? (i.e. can you supply different logger for different tests)
  • No. It's not forcing any particular logging types, either.
  • Does the code have comprehensive testsuite?
  • Yes. It looks comprehensive to me, including checking for specific error types returned.
  • If it contains parsing, does it have fuzzing?
  • It parses out the proxy protocol header from the TCP connection, but no fuzzing.
  • Does the project have vetters and linters setup for PR-s?
  • Yes. It uses golangci-lint, and runs tests with race enabled on CI builds.
  • Are they following semantic versioning?
  • Yes.
  • Ideally there are two independent reviewers.
  • Done. This is the second review.
• Are there any critical issues or PR-s open?
  • No.
• Do the maintainers seem responsive? Are there more than one maintainer?
  • Yes, only one maintainer.
• Check how much it affects storj/uplink size.
  • n/a on uplink, but I don't think it'll make the gateway-mt binary that much bigger than it already is. There's only one direct dependency that it uses, and that's golang.org/x/net (and golang.org/x/text indirectly)

[storj-gerrit[bot]]

Change cmd/{gateway-mt,linksharing}: add config for PROXY protocol address mentions this issue.

[storj-gerrit[bot]]

Change pkg/auth: support PROXY protocol in authservice mentions this issue.