diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-05-04 09:35:21 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-05-04 09:35:42 +0300 |
commit | 6fd80aade3214bad3bb74b8875a94a6b1ee6aec6 (patch) | |
tree | 174931b38919f27c58d6c20deb3e15ad571d9b9b | |
parent | 1707c7b7772461837c83165f6bc73e1c72f542a6 (diff) |
proto: Drop deprecated `http_host` field in FindRemoteRootRef RPC
The `http_host` field was added in order to avoid DNS rebinding attacks.
Callers would send us both the pre-resolved URL and the hostname so that
we would directly connect to the pre-resolved address, but still be able
to set the HTTP `Host` header as expected. This approach was replaced
in favor of the new `resolved_address` field, where Gitaly receives the
un-resolved URL and resolved IP address so that it can perform the
mapping internally.
We have thus deprecated the `http_host` field via cbdc529000 (proto:
Deprecate `http_host` field, 2022-09-30). Remove the field for the
FindRemoteRootRef RPC.
Changelog: removed
-rw-r--r-- | internal/gitaly/service/remote/find_remote_root_ref.go | 7 | ||||
-rw-r--r-- | internal/gitaly/service/remote/find_remote_root_ref_test.go | 20 | ||||
-rw-r--r-- | proto/go/gitalypb/remote.pb.go | 30 | ||||
-rw-r--r-- | proto/remote.proto | 9 |
4 files changed, 12 insertions, 54 deletions
diff --git a/internal/gitaly/service/remote/find_remote_root_ref.go b/internal/gitaly/service/remote/find_remote_root_ref.go index 40fdb2e32..c74d5007d 100644 --- a/internal/gitaly/service/remote/find_remote_root_ref.go +++ b/internal/gitaly/service/remote/find_remote_root_ref.go @@ -37,13 +37,6 @@ func (s *server) findRemoteRootRefCmd(ctx context.Context, request *gitalypb.Fin Value: "Authorization: " + authHeader, }) } - //nolint:staticcheck - if host := request.GetHttpHost(); host != "" { - config = append(config, git.ConfigPair{ - Key: fmt.Sprintf("http.%s.extraHeader", request.RemoteUrl), - Value: "Host: " + host, - }) - } return s.gitCmdFactory.New(ctx, request.Repository, git.Command{ diff --git a/internal/gitaly/service/remote/find_remote_root_ref_test.go b/internal/gitaly/service/remote/find_remote_root_ref_test.go index 196a8fabc..fedc49bea 100644 --- a/internal/gitaly/service/remote/find_remote_root_ref_test.go +++ b/internal/gitaly/service/remote/find_remote_root_ref_test.go @@ -2,7 +2,6 @@ package remote import ( "fmt" - "net/http" "path/filepath" "testing" @@ -81,13 +80,12 @@ func TestFindRemoteRootRef(t *testing.T) { { desc: "successful", setup: func(t *testing.T) setupData { - host := "example.com" secret := "mysecret" _, remoteRepoPath := gittest.CreateRepository(t, ctx, cfg) gittest.WriteCommit(t, cfg, remoteRepoPath, gittest.WithBranch("main")) - port := gittest.HTTPServer(t, ctx, gitCmdFactory, remoteRepoPath, newGitRequestValidationMiddleware(host, secret)) + port := gittest.HTTPServer(t, ctx, gitCmdFactory, remoteRepoPath, nil) originURL := fmt.Sprintf("http://127.0.0.1:%d/%s", port, filepath.Base(remoteRepoPath)) return setupData{ @@ -95,7 +93,6 @@ func TestFindRemoteRootRef(t *testing.T) { Repository: localRepo, RemoteUrl: originURL, HttpAuthorizationHeader: secret, - HttpHost: host, }, expectedResponse: &gitalypb.FindRemoteRootRefResponse{ Ref: "main", @@ -144,21 +141,6 @@ func TestFindRemoteRootRef(t *testing.T) { } } -func newGitRequestValidationMiddleware(host, secret string) func(http.ResponseWriter, *http.Request, http.Handler) { - return func(w http.ResponseWriter, r *http.Request, next http.Handler) { - if r.Host != host { - http.Error(w, "No Host", http.StatusBadRequest) - return - } - if r.Header.Get("Authorization") != secret { - http.Error(w, "Unauthorized", http.StatusUnauthorized) - return - } - - next.ServeHTTP(w, r) - } -} - func TestServer_findRemoteRootRefCmd(t *testing.T) { t.Parallel() diff --git a/proto/go/gitalypb/remote.pb.go b/proto/go/gitalypb/remote.pb.go index 082191a8e..10d95dd97 100644 --- a/proto/go/gitalypb/remote.pb.go +++ b/proto/go/gitalypb/remote.pb.go @@ -299,13 +299,6 @@ type FindRemoteRootRefRequest struct { // HttpAuthorizationHeader is the HTTP header which should be added to the // request in order to authenticate against the repository. HttpAuthorizationHeader string `protobuf:"bytes,4,opt,name=http_authorization_header,json=httpAuthorizationHeader,proto3" json:"http_authorization_header,omitempty"` - // HttpHost is the hostname of the remote repository. Use this when the - // URL hostname has already been resolved to an IP address to prevent DNS - // rebinding. This is deprecated as we will be using resolved_address - // going forward. - // - // Deprecated: Marked as deprecated in remote.proto. - HttpHost string `protobuf:"bytes,5,opt,name=http_host,json=httpHost,proto3" json:"http_host,omitempty"` // ResolvedAddress holds the resolved IP address of the remote_url. This is // used to avoid DNS rebinding by mapping the url to the resolved address. // Only IPv4 dotted decimal ("192.0.2.1"), IPv6 ("2001:db8::68"), or IPv4-mapped @@ -368,14 +361,6 @@ func (x *FindRemoteRootRefRequest) GetHttpAuthorizationHeader() string { return "" } -// Deprecated: Marked as deprecated in remote.proto. -func (x *FindRemoteRootRefRequest) GetHttpHost() string { - if x != nil { - return x.HttpHost - } - return "" -} - func (x *FindRemoteRootRefRequest) GetResolvedAddress() string { if x != nil { return x.ResolvedAddress @@ -571,7 +556,7 @@ var file_remote_proto_rawDesc = []byte{ 0x4e, 0x61, 0x6d, 0x65, 0x22, 0x36, 0x0a, 0x1c, 0x46, 0x69, 0x6e, 0x64, 0x52, 0x65, 0x6d, 0x6f, 0x74, 0x65, 0x52, 0x65, 0x70, 0x6f, 0x73, 0x69, 0x74, 0x6f, 0x72, 0x79, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x16, 0x0a, 0x06, 0x65, 0x78, 0x69, 0x73, 0x74, 0x73, 0x18, 0x01, - 0x20, 0x01, 0x28, 0x08, 0x52, 0x06, 0x65, 0x78, 0x69, 0x73, 0x74, 0x73, 0x22, 0x89, 0x02, 0x0a, + 0x20, 0x01, 0x28, 0x08, 0x52, 0x06, 0x65, 0x78, 0x69, 0x73, 0x74, 0x73, 0x22, 0xf9, 0x01, 0x0a, 0x18, 0x46, 0x69, 0x6e, 0x64, 0x52, 0x65, 0x6d, 0x6f, 0x74, 0x65, 0x52, 0x6f, 0x6f, 0x74, 0x52, 0x65, 0x66, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x38, 0x0a, 0x0a, 0x72, 0x65, 0x70, 0x6f, 0x73, 0x69, 0x74, 0x6f, 0x72, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x12, 0x2e, @@ -582,13 +567,12 @@ var file_remote_proto_rawDesc = []byte{ 0x72, 0x6c, 0x12, 0x3a, 0x0a, 0x19, 0x68, 0x74, 0x74, 0x70, 0x5f, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x7a, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x5f, 0x68, 0x65, 0x61, 0x64, 0x65, 0x72, 0x18, 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, 0x17, 0x68, 0x74, 0x74, 0x70, 0x41, 0x75, 0x74, 0x68, 0x6f, - 0x72, 0x69, 0x7a, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x48, 0x65, 0x61, 0x64, 0x65, 0x72, 0x12, 0x1f, - 0x0a, 0x09, 0x68, 0x74, 0x74, 0x70, 0x5f, 0x68, 0x6f, 0x73, 0x74, 0x18, 0x05, 0x20, 0x01, 0x28, - 0x09, 0x42, 0x02, 0x18, 0x01, 0x52, 0x08, 0x68, 0x74, 0x74, 0x70, 0x48, 0x6f, 0x73, 0x74, 0x12, - 0x29, 0x0a, 0x10, 0x72, 0x65, 0x73, 0x6f, 0x6c, 0x76, 0x65, 0x64, 0x5f, 0x61, 0x64, 0x64, 0x72, - 0x65, 0x73, 0x73, 0x18, 0x06, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0f, 0x72, 0x65, 0x73, 0x6f, 0x6c, - 0x76, 0x65, 0x64, 0x41, 0x64, 0x64, 0x72, 0x65, 0x73, 0x73, 0x4a, 0x04, 0x08, 0x02, 0x10, 0x03, - 0x52, 0x06, 0x72, 0x65, 0x6d, 0x6f, 0x74, 0x65, 0x22, 0x2d, 0x0a, 0x19, 0x46, 0x69, 0x6e, 0x64, + 0x72, 0x69, 0x7a, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x48, 0x65, 0x61, 0x64, 0x65, 0x72, 0x12, 0x29, + 0x0a, 0x10, 0x72, 0x65, 0x73, 0x6f, 0x6c, 0x76, 0x65, 0x64, 0x5f, 0x61, 0x64, 0x64, 0x72, 0x65, + 0x73, 0x73, 0x18, 0x06, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0f, 0x72, 0x65, 0x73, 0x6f, 0x6c, 0x76, + 0x65, 0x64, 0x41, 0x64, 0x64, 0x72, 0x65, 0x73, 0x73, 0x4a, 0x04, 0x08, 0x02, 0x10, 0x03, 0x4a, + 0x04, 0x08, 0x05, 0x10, 0x06, 0x52, 0x06, 0x72, 0x65, 0x6d, 0x6f, 0x74, 0x65, 0x52, 0x09, 0x68, + 0x74, 0x74, 0x70, 0x5f, 0x68, 0x6f, 0x73, 0x74, 0x22, 0x2d, 0x0a, 0x19, 0x46, 0x69, 0x6e, 0x64, 0x52, 0x65, 0x6d, 0x6f, 0x74, 0x65, 0x52, 0x6f, 0x6f, 0x74, 0x52, 0x65, 0x66, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x10, 0x0a, 0x03, 0x72, 0x65, 0x66, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x72, 0x65, 0x66, 0x32, 0xc5, 0x02, 0x0a, 0x0d, 0x52, 0x65, 0x6d, 0x6f, diff --git a/proto/remote.proto b/proto/remote.proto index 4b91efb18..fe8e1fa6a 100644 --- a/proto/remote.proto +++ b/proto/remote.proto @@ -127,11 +127,6 @@ message FindRemoteRootRefRequest { // HttpAuthorizationHeader is the HTTP header which should be added to the // request in order to authenticate against the repository. string http_authorization_header = 4; - // HttpHost is the hostname of the remote repository. Use this when the - // URL hostname has already been resolved to an IP address to prevent DNS - // rebinding. This is deprecated as we will be using resolved_address - // going forward. - string http_host = 5 [deprecated=true]; // ResolvedAddress holds the resolved IP address of the remote_url. This is // used to avoid DNS rebinding by mapping the url to the resolved address. // Only IPv4 dotted decimal ("192.0.2.1"), IPv6 ("2001:db8::68"), or IPv4-mapped @@ -142,6 +137,10 @@ message FindRemoteRootRefRequest { reserved 2; reserved "remote"; + + // HttpHost has been removed in favor of ResolvedAddress. + reserved 5; + reserved "http_host"; } // FindRemoteRootRefResponse represents the response for the FindRemoteRootRef |