Skip to content
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.

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

command/agent/template: pass along TLSServerName #9401

Closed
wants to merge 1 commit into from
Closed

command/agent/template: pass along TLSServerName #9401

wants to merge 1 commit into from

Conversation

danp
Copy link

@danp danp commented Jul 5, 2020

When preparing the consul-template Vault config, the provided
TLSServerName (in the Vault agent config as tls_server_name) was not
being passed along to the consul-template Vault config.

Using the VAULT_TLS_SERVER_NAME environment variable was working since
the consul-template config building does consider it.

To fix that, pass it along when setting up the consul-template
config. Add a test which verifies basic TLS connectivity to Vault,
including passing along the server name.

Fixes #9183

When preparing the consul-template Vault config, the provided
TLSServerName (in the Vault agent config as tls_server_name) was not
being passed along to the consul-template Vault config.

Using the VAULT_TLS_SERVER_NAME environment variable was working since
the consul-template config building does consider it.

To fix that, pass it along when setting up the consul-template
config. Add a test which verifies basic TLS connectivity to Vault,
including passing along the server name.

Fixes #9183
@hashicorp-cla
Copy link

hashicorp-cla commented Jul 5, 2020

CLA assistant check
All committers have signed the CLA.

@danp
Copy link
Author

danp commented Apr 3, 2021

Capturing patch here if anyone wants to carry on this PR:

the patch from this PR
From 29a62b1c063df40f5c01e5a0379e18c5ac8dd684 Mon Sep 17 00:00:00 2001
From: Dan Peterson <[email protected]>
Date: Sun, 5 Jul 2020 19:44:30 -0300
Subject: [PATCH] command/agent/template: pass along TLSServerName

When preparing the consul-template Vault config, the provided
TLSServerName (in the Vault agent config as tls_server_name) was not
being passed along to the consul-template Vault config.

Using the VAULT_TLS_SERVER_NAME environment variable was working since
the consul-template config building does consider it.

To fix that, pass it along when setting up the consul-template
config. Add a test which verifies basic TLS connectivity to Vault,
including passing along the server name.

Fixes https://github.com/hashicorp/vault/issues/9183
---
 command/agent/template/template.go      | 13 ++---
 command/agent/template/template_test.go | 64 +++++++++++++++++++++++--
 2 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/command/agent/template/template.go b/command/agent/template/template.go
index 3ad82050452..2af3a353dc8 100644
--- a/command/agent/template/template.go
+++ b/command/agent/template/template.go
@@ -221,12 +221,13 @@ func newRunnerConfig(sc *ServerConfig, templates ctconfig.TemplateConfigs) (*ctc
 		skipVerify := sc.VaultConf.TLSSkipVerify
 		verify := !skipVerify
 		conf.Vault.SSL = &ctconfig.SSLConfig{
-			Enabled: pointerutil.BoolPtr(true),
-			Verify:  &verify,
-			Cert:    &sc.VaultConf.ClientCert,
-			Key:     &sc.VaultConf.ClientKey,
-			CaCert:  &sc.VaultConf.CACert,
-			CaPath:  &sc.VaultConf.CAPath,
+			Enabled:    pointerutil.BoolPtr(true),
+			Verify:     &verify,
+			Cert:       &sc.VaultConf.ClientCert,
+			Key:        &sc.VaultConf.ClientKey,
+			CaCert:     &sc.VaultConf.CACert,
+			CaPath:     &sc.VaultConf.CAPath,
+			ServerName: &sc.VaultConf.TLSServerName,
 		}
 	}
 
diff --git a/command/agent/template/template_test.go b/command/agent/template/template_test.go
index 9d71bcc547b..c0de3273ded 100644
--- a/command/agent/template/template_test.go
+++ b/command/agent/template/template_test.go
@@ -3,11 +3,13 @@ package template
 import (
 	"context"
 	"encoding/json"
+	"encoding/pem"
 	"fmt"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"path/filepath"
 	"testing"
 
 	ctconfig "github.com/hashicorp/consul-template/config"
@@ -122,11 +124,7 @@ func TestServerRun(t *testing.T) {
 				ExitAfterAuth: true,
 			}
 
-			var server *Server
-			server = NewServer(&sc)
-			if ts == nil {
-				t.Fatal("nil server returned")
-			}
+			server := NewServer(&sc)
 
 			go server.Run(ctx, templateTokenCh, templatesToRender)
 
@@ -162,6 +160,62 @@ func TestServerRun(t *testing.T) {
 	}
 }
 
+func TestServerRunTLS(t *testing.T) {
+	tmpDir, err := ioutil.TempDir("", "agent-tests")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	var gotTLSServerName string
+	ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotTLSServerName = r.TLS.ServerName
+		fmt.Fprintln(w, jsonResponse)
+	}))
+	defer ts.Close()
+
+	templateTokenCh := make(chan string, 1)
+	templatesToRender := []*ctconfig.TemplateConfig{
+		{
+			Destination: pointerutil.StringPtr(filepath.Join(tmpDir, "render_01")),
+			Contents:    pointerutil.StringPtr(templateContents),
+		},
+	}
+
+	caCert := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: ts.Certificate().Raw})
+	caCertFile := filepath.Join(tmpDir, "ca.pem")
+	if err := ioutil.WriteFile(caCertFile, caCert, 0600); err != nil {
+		t.Fatal(err)
+	}
+
+	sc := ServerConfig{
+		Logger: logging.NewVaultLogger(hclog.Trace),
+		VaultConf: &config.Vault{
+			CACert:  caCertFile,
+			Address: ts.URL,
+			// example.com is a name on the net/http localhost cert, see
+			// https://github.com/golang/go/blob/go1.14/src/net/http/internal/testcert.go#L12
+			TLSServerName: "example.com",
+		},
+		LogLevel:      hclog.Trace,
+		LogWriter:     hclog.DefaultOutput,
+		ExitAfterAuth: true,
+	}
+
+	server := NewServer(&sc)
+
+	go server.Run(context.Background(), templateTokenCh, templatesToRender)
+
+	// send a dummy value to trigger the internal Runner to query for secret
+	// info
+	templateTokenCh <- "test"
+	<-server.DoneCh
+
+	if want := "example.com"; gotTLSServerName != want {
+		t.Errorf("got request TLS ServerName %q, want %q", gotTLSServerName, want)
+	}
+}
+
 func handleRequest(w http.ResponseWriter, r *http.Request) {
 	fmt.Fprintln(w, jsonResponse)
 }

@danp danp deleted the template-tls-server-name branch April 3, 2021 13:03
@pbar1
Copy link
Contributor

pbar1 commented Apr 6, 2021

Capturing patch here if anyone wants to carry on this PR:
the patch from this PR

I can take it on, I'll open a PR using that patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vault agent's template do not respect tls_server_name
3 participants