config+lnd+cert: add --tlsdisableautofill to prevent information leaks.

This adds in a new boolean flag that when set, prevents LND from writing the system hostname and network interface IPs to the TLS certificate. This will ensure privacy for those that don't want private IP addresses to be exposed on a public facing LND node.
2020-06-26 12:53:05 -05:00 · 2020-06-26 12:53:05 -05:00 · 86a0609ecf
commit 86a0609ecf
parent cfbc365c20
6 changed files with 122 additions and 32 deletions
--- a/cert/go.mod
+++ b/cert/go.mod
@ -1,3 +1,5 @@
 module github.com/lightningnetwork/lnd/cert

 go 1.13
+
+require github.com/stretchr/testify v1.5.1
--- a/cert/go.sum
+++ b/cert/go.sum
@ -0,0 +1,11 @@
+github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
+github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4=
+github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
+gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
--- a/cert/selfsigned.go
+++ b/cert/selfsigned.go
@ -32,8 +32,9 @@ var (
 )

 // ipAddresses returns the parserd IP addresses to use when creating the TLS
-// certificate.
-func ipAddresses(tlsExtraIPs []string) ([]net.IP, error) {
+// certificate. If tlsDisableAutofill is true, we don't include interface
+// addresses to protect users privacy.
+func ipAddresses(tlsExtraIPs []string, tlsDisableAutofill bool) ([]net.IP, error) {
 	// Collect the host's IP addresses, including loopback, in a slice.
 	ipAddresses := []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("::1")}

@ -47,15 +48,20 @@ func ipAddresses(tlsExtraIPs []string) ([]net.IP, error) {
 		ipAddresses = append(ipAddresses, ipAddr)
 	}

-	// Add all the interface IPs that aren't already in the slice.
-	addrs, err := net.InterfaceAddrs()
-	if err != nil {
-		return nil, err
-	}
-	for _, a := range addrs {
-		ipAddr, _, err := net.ParseCIDR(a.String())
-		if err == nil {
-			addIP(ipAddr)
+	// To protect their privacy, some users might not want to have all
+	// their network addresses include in the certificate as this could
+	// leak sensitive information.
+	if !tlsDisableAutofill {
+		// Add all the interface IPs that aren't already in the slice.
+		addrs, err := net.InterfaceAddrs()
+		if err != nil {
+			return nil, err
+		}
+		for _, a := range addrs {
+			ipAddr, _, err := net.ParseCIDR(a.String())
+			if err == nil {
+				addIP(ipAddr)
+			}
 		}
 	}

@ -72,10 +78,14 @@ func ipAddresses(tlsExtraIPs []string) ([]net.IP, error) {

 // dnsNames returns the host and DNS names to use when creating the TLS
 // ceftificate.
-func dnsNames(tlsExtraDomains []string) (string, []string) {
+func dnsNames(tlsExtraDomains []string, tlsDisableAutofill bool) (string, []string) {
 	// Collect the host's names into a slice.
 	host, err := os.Hostname()
-	if err != nil {
+
+	// To further protect their privacy, some users might not want
+	// to have their hostname include in the certificate as this could
+	// leak sensitive information.
+	if err != nil || tlsDisableAutofill {
 		// Nothing much we can do here, other than falling back to
 		// localhost as fallback. A hostname can still be provided with
 		// the tlsExtraDomain parameter if the problem persists on a
@ -89,6 +99,13 @@ func dnsNames(tlsExtraDomains []string) (string, []string) {
 	}
 	dnsNames = append(dnsNames, tlsExtraDomains...)

+	// Because we aren't including the hostname in the certificate when
+	// tlsDisableAutofill is set, we will use the first extra domain
+	// specified by the user, if it's set, as the Common Name.
+	if tlsDisableAutofill && len(tlsExtraDomains) > 0 {
+		host = tlsExtraDomains[0]
+	}
+
 	// Also add fake hostnames for unix sockets, otherwise hostname
 	// verification will fail in the client.
 	dnsNames = append(dnsNames, "unix", "unixpacket")
@ -104,10 +121,10 @@ func dnsNames(tlsExtraDomains []string) (string, []string) {
 // and domains given. The certificate is considered up to date if it was
 // created with _exactly_ the IPs and domains given.
 func IsOutdated(cert *x509.Certificate, tlsExtraIPs,
-	tlsExtraDomains []string) (bool, error) {
+	tlsExtraDomains []string, tlsDisableAutofill bool) (bool, error) {

 	// Parse the slice of IP strings.
-	ips, err := ipAddresses(tlsExtraIPs)
+	ips, err := ipAddresses(tlsExtraIPs, tlsDisableAutofill)
 	if err != nil {
 		return false, err
 	}
@ -147,7 +164,7 @@ func IsOutdated(cert *x509.Certificate, tlsExtraIPs,
 	}

 	// Get the full list of DNS names to use.
-	_, dnsNames := dnsNames(tlsExtraDomains)
+	_, dnsNames := dnsNames(tlsExtraDomains, tlsDisableAutofill)

 	// We do the same kind of deduplication for the DNS names.
 	dns1 := make(map[string]struct{})
@ -186,7 +203,8 @@ func IsOutdated(cert *x509.Certificate, tlsExtraIPs,
 // This function is adapted from https://github.com/btcsuite/btcd and
 // https://github.com/btcsuite/btcutil
 func GenCertPair(org, certFile, keyFile string, tlsExtraIPs,
-	tlsExtraDomains []string, certValidity time.Duration) error {
+	tlsExtraDomains []string, tlsDisableAutofill bool,
+	certValidity time.Duration) error {

 	now := time.Now()
 	validUntil := now.Add(certValidity)
@ -204,8 +222,8 @@ func GenCertPair(org, certFile, keyFile string, tlsExtraIPs,

 	// Get all DNS names and IP addresses to use when creating the
 	// certificate.
-	host, dnsNames := dnsNames(tlsExtraDomains)
-	ipAddresses, err := ipAddresses(tlsExtraIPs)
+	host, dnsNames := dnsNames(tlsExtraDomains, tlsDisableAutofill)
+	ipAddresses, err := ipAddresses(tlsExtraIPs, tlsDisableAutofill)
 	if err != nil {
 		return err
 	}
--- a/cert/selfsigned_test.go
+++ b/cert/selfsigned_test.go
@ -5,6 +5,7 @@ import (
 	"testing"

 	"github.com/lightningnetwork/lnd/cert"
+	"github.com/stretchr/testify/require"
 )

 var (
@ -26,7 +27,7 @@ func TestIsOutdatedCert(t *testing.T) {
 	// Generate TLS files with two extra IPs and domains.
 	err = cert.GenCertPair(
 		"lnd autogenerated cert", certPath, keyPath, extraIPs[:2],
-		extraDomains[:2], cert.DefaultAutogenValidity,
+		extraDomains[:2], false, cert.DefaultAutogenValidity,
 	)
 	if err != nil {
 		t.Fatal(err)
@ -48,7 +49,7 @@ func TestIsOutdatedCert(t *testing.T) {
 			// above.
 			outdated, err := cert.IsOutdated(
 				parsedCert, extraIPs[:numIPs],
-				extraDomains[:numDomains],
+				extraDomains[:numDomains], false,
 			)
 			if err != nil {
 				t.Fatal(err)
@ -81,7 +82,7 @@ func TestIsOutdatedPermutation(t *testing.T) {
 	// Generate TLS files from the IPs and domains.
 	err = cert.GenCertPair(
 		"lnd autogenerated cert", certPath, keyPath, extraIPs[:],
-		extraDomains[:], cert.DefaultAutogenValidity,
+		extraDomains[:], false, cert.DefaultAutogenValidity,
 	)
 	if err != nil {
 		t.Fatal(err)
@ -102,7 +103,7 @@ func TestIsOutdatedPermutation(t *testing.T) {
 		dupDNS[i] = extraDomains[i/2]
 	}

-	outdated, err := cert.IsOutdated(parsedCert, dupIPs, dupDNS)
+	outdated, err := cert.IsOutdated(parsedCert, dupIPs, dupDNS, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@ -123,7 +124,7 @@ func TestIsOutdatedPermutation(t *testing.T) {
 		revDNS[i] = extraDomains[len(extraDomains)-1-i]
 	}

-	outdated, err = cert.IsOutdated(parsedCert, revIPs, revDNS)
+	outdated, err = cert.IsOutdated(parsedCert, revIPs, revDNS, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@ -133,3 +134,59 @@ func TestIsOutdatedPermutation(t *testing.T) {
 			"considered outdated")
 	}
 }
+
+// TestTLSDisableAutofill checks that setting the --tlsdisableautofill flag
+// does not add interface ip addresses or hostnames to the cert.
+func TestTLSDisableAutofill(t *testing.T) {
+	tempDir, err := ioutil.TempDir("", "certtest")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	certPath := tempDir + "/tls.cert"
+	keyPath := tempDir + "/tls.key"
+
+	// Generate TLS files with two extra IPs and domains and no interface IPs.
+	err = cert.GenCertPair(
+		"lnd autogenerated cert", certPath, keyPath, extraIPs[:2],
+		extraDomains[:2], true, cert.DefaultAutogenValidity,
+	)
+	require.NoError(
+		t, err,
+		"unable to generate tls certificate pair",
+	)
+
+	_, parsedCert, err := cert.LoadCert(
+		certPath, keyPath,
+	)
+	require.NoError(
+		t, err,
+		"unable to load tls certificate pair",
+	)
+
+	// Check if the TLS cert is outdated while still preventing
+	// interface IPs from being used. Should not be outdated
+	shouldNotBeOutdated, err := cert.IsOutdated(
+		parsedCert, extraIPs[:2],
+		extraDomains[:2], true,
+	)
+	require.NoError(t, err)
+
+	require.Equal(
+		t, false, shouldNotBeOutdated,
+		"TLS Certificate was marked as outdated when it should not be",
+	)
+
+	// Check if the TLS cert is outdated while allowing for
+	// interface IPs to be used. Should report as outdated.
+	shouldBeOutdated, err := cert.IsOutdated(
+		parsedCert, extraIPs[:2],
+		extraDomains[:2], false,
+	)
+	require.NoError(t, err)
+
+	require.Equal(
+		t, true, shouldBeOutdated,
+		"TLS Certificate was not marked as outdated when it should be",
+	)
+}
--- a/config.go
+++ b/config.go
@ -140,11 +140,12 @@ type Config struct {
 	DataDir      string `short:"b" long:"datadir" description:"The directory to store lnd's data within"`
 	SyncFreelist bool   `long:"sync-freelist" description:"Whether the databases used within lnd should sync their freelist to disk. This is disabled by default resulting in improved memory performance during operation, but with an increase in startup time."`

-	TLSCertPath     string   `long:"tlscertpath" description:"Path to write the TLS certificate for lnd's RPC and REST services"`
-	TLSKeyPath      string   `long:"tlskeypath" description:"Path to write the TLS private key for lnd's RPC and REST services"`
-	TLSExtraIPs     []string `long:"tlsextraip" description:"Adds an extra ip to the generated certificate"`
-	TLSExtraDomains []string `long:"tlsextradomain" description:"Adds an extra domain to the generated certificate"`
-	TLSAutoRefresh  bool     `long:"tlsautorefresh" description:"Re-generate TLS certificate and key if the IPs or domains are changed"`
+	TLSCertPath        string   `long:"tlscertpath" description:"Path to write the TLS certificate for lnd's RPC and REST services"`
+	TLSKeyPath         string   `long:"tlskeypath" description:"Path to write the TLS private key for lnd's RPC and REST services"`
+	TLSExtraIPs        []string `long:"tlsextraip" description:"Adds an extra ip to the generated certificate"`
+	TLSExtraDomains    []string `long:"tlsextradomain" description:"Adds an extra domain to the generated certificate"`
+	TLSAutoRefresh     bool     `long:"tlsautorefresh" description:"Re-generate TLS certificate and key if the IPs or domains are changed"`
+	TLSDisableAutofill bool     `long:"tlsdisableautofill" description:"Do not include the interface IPs or the system hostname in TLS certificate, use first --tlsextradomain as Common Name instead, if set"`

 	NoMacaroons     bool          `long:"no-macaroons" description:"Disable macaroon authentication"`
 	AdminMacPath    string        `long:"adminmacaroonpath" description:"Path to write the admin macaroon for lnd's RPC and REST services if it doesn't exist"`
--- a/lnd.go
+++ b/lnd.go
@ -775,7 +775,7 @@ func getTLSConfig(cfg *Config) (*tls.Config, *credentials.TransportCredentials,
 		err := cert.GenCertPair(
 			"lnd autogenerated cert", cfg.TLSCertPath,
 			cfg.TLSKeyPath, cfg.TLSExtraIPs, cfg.TLSExtraDomains,
-			cert.DefaultAutogenValidity,
+			cfg.TLSDisableAutofill, cert.DefaultAutogenValidity,
 		)
 		if err != nil {
 			return nil, nil, "", err
@ -797,7 +797,8 @@ func getTLSConfig(cfg *Config) (*tls.Config, *credentials.TransportCredentials,
 	refresh := false
 	if cfg.TLSAutoRefresh {
 		refresh, err = cert.IsOutdated(
-			parsedCert, cfg.TLSExtraIPs, cfg.TLSExtraDomains,
+			parsedCert, cfg.TLSExtraIPs,
+			cfg.TLSExtraDomains, cfg.TLSDisableAutofill,
 		)
 		if err != nil {
 			return nil, nil, "", err
@ -824,7 +825,7 @@ func getTLSConfig(cfg *Config) (*tls.Config, *credentials.TransportCredentials,
 		err = cert.GenCertPair(
 			"lnd autogenerated cert", cfg.TLSCertPath,
 			cfg.TLSKeyPath, cfg.TLSExtraIPs, cfg.TLSExtraDomains,
-			cert.DefaultAutogenValidity,
+			cfg.TLSDisableAutofill, cert.DefaultAutogenValidity,
 		)
 		if err != nil {
 			return nil, nil, "", err