Skip to content

Commit

Permalink
Merge f3cfe7e into cfbc365
Browse files Browse the repository at this point in the history
  • Loading branch information
gkrizek committed Jul 22, 2020
2 parents cfbc365 + f3cfe7e commit eaa4706
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 32 deletions.
56 changes: 37 additions & 19 deletions cert/selfsigned.go
Expand Up @@ -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")}

Expand All @@ -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)
}
}
}

Expand All @@ -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
Expand All @@ -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")
Expand All @@ -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
}
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Expand Down
67 changes: 62 additions & 5 deletions cert/selfsigned_test.go
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

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

var (
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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",
)
}
11 changes: 6 additions & 5 deletions config.go
Expand Up @@ -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"`
Expand Down
7 changes: 4 additions & 3 deletions lnd.go
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit eaa4706

Please sign in to comment.