Repository: xenolf/lego (go)

Issue:
Allow non-standard CA for validating ACME server HTTPS in development
Hi folks :wave:,

It would be wonderful if Lego was able to easily interact with a [Pebble](https://github.com/letsencrypt/pebble) instance. This is my own preferred way to debug issues with an ACME client and it may prove useful for integration testing now that Lego has the required ACME v2 support. 

The only stumbling block that I'm aware of is that Lego doesn't facilitate interacting with an ACME server that has a HTTPS certificate from a CA that isn't part of the system trust store. This _can_ be worked around by adding the Pebble CA certificate to the system's CA trust store but we [specifically recommend against this](https://github.com/letsencrypt/pebble#avoiding-client-https-errors) because it can easily compromise the security of a developer's system if done carelessly or forgotten about.

Would you be interested in a feature to specify a non-standard CA certificate via command line flag or an environment variable? For instance, Certbot can be made to trust the Pebble CA by using the `REQUESTS_CA_BUNDLE` env var.

It seems like the challenge is primarily in piping a `*x509.CertPool` populated with the provided non-standard CA to the `*http.Transport` instance that is used by the Lego `*http.Client` here:

https://github.com/xenolf/lego/blob/7fedfd1388f016c7ca7ed92a7f2024d06a7e20d8/acme/http.go#L18-L30

I'd be willing to submit a PR to do the work required but wanted to touch base first to see if the feature was interesting to you and whether there were recommendations on how you would like to see the `*x509.CertPool` propagated forward.

Thanks!

Pull request:
ACME HTTP: Allow customizing HTTP client x509.CertPool
This PR updates `acme/http.go` to allow customizing the `*x509.CertPool` used by the `HTTPClient` by specifying the filepath of one or more custom CA certificates via the `LEGO_CA_CERTIFICATES` environment variable.

This allows developers to easily trust a non-standard CA when interacting with an ACME test server (e.g. [Pebble](https://github.com/letsencrypt/pebble)):

```
LEGO_CA_CERTIFICATES=~/go/src/github.com/letsencrypt/pebble/test/certs/pebble.minica.pem \
lego \
  --server https://localhost:14000/dir \
  --email foo@bar.com \
  -d example.com \
  run
```

Resolves #569

Patch:
diff --git a/acme/http.go b/acme/http.go
index f45e7f87..66caa728 100644
--- a/acme/http.go
+++ b/acme/http.go
@@ -1,33 +1,44 @@
 package acme
 
 import (
+	"crypto/tls"
+	"crypto/x509"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"net"
 	"net/http"
+	"os"
 	"runtime"
 	"strings"
 	"time"
 )
 
-// UserAgent (if non-empty) will be tacked onto the User-Agent string in requests.
-var UserAgent string
-
-// HTTPClient is an HTTP client with a reasonable timeout value.
-var HTTPClient = http.Client{
-	Transport: &http.Transport{
-		Proxy: http.ProxyFromEnvironment,
-		Dial: (&net.Dialer{
-			Timeout:   30 * time.Second,
-			KeepAlive: 30 * time.Second,
-		}).Dial,
-		TLSHandshakeTimeout:   15 * time.Second,
-		ResponseHeaderTimeout: 15 * time.Second,
-		ExpectContinueTimeout: 1 * time.Second,
-	},
-}
+var (
+	// UserAgent (if non-empty) will be tacked onto the User-Agent string in requests.
+	UserAgent string
+
+	// HTTPClient is an HTTP client with a reasonable timeout value and
+	// potentially a custom *x509.CertPool based on the caCertificatesEnvVar
+	// environment variable (see the `initCertPool` function)
+	HTTPClient = http.Client{
+		Transport: &http.Transport{
+			Proxy: http.ProxyFromEnvironment,
+			Dial: (&net.Dialer{
+				Timeout:   30 * time.Second,
+				KeepAlive: 30 * time.Second,
+			}).Dial,
+			TLSHandshakeTimeout:   15 * time.Second,
+			ResponseHeaderTimeout: 15 * time.Second,
+			ExpectContinueTimeout: 1 * time.Second,
+			TLSClientConfig: &tls.Config{
+				RootCAs: initCertPool(),
+			},
+		},
+	}
+)
 
 const (
 	// defaultGoUserAgent is the Go HTTP package user agent string. Too
@@ -36,8 +47,36 @@ const (
 
 	// ourUserAgent is the User-Agent of this underlying library package.
 	ourUserAgent = "xenolf-acme"
+
+	// caCertificatesEnvVar is the environment variable name that can be used to
+	// specify the path to PEM encoded CA Certificates that can be used to
+	// authenticate an ACME server with a HTTPS certificate not issued by a CA in
+	// the system-wide trusted root list.
+	caCertificatesEnvVar = "LEGO_CA_CERTIFICATES"
 )
 
+// initCertPool creates a *x509.CertPool populated with the PEM certificates
+// found in the filepath specified in the caCertificatesEnvVar OS environment
+// variable. If the caCertificatesEnvVar is not set then initCertPool will
+// return nil. If there is an error creating a *x509.CertPool from the provided
+// caCertificatesEnvVar value then initCertPool will panic.
+func initCertPool() *x509.CertPool {
+	if customCACertsPath := os.Getenv(caCertificatesEnvVar); customCACertsPath != "" {
+		customCAs, err := ioutil.ReadFile(customCACertsPath)
+		if err != nil {
+			panic(fmt.Sprintf("error reading %s=%q: %v",
+				caCertificatesEnvVar, customCACertsPath, err))
+		}
+		certPool := x509.NewCertPool()
+		if ok := certPool.AppendCertsFromPEM(customCAs); !ok {
+			panic(fmt.Sprintf("error creating x509 cert pool from %s=%q: %v",
+				caCertificatesEnvVar, customCACertsPath, err))
+		}
+		return certPool
+	}
+	return nil
+}
+
 // httpHead performs a HEAD request with a proper User-Agent string.
 // The response body (resp.Body) is already closed when this function returns.
 func httpHead(url string) (resp *http.Response, err error) {


Test patch:
diff --git a/acme/http_test.go b/acme/http_test.go
index 33a48a33..9b88e8a7 100644
--- a/acme/http_test.go
+++ b/acme/http_test.go
@@ -1,8 +1,10 @@
 package acme
 
 import (
+	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"os"
 	"strings"
 	"testing"
 )
@@ -98,3 +100,111 @@ func TestUserAgent(t *testing.T) {
 		t.Errorf("Expected custom UA to contain %s, got '%s'", UserAgent, ua)
 	}
 }
+
+// TestInitCertPool tests the http.go initCertPool function for customizing the
+// HTTP Client *x509.CertPool with an environment variable.
+func TestInitCertPool(t *testing.T) {
+	// writeTemp creates a temp file with the given contents & prefix and returns
+	// the file path. If an error occurs, t.Fatalf is called to end the test run.
+	writeTemp := func(t *testing.T, contents, prefix string) string {
+		t.Helper()
+		tmpFile, err := ioutil.TempFile("", prefix)
+		if err != nil {
+			t.Fatalf("Unable to create tempfile: %v", err)
+		}
+		err = ioutil.WriteFile(tmpFile.Name(), []byte(contents), 0700)
+		if err != nil {
+			t.Fatalf("Unable to write tempfile contents: %v", err)
+		}
+		return tmpFile.Name()
+	}
+
+	invalidFileContents := "not a certificate"
+	invalidFile := writeTemp(t, invalidFileContents, "invalid.pem")
+
+	// validFileContents is lifted from Pebble[0]. Generate your own CA cert with
+	// MiniCA[1].
+	// [0]: https://github.com/letsencrypt/pebble/blob/de6fa233ea1f283eeb9751d42c8e1ae72718c44e/test/certs/pebble.minica.pem
+	// [1]: https://github.com/jsha/minica
+	validFileContents := `
+-----BEGIN CERTIFICATE-----
+MIIDCTCCAfGgAwIBAgIIJOLbes8sTr4wDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE
+AxMVbWluaWNhIHJvb3QgY2EgMjRlMmRiMCAXDTE3MTIwNjE5NDIxMFoYDzIxMTcx
+MjA2MTk0MjEwWjAgMR4wHAYDVQQDExVtaW5pY2Egcm9vdCBjYSAyNGUyZGIwggEi
+MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC5WgZNoVJandj43kkLyU50vzCZ
+alozvdRo3OFiKoDtmqKPNWRNO2hC9AUNxTDJco51Yc42u/WV3fPbbhSznTiOOVtn
+Ajm6iq4I5nZYltGGZetGDOQWr78y2gWY+SG078MuOO2hyDIiKtVc3xiXYA+8Hluu
+9F8KbqSS1h55yxZ9b87eKR+B0zu2ahzBCIHKmKWgc6N13l7aDxxY3D6uq8gtJRU0
+toumyLbdzGcupVvjbjDP11nl07RESDWBLG1/g3ktJvqIa4BWgU2HMh4rND6y8OD3
+Hy3H8MY6CElL+MOCbFJjWqhtOxeFyZZV9q3kYnk9CAuQJKMEGuN4GU6tzhW1AgMB
+AAGjRTBDMA4GA1UdDwEB/wQEAwIChDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB
+BQUHAwIwEgYDVR0TAQH/BAgwBgEB/wIBADANBgkqhkiG9w0BAQsFAAOCAQEAF85v
+d40HK1ouDAtWeO1PbnWfGEmC5Xa478s9ddOd9Clvp2McYzNlAFfM7kdcj6xeiNhF
+WPIfaGAi/QdURSL/6C1KsVDqlFBlTs9zYfh2g0UXGvJtj1maeih7zxFLvet+fqll
+xseM4P9EVJaQxwuK/F78YBt0tCNfivC6JNZMgxKF59h0FBpH70ytUSHXdz7FKwix
+Mfn3qEb9BXSk0Q3prNV5sOV3vgjEtB4THfDxSz9z3+DepVnW3vbbqwEbkXdk3j82
+2muVldgOUgTwK8eT+XdofVdntzU/kzygSAtAQwLJfn51fS1GvEcYGBc1bDryIqmF
+p9BI7gVKtWSZYegicA==
+-----END CERTIFICATE-----
+	`
+	validFile := writeTemp(t, validFileContents, "valid.pem")
+
+	testCases := []struct {
+		Name        string
+		EnvVar      string
+		ExpectPanic bool
+		ExpectNil   bool
+	}{
+		// Setting the env var to a file that doesn't exist should panic
+		{
+			Name:        "Env var with missing file",
+			EnvVar:      "not.a.real.file.pem",
+			ExpectPanic: true,
+		},
+		// Setting the env var to a file that contains invalid content should panic
+		{
+			Name:        "Env var with invalid content",
+			EnvVar:      invalidFile,
+			ExpectPanic: true,
+		},
+		// Setting the env var to the empty string should not panic and should
+		// return nil
+		{
+			Name:        "No env var",
+			EnvVar:      "",
+			ExpectPanic: false,
+			ExpectNil:   true,
+		},
+		// Setting the env var to a file that contains valid content should not
+		// panic and should not return nil
+		{
+			Name:        "Env var with valid content",
+			EnvVar:      validFile,
+			ExpectPanic: false,
+			ExpectNil:   false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Name, func(t *testing.T) {
+			os.Setenv(caCertificatesEnvVar, tc.EnvVar)
+			defer os.Setenv(caCertificatesEnvVar, "")
+
+			defer func() {
+				if r := recover(); r == nil && tc.ExpectPanic {
+					t.Errorf("expected initCertPool() to panic, it did not")
+				} else if r != nil && !tc.ExpectPanic {
+					t.Errorf("expected initCertPool() to not panic, but it did")
+				}
+			}()
+
+			result := initCertPool()
+
+			if result == nil && !tc.ExpectNil {
+				t.Errorf("initCertPool() returned nil, expected non-nil")
+			} else if result != nil && tc.ExpectNil {
+				t.Errorf("initCertPool() returned non-nil, expected nil")
+			}
+		})
+	}
+}