Support TLS for metrics endpointsid-tls-for-metrics-endpoints

Our monitoring endpoints are HTTP by default.

Let's allow configuring TLS in order to provide TLS on these
endpoints

10 files changed, 303 insertions, 20 deletions

diff --git a/cmd/gitlab-sshd/main.go b/cmd/gitlab-sshd/main.go
index 8040a54..bf1fae8 100644
--- a/cmd/gitlab-sshd/main.go
+++ b/cmd/gitlab-sshd/main.go
@@ -12,9 +12,9 @@ import (
 	"gitlab.com/gitlab-org/gitlab-shell/internal/config"
 	"gitlab.com/gitlab-org/gitlab-shell/internal/logger"
 	"gitlab.com/gitlab-org/gitlab-shell/internal/sshd"
+	"gitlab.com/gitlab-org/gitlab-shell/internal/sshd/monitoring"
 
 	"gitlab.com/gitlab-org/labkit/log"
-	"gitlab.com/gitlab-org/labkit/monitoring"
 )
 
 var (
@@ -76,17 +76,10 @@ func main() {
 		log.WithError(err).Fatal("Failed to start GitLab built-in sshd")
 	}
 
-	// Startup monitoring endpoint.
-	if cfg.Server.WebListen != "" {
-		go func() {
-			err := monitoring.Start(
-				monitoring.WithListenerAddress(cfg.Server.WebListen),
-				monitoring.WithBuildInformation(Version, BuildTime),
-				monitoring.WithServeMux(server.MonitoringServeMux()),
-			)
-
-			log.WithError(err).Fatal("monitoring service raised an error")
-		}()
+	listenerConfigFromWebListen := config.ListenerConfig{Addr: cfg.Server.WebListen}
+	webServer := &monitoring.WebServer{ListenerConfigs: append(cfg.Server.WebListeners, listenerConfigFromWebListen)}
+	if err := webServer.Start(Version, BuildTime, server); err != nil {
+		log.WithError(err).Fatal("Failed to start monitoring server")
 	}
 
 	ctx, cancel := context.WithCancel(ctx)
diff --git a/config.yml.example b/config.yml.example
index 579bf3c..4da143a 100644
--- a/config.yml.example
+++ b/config.yml.example
@@ -84,3 +84,13 @@ sshd:
     - /run/secrets/ssh-hostkeys/ssh_host_rsa_key
     - /run/secrets/ssh-hostkeys/ssh_host_ecdsa_key
     - /run/secrets/ssh-hostkeys/ssh_host_ed25519_key
+  # This section configures web listeners for monitoring/health checks
+  # It's an array that contains address and TLS config if necessary
+  # web_listeners:
+  #   -
+  #     addr: "127.0.0.1:9122"
+  #     tls:
+  #       certificate: "/path/to/certificate"
+  #       key: "/path/to/private/key"
+  #     min_version: "tls1.2"
+  #     max_version: "tls1.3"
diff --git a/internal/config/config.go b/internal/config/config.go
index ff0c79a..c83109d 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -21,15 +21,28 @@ const (
 	defaultSecretFileName = ".gitlab_shell_secret"
 )
 
+type TlsConfig struct {
+	Certificate string `yaml:"certificate"`
+	Key         string `yaml:"key"`
+	MinVersion  string `yaml:"min_version"`
+	MaxVersion  string `yaml:"max_version"`
+}
+
+type ListenerConfig struct {
+	Addr string     `yaml:"addr"`
+	Tls  *TlsConfig `yaml:"tls"`
+}
+
 type ServerConfig struct {
-	Listen                  string   `yaml:"listen,omitempty"`
-	ProxyProtocol           bool     `yaml:"proxy_protocol,omitempty"`
-	WebListen               string   `yaml:"web_listen,omitempty"`
-	ConcurrentSessionsLimit int64    `yaml:"concurrent_sessions_limit,omitempty"`
-	GracePeriodSeconds      uint64   `yaml:"grace_period"`
-	ReadinessProbe          string   `yaml:"readiness_probe"`
-	LivenessProbe           string   `yaml:"liveness_probe"`
-	HostKeyFiles            []string `yaml:"host_key_files,omitempty"`
+	Listen                  string           `yaml:"listen,omitempty"`
+	ProxyProtocol           bool             `yaml:"proxy_protocol,omitempty"`
+	WebListen               string           `yaml:"web_listen,omitempty"`
+	ConcurrentSessionsLimit int64            `yaml:"concurrent_sessions_limit,omitempty"`
+	GracePeriodSeconds      uint64           `yaml:"grace_period"`
+	ReadinessProbe          string           `yaml:"readiness_probe"`
+	LivenessProbe           string           `yaml:"liveness_probe"`
+	HostKeyFiles            []string         `yaml:"host_key_files,omitempty"`
+	WebListeners            []ListenerConfig `yaml:"web_listeners"`
 }
 
 type HttpSettingsConfig struct {
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index a929106..ddeaf85 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -2,6 +2,7 @@ package config
 
 import (
 	"os"
+	"path"
 	"testing"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -57,3 +58,24 @@ func TestCustomPrometheusMetrics(t *testing.T) {
 
 	require.Equal(t, expectedMetricNames, actualNames)
 }
+
+func TestNewFromDir(t *testing.T) {
+	testhelper.PrepareTestRootDir(t)
+
+	cfg, err := NewFromDir(testhelper.TestRoot)
+	require.NoError(t, err)
+
+	require.Equal(t, cfg.SecretFilePath, path.Join(testhelper.TestRoot, "gitlab_shell_secret"))
+	require.Equal(t, cfg.Secret, "some.secret.value\n")
+
+	httpsListener, httpListener := cfg.Server.WebListeners[0], cfg.Server.WebListeners[1]
+	require.Equal(t, "127.0.0.1:9122", httpsListener.Addr)
+	require.Equal(t, *httpsListener.Tls, TlsConfig{
+		Certificate: "/path/to/certificate",
+		Key:         "/path/to/key",
+		MinVersion:  "tls1.2",
+		MaxVersion:  "tls1.3",
+	})
+	require.Equal(t, "127.0.0.1:9123", httpListener.Addr)
+	require.Nil(t, httpListener.Tls)
+}
diff --git a/internal/sshd/monitoring/monitoring.go b/internal/sshd/monitoring/monitoring.go
new file mode 100644
index 0000000..2fbe6c2
--- /dev/null
+++ b/internal/sshd/monitoring/monitoring.go
@@ -0,0 +1,89 @@
+package monitoring
+
+import (
+	"crypto/tls"
+	"net"
+
+	"github.com/prometheus/client_golang/prometheus"
+
+	"gitlab.com/gitlab-org/labkit/log"
+	"gitlab.com/gitlab-org/labkit/monitoring"
+
+	"gitlab.com/gitlab-org/gitlab-shell/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/internal/sshd"
+)
+
+var tlsVersions = map[string]uint16{
+	"":       0, // Default value in tls.Config
+	"tls1.0": tls.VersionTLS10,
+	"tls1.1": tls.VersionTLS11,
+	"tls1.2": tls.VersionTLS12,
+	"tls1.3": tls.VersionTLS13,
+}
+
+type WebServer struct {
+	ListenerConfigs []config.ListenerConfig
+
+	listeners []net.Listener
+}
+
+func (w *WebServer) Start(version, buildTime string, sshdServer *sshd.Server) error {
+	var prometheusRegisterer prometheus.Registerer
+	for _, cfg := range w.ListenerConfigs {
+		if cfg.Addr == "" {
+			continue
+		}
+
+		listener, err := w.newListener(cfg)
+		if err != nil {
+			return err
+		}
+
+		monitoringOpts := []monitoring.Option{
+			monitoring.WithListener(listener),
+			monitoring.WithServeMux(sshdServer.MonitoringServeMux()),
+		}
+
+		// We have to cache and reuse Prometheus registerer in order to avoid
+		// duplicate metrics collector registration attempted by Labkit
+		if prometheusRegisterer == nil {
+			monitoringOpts = append(monitoringOpts, monitoring.WithBuildInformation(version, buildTime))
+		} else {
+			monitoringOpts = append(monitoringOpts, monitoring.WithPrometheusRegisterer(prometheusRegisterer))
+		}
+
+		go func() {
+			if err := monitoring.Start(monitoringOpts...); err != nil {
+				log.WithError(err).Fatal("monitoring service raised an error")
+			}
+		}()
+
+		prometheusRegisterer = prometheus.DefaultRegisterer
+		w.listeners = append(w.listeners, listener)
+	}
+
+	return nil
+}
+
+func (w *WebServer) newListener(cfg config.ListenerConfig) (net.Listener, error) {
+	if cfg.Tls == nil {
+		log.WithFields(log.Fields{"address": cfg.Addr}).Infof("Running monitoring server")
+
+		return net.Listen("tcp", cfg.Addr)
+	}
+
+	cert, err := tls.LoadX509KeyPair(cfg.Tls.Certificate, cfg.Tls.Key)
+	if err != nil {
+		return nil, err
+	}
+
+	tlsConfig := &tls.Config{
+		MinVersion:   tlsVersions[cfg.Tls.MinVersion],
+		MaxVersion:   tlsVersions[cfg.Tls.MaxVersion],
+		Certificates: []tls.Certificate{cert},
+	}
+
+	log.WithFields(log.Fields{"address": cfg.Addr}).Infof("Running monitoring server with tls")
+
+	return tls.Listen("tcp", cfg.Addr, tlsConfig)
+}
diff --git a/internal/sshd/monitoring/monitoring_test.go b/internal/sshd/monitoring/monitoring_test.go
new file mode 100644
index 0000000..4f78e63
--- /dev/null
+++ b/internal/sshd/monitoring/monitoring_test.go
@@ -0,0 +1,88 @@
+package monitoring
+
+import (
+	"crypto/tls"
+	"crypto/x509"
+	"net"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"gitlab.com/gitlab-org/gitlab-shell/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/internal/sshd"
+)
+
+const (
+	certFile = "testdata/localhost.crt"
+	keyFile  = "testdata/localhost.key"
+)
+
+func TestRun(t *testing.T) {
+	server := WebServer{
+		ListenerConfigs: []config.ListenerConfig{
+			{
+				Addr: "127.0.0.1:0",
+			},
+			{
+				Addr: "127.0.0.1:0",
+				Tls: &config.TlsConfig{
+					Certificate: certFile,
+					Key:         keyFile,
+				},
+			},
+			{
+				Addr: "",
+			},
+		},
+	}
+
+	sshdServer := &sshd.Server{Config: &config.Config{Server: config.DefaultServerConfig}}
+	require.NoError(t, server.Start("2021-02-16T09:28:07+01:00", "(unknown)", sshdServer))
+
+	require.Len(t, server.listeners, 2)
+
+	for url, client := range buildClients(t, server.listeners) {
+		resp, err := client.Get(url)
+		require.NoError(t, err)
+		require.Equal(t, 200, resp.StatusCode, "get: "+url)
+	}
+}
+
+func buildHttpsClient(t *testing.T) *http.Client {
+	t.Helper()
+
+	client := &http.Client{}
+	certpool := x509.NewCertPool()
+
+	tlsCertificate, err := tls.LoadX509KeyPair(certFile, keyFile)
+	require.NoError(t, err)
+
+	certificate, err := x509.ParseCertificate(tlsCertificate.Certificate[0])
+	require.NoError(t, err)
+
+	certpool.AddCert(certificate)
+	client.Transport = &http.Transport{
+		TLSClientConfig: &tls.Config{
+			RootCAs: certpool,
+		},
+	}
+
+	return client
+}
+
+func buildClients(t *testing.T, listeners []net.Listener) map[string]*http.Client {
+	httpListener, httpsListener := listeners[0], listeners[1]
+	httpsClient := buildHttpsClient(t)
+
+	clients := map[string]*http.Client{}
+
+	clients["http://"+httpListener.Addr().String()+"/metrics"] = http.DefaultClient
+	clients["http://"+httpListener.Addr().String()+"/debug/pprof"] = http.DefaultClient
+	clients["http://"+httpListener.Addr().String()+"/health"] = http.DefaultClient
+	clients["https://"+httpsListener.Addr().String()+"/metrics"] = httpsClient
+	clients["https://"+httpsListener.Addr().String()+"/debug/pprof"] = httpsClient
+	clients["https://"+httpsListener.Addr().String()+"/health"] = httpsClient
+
+	return clients
+}
diff --git a/internal/sshd/monitoring/testdata/localhost.crt b/internal/sshd/monitoring/testdata/localhost.crt
new file mode 100644
index 0000000..bee60e4
--- /dev/null
+++ b/internal/sshd/monitoring/testdata/localhost.crt
@@ -0,0 +1,27 @@
+-----BEGIN CERTIFICATE-----
+MIIEjjCCAvagAwIBAgIQC2au+A/aGQ2Z21O0wVoEwjANBgkqhkiG9w0BAQsFADCB
+pTEeMBwGA1UEChMVbWtjZXJ0IGRldmVsb3BtZW50IENBMT0wOwYDVQQLDDRpZ29y
+ZHJvemRvdkBJZ29ycy1NYWNCb29rLVByby0yLmxvY2FsIChJZ29yIERyb3pkb3Yp
+MUQwQgYDVQQDDDtta2NlcnQgaWdvcmRyb3pkb3ZASWdvcnMtTWFjQm9vay1Qcm8t
+Mi5sb2NhbCAoSWdvciBEcm96ZG92KTAeFw0yMjAzMDcwNDMxMjRaFw0yNDA2MDcw
+NDMxMjRaMGgxJzAlBgNVBAoTHm1rY2VydCBkZXZlbG9wbWVudCBjZXJ0aWZpY2F0
+ZTE9MDsGA1UECww0aWdvcmRyb3pkb3ZASWdvcnMtTWFjQm9vay1Qcm8tMi5sb2Nh
+bCAoSWdvciBEcm96ZG92KTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB
+AMJ8ofGdcnenVRtNGViF4oxPv+CCFA6D2nfsjkJG8kmO6WW7VlbhJYxCMAuyFF1F
+b2UI2rrTFL8Aeq1KxeQzdrb3cpCquVH/UQ00G4ply28XVPRdbIyLQvOThMEeLL6v
+6gb4edL5oZmo/vWhdQxv0NGt282PAEt+bjnbdl28on8WVzmsw/m0nZ2BVWke+oUM
+krfsbyFaZj7aW8w0dNeK25ANy/Ldx55ENRDquphwYHDnpFOQpkHo5nPuoms5j2Sf
+GW3u3hgeFhRrFjqDstU3OKdA4AdHntDjl0gHm35w1m8PXiql/3EpkEMMx5ixQAqM
+cMZ7VVzy0HIjqsjdJZpzjx8CAwEAAaN2MHQwDgYDVR0PAQH/BAQDAgWgMBMGA1Ud
+JQQMMAoGCCsGAQUFBwMBMB8GA1UdIwQYMBaAFKTVZ2JsYLGJOP+UX0AwGO/81Kab
+MCwGA1UdEQQlMCOCCWxvY2FsaG9zdIcEfwAAAYcQAAAAAAAAAAAAAAAAAAAAATAN
+BgkqhkiG9w0BAQsFAAOCAYEAkGntoogSlhukGqTNbTXN9T/gXLtx9afWlgcBEafF
+MYQoJ1DOwXoYCQkMsxE0xWUyLDTpvjzfKkkyQwWzTwcYqRHOKafKYVSvENU5oaDY
+c2nk32SfkcF6bqJ50uBlFMEvKFExU1U+YSJhuEH/iqT9sSd52uwmnB0TJhSOc3J/
+1ZapKM2G71ezi8OyizwlwDJAwQ37CqrYS2slVO6Cy8zJ1l/ZsZ+kxRb+ME0LREI0
+J/rFTo9A6iyuXeBQ2jiRUrC6pmmbUQbVSjROx4RSmWoI/58/VnuZBY9P62OAOgUv
+pukfAbh3SUjN5++m4Py7WjP/y+L2ILPOFtxTY+CQPWQ5Hbff8iMB4NNfutdU1wSS
+CzXT1zWbU12kXod80wkMqWvNb3yU5spqXV6WYhOHiDIyqpPIqp5/i93Ck3Hd6/BQ
+DYlNOQsVHdSjWzNw9UubjpatiFqMK4hvJZE0haoLlmfDeZeqWk9oAuuCibLJGPg4
+TQri+lKgi0e76ynUr1zP1xUR
+-----END CERTIFICATE-----
diff --git a/internal/sshd/monitoring/testdata/localhost.key b/internal/sshd/monitoring/testdata/localhost.key
new file mode 100644
index 0000000..b708582
--- /dev/null
+++ b/internal/sshd/monitoring/testdata/localhost.key
@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDCfKHxnXJ3p1Ub
+TRlYheKMT7/gghQOg9p37I5CRvJJjullu1ZW4SWMQjALshRdRW9lCNq60xS/AHqt
+SsXkM3a293KQqrlR/1ENNBuKZctvF1T0XWyMi0Lzk4TBHiy+r+oG+HnS+aGZqP71
+oXUMb9DRrdvNjwBLfm4523ZdvKJ/Flc5rMP5tJ2dgVVpHvqFDJK37G8hWmY+2lvM
+NHTXituQDcvy3ceeRDUQ6rqYcGBw56RTkKZB6OZz7qJrOY9knxlt7t4YHhYUaxY6
+g7LVNzinQOAHR57Q45dIB5t+cNZvD14qpf9xKZBDDMeYsUAKjHDGe1Vc8tByI6rI
+3SWac48fAgMBAAECggEALuZXNyi8vdYAVAEXp51BsIxavQ0hQQ7S1DCbbagmLU7l
+Qb8XZwQMRfKAG5HqD0P7ROYJuRvF2PmIm9l4Nzuh2SV63yAMaJWlOgXizlEV6cg6
+mGMfFhVPI+XjEZ7xM1rAmMW6uwGv0ppKQXmZ/FHKjYXbh4qAi7QFaLZfqOMgXHzf
+C4nxf0xMzPP7rBnaxAGBRJWC+/UWxd1MVoHRjink4V/Tdy4zu+cEJ+2wuGawp4nz
+dEWYITzXMcBUKmZQHiOm+r58HpWK3mgXpJQBg3WqjR2iNa+ElyoPoGC6zu5Jd8Xg
+mMG2jHPFu+2F4UvymgxbKZqKHqcNjO7WMZRtIRiJgQKBgQDZGXUme0S5Bh8/y1us
+ltEfy4INFYJAejVxPwv7mRLtySqZLkWAPQTaSGgIk/XMTBYS3Ia9XD6Jl3zwo1qF
+R+y3ZkusGmk73o35kBxjc6purDei7CqMzwulbFTsUglDiF9T4X24bv1yK3lP2n8A
+Y6kLsscEC1wIEuwV5HFyQ2S9zwKBgQDlVepMrQ84FxQxN474LakwWLSkwo+6jS37
+61VPUqDUQpE4fGM6+F3fG+9YDMgvOVDneZ0MvzoiDRynbzF7K3k3fIBrYYbTRz7J
+p23BbTninzhrYTE/xd3LuFCZibCXA7nRa0QmYdXG4nUM2jjsjdR5AG7c/qJQDNun
+SXTbfM49sQKBgQCM9Jl6hbiGBTKO4gNAmJ9o7GIhCqEKKg6+23d1QNroZp9w23km
+nPeknjRltWN25MPENUiKc/Tqst/dAcLJHHzWSuXA9Vj0FTjLG0VDURsMRmbNMlci
+G1/tZNvyoAUBwu5Z8OMGt5F46j8WmL+yygI85TOQLavwVhDQ2gTKcnVbQwKBgQC0
+2VCf0KU8xS5eNYLgARn3jyw89VTkduq5S3aFzBIZ8LiWQ7j4yt0z0NKoq8O9QcSk
+FUocwDv2mEJtYwkxKTI46ExY4Zqxx/Aik47AxwKrzIVwYD+3G7DxMtMUkPkZzY1e
+MOmYHvS3FuPZE8lp+dqA5S+HxKF44Pria9HkOAJnsQKBgE853d9sR0DlJtEj64yu
+FX1rCle/UUODClktPgrwuM+xYutxOiEu6HUWHJI2yvWNk4oNL8Xd0IkR9NlwdatU
+E3+WDua+yYAsI9yWYn3+iqp+owNATkEDjWGivt0Onmgttt5kLHzPFCViIcgl32vv
+7V/plCsmgrS98xZHRrriTLvz
+-----END PRIVATE KEY-----
diff --git a/internal/testhelper/testdata/testroot/config.yml b/internal/testhelper/testdata/testroot/config.yml
index e69de29..bc21f9b 100644
--- a/internal/testhelper/testdata/testroot/config.yml
+++ b/internal/testhelper/testdata/testroot/config.yml
@@ -0,0 +1,12 @@
+secret_file: "gitlab_shell_secret"
+sshd:
+  web_listeners:
+    -
+      addr: "127.0.0.1:9122"
+      tls:
+        certificate: "/path/to/certificate"
+        key: "/path/to/key"
+        min_version: "tls1.2"
+        max_version: "tls1.3"
+    -
+      addr: "127.0.0.1:9123"
diff --git a/internal/testhelper/testdata/testroot/gitlab_shell_secret b/internal/testhelper/testdata/testroot/gitlab_shell_secret
new file mode 100644
index 0000000..59fe424
--- /dev/null
+++ b/internal/testhelper/testdata/testroot/gitlab_shell_secret
@@ -0,0 +1 @@
+some.secret.value