kms: require an https endpoint

The client secret travels in the body of the login and the bearer travels in a
header of every read. An endpoint that does not encrypt them is a configuration
to fix, not a degraded mode to run in, so FromEnv refuses one.

The transport tests build their client directly rather than through FromEnv:
what goes over the wire and what an operator configured are two questions, and
a test server on loopback is not an https endpoint.

pkg/kms/kms.go

@@ -76,6 +76,13 @@ func FromEnv() (*Client, error) {
 	if endpoint == "" {
 		return nil, nil
 	}
+	// The credential travels in the body of the login and the bearer travels
+	// in a header of every read, so the endpoint has to be one that encrypts
+	// them. A plaintext endpoint is a configuration to fix, not a degraded mode
+	// to run in.
+	if !strings.HasPrefix(endpoint, "https://") {
+		return nil, fmt.Errorf("kms: %s is %q; the endpoint carries a client secret and must be https", envEndpoint, endpoint)
+	}
 	id := firstOf(os.Getenv(envClientID), os.Getenv("IAM_CLIENT_ID"))
 	secret := firstOf(os.Getenv(envSecret), os.Getenv("IAM_CLIENT_SECRET"))
 	if id == "" || secret == "" {

pkg/kms/kms_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 )
 
 // fakeKMS answers the two calls the client makes, and records what it was
@@ -55,20 +56,40 @@ func (f *fakeKMS) server(t *testing.T) *httptest.Server {
 	return srv
 }
 
+// configured points a client at a test server. It builds the struct rather
+// than going through FromEnv because these tests are about what goes over the
+// wire, and FromEnv is about what an operator configured — including that the
+// endpoint is https, which a test server on loopback is not.
 func configured(t *testing.T, endpoint string) *Client {
 	t.Helper()
-	t.Setenv(envEndpoint, endpoint)
+	return &Client{
+		endpoint: endpoint,
+		clientID: "ingress",
+		secret:   "shhh",
+		org:      "hanzo",
+		env:      "default",
+		path:     "ingress",
+		http:     &http.Client{Timeout: 5 * time.Second},
+	}
+}
+
+// The client secret is in the body of the login and the bearer is in a header
+// of every read, so a plaintext endpoint is a configuration to fix rather than
+// a degraded mode to run in.
+func TestFromEnv_RequiresHTTPS(t *testing.T) {
 	t.Setenv(envClientID, "ingress")
 	t.Setenv(envSecret, "shhh")
-	t.Setenv(envPath, "ingress")
+
+	t.Setenv(envEndpoint, "http://kms.hanzo.ai")
+	if _, err := FromEnv(); err == nil {
+		t.Error("FromEnv accepted a plaintext endpoint")
+	}
+
+	t.Setenv(envEndpoint, "https://kms.hanzo.ai")
 	c, err := FromEnv()
-	if err != nil {
-		t.Fatal(err)
+	if err != nil || c == nil {
+		t.Errorf("FromEnv(https) = (%v, %v), want a client", c, err)
 	}
-	if c == nil {
-		t.Fatal("FromEnv returned no client for a configured endpoint")
-	}
-	return c
 }
 
 func TestClient_Get(t *testing.T) {