Merge branch 'jv-update-auth-documentation' into 'master'

Clarify how v2 auth works

See merge request gitlab-org/gitaly!1846

6 files changed, 57 insertions, 34 deletions

diff --git a/auth/README.md b/auth/README.md
new file mode 100644
index 000000000..b5dc6f568
--- /dev/null
+++ b/auth/README.md
@@ -0,0 +1,27 @@
+# Gitaly authentication middleware for Go
+
+This package contains code that plugs into
+github.com/grpc-ecosystem/go-grpc-middleware/auth to provide client
+and server authentication middleware for Gitaly.
+
+Gitaly has two authentication schemes.
+
+## V1 authentication (deprecated)
+
+This scheme uses a shared secret. The shared secret is base64-encoded
+and passed by the client as a bearer token.
+
+## V2 authentication
+
+This scheme uses a time limited token derived from a shared secret.
+
+The client creates a timestamp and computes the SHA256 HMAC signature
+for that timestamp, treating the timestamp as the message. The shared
+secret is used as the key for the HMAC. The client then sends both the
+message and the signature to the server as a bearer token.
+
+The server takes the message and computes the signature. If the
+client-provided signature matches the computed signature the message is
+accepted. Next, the server checks if its current time is no more than
+30 seconds ahead or behind the timestamp. If the timestamp is too old
+or too new the request is denied. Otherwise it goes ahead.
diff --git a/auth/rpccredentials.go b/auth/rpccredentials.go
index ca54901da..fc39a620b 100644
--- a/auth/rpccredentials.go
+++ b/auth/rpccredentials.go
@@ -11,46 +11,42 @@ import (
 )
 
 // RPCCredentials can be used with grpc.WithPerRPCCredentials to create a
