Skip to content

feat: Support x-forwarded-for headers for IPs #4684

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 23, 2022
Merged

feat: Support x-forwarded-for headers for IPs #4684

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26ed7c9
feat: Support x-forwarded-for headers for IPs
kylecarbs Oct 21, 2022
39f35f5
Fix realip accepting headers
kylecarbs Oct 23, 2022
a8976d7
Merge branch 'main' into realip
kylecarbs Oct 23, 2022
f8d3839
Fix unused headers
kylecarbs Oct 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions cli/deployment/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,18 @@ func Flags() *codersdk.DeploymentFlags {
Description: "Scopes to grant when authenticating with OIDC.",
Default: []string{oidc.ScopeOpenID, "profile", "email"},
},
ProxyTrustedHeaders: &codersdk.StringArrayFlag{
Name: "Trusted HTTP Proxy Headers",
Flag: "proxy-trusted-headers",
EnvVar: "CODER_PROXY_TRUSTED_HEADERS",
Description: "Headers to trust for forwarding IP addresses. e.g. \"X-Forwarded-for\"",
},
ProxyTrustedOrigins: &codersdk.StringArrayFlag{
Name: "Trusted HTTP Proxy Origins",
Flag: "proxy-trusted-origins",
EnvVar: "CODER_PROXY_TRUSTED_ORIGINS",
Description: "Origin addresses to respect \"proxy-trusted-headers\".",
},
TelemetryEnable: &codersdk.BoolFlag{
Name: "Telemetry Enabled",
Flag: "telemetry",
Expand Down
7 changes: 7 additions & 0 deletions cli/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import (
"github.com/coder/coder/coderd/devtunnel"
"github.com/coder/coder/coderd/gitsshkey"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
"github.com/coder/coder/coderd/prometheusmetrics"
"github.com/coder/coder/coderd/telemetry"
"github.com/coder/coder/coderd/tracing"
Expand Down Expand Up @@ -321,6 +322,11 @@ func Server(dflags *codersdk.DeploymentFlags, newAPI func(context.Context, *code
}
}

realIPConfig, err := httpmw.ParseRealIPConfig(dflags.ProxyTrustedHeaders.Value, dflags.ProxyTrustedOrigins.Value)
if err != nil {
return xerrors.Errorf("parse real ip config: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: will a use know what real ip config means here? wondering if we can make that friendlier

}

options := &coderd.Options{
AccessURL: accessURLParsed,
AppHostname: appHostname,
Expand All @@ -332,6 +338,7 @@ func Server(dflags *codersdk.DeploymentFlags, newAPI func(context.Context, *code
CacheDir: dflags.CacheDir.Value,
GoogleTokenValidator: googleTokenValidator,
SecureAuthCookie: dflags.SecureAuthCookie.Value,
RealIPConfig: realIPConfig,
SSHKeygenAlgorithm: sshKeygenAlgorithm,
TracerProvider: tracerProvider,
Telemetry: telemetry.NewNoop(),
Expand Down
3 changes: 1 addition & 2 deletions coderd/apikey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,7 @@ func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*h
}
}

host, _, _ := net.SplitHostPort(params.RemoteAddr)
ip := net.ParseIP(host)
ip := net.ParseIP(params.RemoteAddr)
if ip == nil {
ip = net.IPv4(0, 0, 0, 0)
}
Expand Down
3 changes: 1 addition & 2 deletions coderd/audit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
return
}

ipRaw, _, _ := net.SplitHostPort(r.RemoteAddr)
ip := net.ParseIP(ipRaw)
ip := net.ParseIP(r.RemoteAddr)
ipNet := pqtype.Inet{}
if ip != nil {
ipNet = pqtype.Inet{
Expand Down
20 changes: 4 additions & 16 deletions coderd/audit/request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,8 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
}
}

ip, err := parseIP(p.Request.RemoteAddr)
if err != nil {
p.Log.Warn(logCtx, "parse ip", slog.Error(err))
}

err = p.Audit.Export(ctx, database.AuditLog{
ip := parseIP(p.Request.RemoteAddr)
err := p.Audit.Export(ctx, database.AuditLog{
ID: uuid.New(),
Time: database.Now(),
UserID: httpmw.APIKey(p.Request).UserID,
Expand Down Expand Up @@ -166,16 +162,8 @@ func either[T Auditable, R any](old, new T, fn func(T) R) R {
}
}

func parseIP(ipStr string) (pqtype.Inet, error) {
var err error

ipStr, _, err = net.SplitHostPort(ipStr)
if err != nil {
return pqtype.Inet{}, err
}

func parseIP(ipStr string) pqtype.Inet {
ip := net.ParseIP(ipStr)

ipNet := net.IPNet{}
if ip != nil {
ipNet = net.IPNet{
Expand All @@ -187,5 +175,5 @@ func parseIP(ipStr string) (pqtype.Inet, error) {
return pqtype.Inet{
IPNet: ipNet,
Valid: ip != nil,
}, nil
}
}
2 changes: 2 additions & 0 deletions coderd/coderd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ type Options struct {
Telemetry telemetry.Reporter
TracerProvider trace.TracerProvider
AutoImportTemplates []AutoImportTemplate
RealIPConfig *httpmw.RealIPConfig

// TLSCertificates is used to mesh DERP servers securely.
TLSCertificates []tls.Certificate
Expand Down Expand Up @@ -198,6 +199,7 @@ func New(options *Options) *API {
r.Use(
httpmw.AttachRequestID,
httpmw.Recover(api.Logger),
httpmw.ExtractRealIP(api.RealIPConfig),
httpmw.Logger(api.Logger),
httpmw.Prometheus(options.PrometheusRegistry),
// handleSubdomainApplications checks if the first subdomain is a valid
Expand Down
3 changes: 3 additions & 0 deletions coderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import (
"github.com/coder/coder/coderd/database/dbtestutil"
"github.com/coder/coder/coderd/gitsshkey"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/coderd/telemetry"
"github.com/coder/coder/coderd/util/ptr"
Expand All @@ -77,6 +78,7 @@ type Options struct {
Experimental bool
AzureCertificates x509.VerifyOptions
GithubOAuth2Config *coderd.GithubOAuth2Config
RealIPConfig *httpmw.RealIPConfig
OIDCConfig *coderd.OIDCConfig
GoogleTokenValidator *idtoken.Validator
SSHKeygenAlgorithm gitsshkey.Algorithm
Expand Down Expand Up @@ -238,6 +240,7 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
AWSCertificates: options.AWSCertificates,
AzureCertificates: options.AzureCertificates,
GithubOAuth2Config: options.GithubOAuth2Config,
RealIPConfig: options.RealIPConfig,
OIDCConfig: options.OIDCConfig,
GoogleTokenValidator: options.GoogleTokenValidator,
SSHKeygenAlgorithm: options.SSHKeygenAlgorithm,
Expand Down
3 changes: 1 addition & 2 deletions coderd/httpmw/apikey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
// Only update LastUsed once an hour to prevent database spam.
if now.Sub(key.LastUsed) > time.Hour {
key.LastUsed = now
host, _, _ := net.SplitHostPort(r.RemoteAddr)
remoteIP := net.ParseIP(host)
remoteIP := net.ParseIP(r.RemoteAddr)
if remoteIP == nil {
remoteIP = net.IPv4(0, 0, 0, 0)
}
Expand Down
2 changes: 1 addition & 1 deletion coderd/httpmw/apikey_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ func TestAPIKey(t *testing.T) {
rw = httptest.NewRecorder()
user = createUser(r.Context(), t, db)
)
r.RemoteAddr = "1.1.1.1:3555"
r.RemoteAddr = "1.1.1.1"
r.Header.Set(codersdk.SessionCustomHeader, fmt.Sprintf("%s-%s", id, secret))

_, err := db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{
Expand Down
Loading