-// grpc.DialOption that inserts the supplied token for authentication
-// with a Gitaly server.
-func RPCCredentials(token string) credentials.PerRPCCredentials {
-	return &rpcCredentials{token: base64.StdEncoding.EncodeToString([]byte(token))}
+// grpc.DialOption that uses v1 Gitaly authentication for authentication
+// with a Gitaly server. The shared secret must match the one used on the
+// Gitaly server.
+func RPCCredentials(sharedSecret string) credentials.PerRPCCredentials {
+	return &rpcCredentials{sharedSecret: base64.StdEncoding.EncodeToString([]byte(sharedSecret))}
 }
 
 type rpcCredentials struct {
-	token string
+	sharedSecret string
 }
 
 func (*rpcCredentials) RequireTransportSecurity() bool { return false }
 
 func (rc *rpcCredentials) GetRequestMetadata(context.Context, ...string) (map[string]string, error) {
-	return map[string]string{"authorization": "Bearer " + rc.token}, nil
+	return map[string]string{"authorization": "Bearer " + rc.sharedSecret}, nil
 }
 
-// RPCCredentialsV2 can be used with grpc.WithPerRPCCredentials to create a
-// grpc.DialOption that inserts an HMAC token with the current timestamp
-// for authentication with a Gitaly server.
-func RPCCredentialsV2(token string) credentials.PerRPCCredentials {
-	return &rpcCredentialsV2{token: token}
+// RPCCredentialsV2 can be used with grpc.WithPerRPCCredentials to create
+// a grpc.DialOption that inserts an V2 (HMAC) token with the current
+// timestamp for authentication with a Gitaly server. The shared secret
+// must match the one used on the Gitaly server.
+func RPCCredentialsV2(sharedSecret string) credentials.PerRPCCredentials {
+	return &rpcCredentialsV2{sharedSecret: sharedSecret}
 }
 
 type rpcCredentialsV2 struct {
-	token string
+	sharedSecret string
 }
 
 func (*rpcCredentialsV2) RequireTransportSecurity() bool { return false }
 
-func (rc *rpcCredentialsV2) GetRequestMetadata(context.Context, ...string) (map[string]string, error) {
-	return map[string]string{"authorization": "Bearer " + rc.hmacToken()}, nil
-}
-
-func (rc *rpcCredentialsV2) hmacToken() string {
-	return hmacToken("v2", []byte(rc.token), time.Now())
-}
-
-func hmacToken(version string, secret []byte, timestamp time.Time) string {
-	intTime := timestamp.Unix()
-	signedTimestamp := hmacSign(secret, strconv.FormatInt(intTime, 10))
+func (rc2 *rpcCredentialsV2) GetRequestMetadata(context.Context, ...string) (map[string]string, error) {
+	message := strconv.FormatInt(time.Now().Unix(), 10)
+	signature := hmacSign([]byte(rc2.sharedSecret), message)
 
-	return fmt.Sprintf("%s.%x.%d", version, signedTimestamp, intTime)
+	return map[string]string{
+		"authorization": "Bearer " + fmt.Sprintf("v2.%x.%s", signature, message),
+	}, nil
 }
diff --git a/ruby/lib/gitaly_server/client.rb b/ruby/lib/gitaly_server/client.rb
index 16e86302f..49fab89ce 100644
--- a/ruby/lib/gitaly_server/client.rb
+++ b/ruby/lib/gitaly_server/client.rb
@@ -9,7 +9,7 @@ module GitalyServer
       @servers = encoded_servers.present? ? JSON.parse(Base64.strict_decode64(encoded_servers)) : {}
     end
 
-    def token(storage)
+    def shared_secret(storage)
       server(storage)['token']
     end
 
diff --git a/ruby/lib/gitlab/git/gitaly_remote_repository.rb b/ruby/lib/gitlab/git/gitaly_remote_repository.rb
index cba4132c7..9ed5b750c 100644
--- a/ruby/lib/gitlab/git/gitaly_remote_repository.rb
+++ b/ruby/lib/gitlab/git/gitaly_remote_repository.rb
@@ -87,8 +87,8 @@ module Gitlab
         addr
       end
 
-      def token
-        gitaly_client.token(storage).to_s
+      def shared_secret
+        gitaly_client.shared_secret(storage).to_s
       end
 
       def request_kwargs
@@ -104,7 +104,7 @@ module Gitlab
 
       def authorization_token
         issued_at = Time.now.to_i.to_s
-        hmac = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA256.new, token, issued_at)
+        hmac = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA256.new, shared_secret, issued_at)
 
         "v2.#{hmac}.#{issued_at}"
       end
diff --git a/ruby/lib/gitlab/git/remote_repository.rb b/ruby/lib/gitlab/git/remote_repository.rb
index 721753399..50f07ad94 100644
--- a/ruby/lib/gitlab/git/remote_repository.rb
+++ b/ruby/lib/gitlab/git/remote_repository.rb
@@ -38,7 +38,7 @@ module Gitlab
       def fetch_env(git_config_options: [])
         gitaly_ssh = File.absolute_path(File.join(Gitlab.config.gitaly.bin_dir, 'gitaly-ssh'))
         gitaly_address = gitaly_client.address(storage)
-        gitaly_token = gitaly_client.token(storage)
+        shared_secret = gitaly_client.shared_secret(storage)
 
         request = Gitaly::SSHUploadPackRequest.new(repository: gitaly_repository, git_config_options: git_config_options)
         env = {
@@ -47,7 +47,7 @@ module Gitlab
           'GITALY_WD' => Dir.pwd,
           'GIT_SSH_COMMAND' => "#{gitaly_ssh} upload-pack"
         }
-        env['GITALY_TOKEN'] = gitaly_token if gitaly_token.present?
+        env['GITALY_TOKEN'] = shared_secret if shared_secret.present?
 
         env
       end
diff --git a/ruby/spec/lib/gitlab/git/remote_repository_spec.rb b/ruby/spec/lib/gitlab/git/remote_repository_spec.rb
index df9468594..325ec5205 100644
--- a/ruby/spec/lib/gitlab/git/remote_repository_spec.rb
+++ b/ruby/spec/lib/gitlab/git/remote_repository_spec.rb
@@ -65,7 +65,7 @@ describe Gitlab::Git::RemoteRepository do
 
     let(:gitaly_client) { double(:gitaly_client) }
     let(:address) { 'fake-address' }
-    let(:token) { 'fake-token' }
+    let(:shared_secret) { 'fake-secret' }
 
     subject { remote_repository.fetch_env }
 
@@ -73,12 +73,12 @@ describe Gitlab::Git::RemoteRepository do
       allow(remote_repository).to receive(:gitaly_client).and_return(gitaly_client)
 
       expect(gitaly_client).to receive(:address).with(repository.storage).and_return(address)
-      expect(gitaly_client).to receive(:token).with(repository.storage).and_return(token)
+      expect(gitaly_client).to receive(:shared_secret).with(repository.storage).and_return(shared_secret)
     end
 
     it { expect(subject).to be_a(Hash) }
     it { expect(subject['GITALY_ADDRESS']).to eq(address) }
-    it { expect(subject['GITALY_TOKEN']).to eq(token) }
+    it { expect(subject['GITALY_TOKEN']).to eq(shared_secret) }
     it { expect(subject['GITALY_WD']).to eq(Dir.pwd) }
 
     it 'creates a plausible GIT_SSH_COMMAND' do
@@ -95,7 +95,7 @@ describe Gitlab::Git::RemoteRepository do
     end
 
     context 'when the token is blank' do
-      let(:token) { '' }
+      let(:shared_secret) { '' }
 
       it { expect(subject.keys).not_to include('GITALY_TOKEN') }
     end