[lxc-devel] [lxd/master] Rework lxd.NewClient so we don't need a disk cache.

jameinel on Github lxc-bot at linuxcontainers.org
Tue Feb 23 14:27:16 UTC 2016


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 668 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160223/1b52ce8b/attachment.bin>
-------------- next part --------------
From 12c5fa2ab53194584da26b66f5df5ce2ac694bfb Mon Sep 17 00:00:00 2001
From: John Arbash Meinel <john at arbash-meinel.com>
Date: Tue, 23 Feb 2016 18:25:13 +0400
Subject: [PATCH] Rework lxd.NewClient so we don't need a disk cache.

This adds a new interface NewClientFromInfo which lets API clients
decide how they want to track the information that they need, and
just supply that information when they go to connect.
NewClient still uses the disk cache, but shares all of the actual
setting up of connections via NewClientFromInfo.

Signed-off-by: John Arbash Meinel <john at arbash-meinel.com>
---
 client.go         | 231 +++++++++++++++++++++++++++++++++++++-----------------
 shared/network.go |  63 ++++++++++++---
 2 files changed, 210 insertions(+), 84 deletions(-)

diff --git a/client.go b/client.go
index 00b7190..a26c00a 100644
--- a/client.go
+++ b/client.go
@@ -142,7 +142,7 @@ func HoistResponse(r *http.Response, rtype ResponseType) (*Response, error) {
 	return resp, nil
 }
 
-func readMyCert(configDir string) (string, string, error) {
+func ensureMyCert(configDir string) (string, string, error) {
 	certf := path.Join(configDir, "client.crt")
 	keyf := path.Join(configDir, "client.key")
 
@@ -153,94 +153,181 @@ func readMyCert(configDir string) (string, string, error) {
 
 // NewClient returns a new LXD client.
 func NewClient(config *Config, remote string) (*Client, error) {
-	c := Client{
-		Config: *config,
-		Http:   http.Client{},
-	}
-
-	c.Name = remote
-
 	if remote == "" {
 		return nil, fmt.Errorf("A remote name must be provided.")
 	}
 
-	if r, ok := config.Remotes[remote]; ok {
-		if r.Addr[0:5] == "unix:" {
-			if r.Addr == "unix://" {
-				r.Addr = fmt.Sprintf("unix:%s", shared.VarPath("unix.socket"))
-			}
-
-			c.BaseURL = "http://unix.socket"
-			c.BaseWSURL = "ws://unix.socket"
-			c.Transport = "unix"
-			uDial := func(networ, addr string) (net.Conn, error) {
-				var err error
-				var raddr *net.UnixAddr
-				if r.Addr[7:] == "unix://" {
-					raddr, err = net.ResolveUnixAddr("unix", r.Addr[7:])
-				} else {
-					raddr, err = net.ResolveUnixAddr("unix", r.Addr[5:])
-				}
-				if err != nil {
-					return nil, err
-				}
-				return net.DialUnix("unix", nil, raddr)
-			}
-			c.Http.Transport = &http.Transport{Dial: uDial}
-			c.websocketDialer.NetDial = uDial
-			c.Remote = &r
-
-			st, err := c.ServerStatus()
+	r, ok := config.Remotes[remote]
+	if !ok {
+		return nil, fmt.Errorf("unknown remote name: %q", remote)
+	}
+	info := ConnectInfo{
+		Name: remote,
+		Addr: r.Addr,
+	}
+	if r.Addr[0:5] != "unix:" {
+		certf, keyf, err := ensureMyCert(config.ConfigDir)
+		if err != nil {
+			return nil, err
+		}
+		certBytes, err := ioutil.ReadFile(certf)
+		if err != nil {
+			return nil, err
+		}
+		keyBytes, err := ioutil.ReadFile(keyf)
+		if err != nil {
+			return nil, err
+		}
+		info.ClientPEMCert = string(certBytes)
+		info.ClientPEMKey = string(keyBytes)
+		serverCertPath := config.ServerCertPath(remote)
+		if shared.PathExists(serverCertPath) {
+			cert, err := shared.ReadCert(serverCertPath)
 			if err != nil {
 				return nil, err
 			}
-			c.Certificate = st.Environment.Certificate
+
+			info.ServerPEMCert = string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}))
+		}
+	}
+	c, err := NewClientFromInfo(info)
+	if err != nil {
+		return nil, err
+	}
+	c.Config = *config
+	return c, nil
+}
+
+// ConnectInfo contains the information we need to connect to a specific LXD server
+type ConnectInfo struct {
+	// Name is a simple identifier for the remote server. In 'lxc' it is
+	// the name used to lookup the address and other information in the
+	// config.yml file.
+	Name string
+	// Addr is the host address to connect to. It can be unix: to indicate
+	// we should connect over a unix socket, or it can be an IP Address or
+	// Hostname, or an https:// URL.
+	Addr string
+	// ClientPEMCert is the PEM encoded bytes of the client's certificate.
+	// If Addr indicates a Unix socket, the certificate and key bytes will
+	// not be used.
+	ClientPEMCert string
+	// ClientPEMKey is the PEM encoded private bytes of the client's key associated with its certificate
+	ClientPEMKey string
+	// ServerPEMCert is the PEM encoded server certificate that we are
+	// connecting to. It can be the empty string if we do not know the
+	// server's certificate yet.
+	ServerPEMCert string
+}
+
+// how to handle ServerCerts that we want to cache?
+
+func connectViaUnix(c *Client, addr string) error {
+	if addr == "unix://" {
+		addr = fmt.Sprintf("unix:%s", shared.VarPath("unix.socket"))
+	}
+
+	c.BaseURL = "http://unix.socket"
+	c.BaseWSURL = "ws://unix.socket"
+	c.Transport = "unix"
+	r := &RemoteConfig{
+		Addr: addr,
+		// TODO: (jam) RemoteConfig.Public is not relevant for the purposes of
+		// an active connection. It only seems to be used to display server
+		// information in "list", though if we aren't getting that information
+		// from the service itself, it seems like it can't be guaranteed to be
+		// correct, as it is just the local information about a remote server.
+		// Is there another reason it is here?
+		Public: false,
+	}
+	uDial := func(networ, addr string) (net.Conn, error) {
+		// TODO: (jam) Why are we ignoring the passed in 'addr' and only using our own Addr?
+		// Do we allow other code to mutate Client.Remote.Addr and
+		// change what we connect to, and still ignore the 'adrd'
+		// passed to Dial?
+		var err error
+		var raddr *net.UnixAddr
+		if r.Addr[7:] == "unix://" {
+			raddr, err = net.ResolveUnixAddr("unix", r.Addr[7:])
 		} else {
-			certf, keyf, err := readMyCert(config.ConfigDir)
-			if err != nil {
-				return nil, err
-			}
+			raddr, err = net.ResolveUnixAddr("unix", r.Addr[5:])
+		}
+		if err != nil {
+			return nil, err
+		}
+		return net.DialUnix("unix", nil, raddr)
+	}
+	c.Http.Transport = &http.Transport{Dial: uDial}
+	c.websocketDialer.NetDial = uDial
+	c.Remote = r
 
-			var cert *x509.Certificate
-			if shared.PathExists(c.Config.ServerCertPath(c.Name)) {
-				cert, err = shared.ReadCert(c.Config.ServerCertPath(c.Name))
-				if err != nil {
-					return nil, err
-				}
+	st, err := c.ServerStatus()
+	if err != nil {
+		return err
+	}
+	c.Certificate = st.Environment.Certificate
+	return nil
+}
 
-				c.Certificate = string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}))
-			}
+func connectViaHttp(c *Client, addr, clientCert, clientKey, serverCert string) error {
 
-			tlsconfig, err := shared.GetTLSConfig(certf, keyf, cert)
-			if err != nil {
-				return nil, err
-			}
+	tlsconfig, err := shared.GetTLSConfigMem(clientCert, clientKey, serverCert)
+	if err != nil {
+		return err
+	}
 
-			tr := &http.Transport{
-				TLSClientConfig: tlsconfig,
-				Dial:            shared.RFC3493Dialer,
-				Proxy:           http.ProxyFromEnvironment,
-			}
+	tr := &http.Transport{
+		TLSClientConfig: tlsconfig,
+		Dial:            shared.RFC3493Dialer,
+		Proxy:           http.ProxyFromEnvironment,
+	}
 
-			c.websocketDialer.NetDial = shared.RFC3493Dialer
-			c.websocketDialer.TLSClientConfig = tlsconfig
+	c.websocketDialer.NetDial = shared.RFC3493Dialer
+	c.websocketDialer.TLSClientConfig = tlsconfig
 
-			if r.Addr[0:8] == "https://" {
-				c.BaseURL = "https://" + r.Addr[8:]
-				c.BaseWSURL = "wss://" + r.Addr[8:]
-			} else {
-				c.BaseURL = "https://" + r.Addr
-				c.BaseWSURL = "wss://" + r.Addr
-			}
-			c.Transport = "https"
-			c.Http.Transport = tr
-			c.Remote = &r
-		}
+	if addr[0:8] == "https://" {
+		c.BaseURL = "https://" + addr[8:]
+		c.BaseWSURL = "wss://" + addr[8:]
 	} else {
-		return nil, fmt.Errorf("unknown remote name: %q", remote)
+		c.BaseURL = "https://" + addr
+		c.BaseWSURL = "wss://" + addr
+	}
+	c.Transport = "https"
+	c.Http.Transport = tr
+	c.Remote = &RemoteConfig{
+		Addr: addr,
+		// TODO: (jam) RemoteConfig.Public is not relevant for the purposes of
+		// an active connection. It only seems to be used to display server
+		// information in "list", though if we aren't getting that information
+		// from the service itself, it seems like it can't be guaranteed to be
+		// correct, as it is just the local information about a remote server.
+		// Is there another reason it is here?
+		Public: false,
+	}
+	// TODO: (jam) It is odd to me that the Unix socket path actually
+	// connects to the service, but the HTTP path just sets up a transport
+	// and socket dialer, but doesn't actually try to connect.
+	return nil
+}
+
+// NewClientFromInfo returns a new LXD client.
+func NewClientFromInfo(info ConnectInfo) (*Client, error) {
+	c := &Client{
+		// Config: *config,
+		Http: http.Client{},
+	}
+	c.Name = info.Name
+	var err error
+	if info.Addr[0:5] == "unix:" {
+		err = connectViaUnix(c, info.Addr)
+	} else {
+		err = connectViaHttp(c, info.Addr, info.ClientPEMCert, info.ClientPEMKey, info.ServerPEMCert)
+	}
+	if err != nil {
+		return nil, err
 	}
 
-	return &c, nil
+	return c, nil
 }
 
 func (c *Client) Addresses() ([]string, error) {
diff --git a/shared/network.go b/shared/network.go
index d853a85..cda850d 100644
--- a/shared/network.go
+++ b/shared/network.go
@@ -3,6 +3,7 @@ package shared
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/pem"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -62,8 +63,8 @@ func GetRemoteCertificate(address string) (*x509.Certificate, error) {
 	return resp.TLS.PeerCertificates[0], nil
 }
 
-func GetTLSConfig(tlsClientCert string, tlsClientKey string, tlsRemoteCert *x509.Certificate) (*tls.Config, error) {
-	tlsConfig := &tls.Config{
+func initTLSConfig() *tls.Config {
+	return &tls.Config{
 		MinVersion: tls.VersionTLS12,
 		MaxVersion: tls.VersionTLS12,
 		CipherSuites: []uint16{
@@ -71,17 +72,9 @@ func GetTLSConfig(tlsClientCert string, tlsClientKey string, tlsRemoteCert *x509
 			tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
 		PreferServerCipherSuites: true,
 	}
+}
 
-	// Client authentication
-	if tlsClientCert != "" && tlsClientKey != "" {
-		cert, err := tls.LoadX509KeyPair(tlsClientCert, tlsClientKey)
-		if err != nil {
-			return nil, err
-		}
-
-		tlsConfig.Certificates = []tls.Certificate{cert}
-	}
-
+func finalizeTLSConfig(tlsConfig *tls.Config, tlsRemoteCert *x509.Certificate) {
 	// Trusted certificates
 	if tlsRemoteCert != nil {
 		caCertPool := x509.NewCertPool()
@@ -101,6 +94,52 @@ func GetTLSConfig(tlsClientCert string, tlsClientKey string, tlsRemoteCert *x509
 	}
 
 	tlsConfig.BuildNameToCertificate()
+}
+
+func GetTLSConfig(tlsClientCertFile string, tlsClientKeyFile string, tlsRemoteCert *x509.Certificate) (*tls.Config, error) {
+	tlsConfig := initTLSConfig()
+
+	// Client authentication
+	if tlsClientCertFile != "" && tlsClientKeyFile != "" {
+		cert, err := tls.LoadX509KeyPair(tlsClientCertFile, tlsClientKeyFile)
+		if err != nil {
+			return nil, err
+		}
+
+		tlsConfig.Certificates = []tls.Certificate{cert}
+	}
+
+	finalizeTLSConfig(tlsConfig, tlsRemoteCert)
+	return tlsConfig, nil
+}
+
+func GetTLSConfigMem(tlsClientCert string, tlsClientKey string, tlsRemoteCertPEM string) (*tls.Config, error) {
+	tlsConfig := initTLSConfig()
+
+	// Client authentication
+	if tlsClientCert != "" && tlsClientKey != "" {
+		cert, err := tls.X509KeyPair([]byte(tlsClientCert), []byte(tlsClientKey))
+		if err != nil {
+			return nil, err
+		}
+
+		tlsConfig.Certificates = []tls.Certificate{cert}
+	}
+
+	var tlsRemoteCert *x509.Certificate
+	if tlsRemoteCertPEM != "" {
+		/// XXX: jam 2016-02-23 shared.ReadCert ignores any trailing
+		//content. Is that secure? I know there were security holes in
+		//some gpg decrypt usage because it would pass on extra bytes
+		//outside of the signed portion.
+		certBlock, _ := pem.Decode([]byte(tlsRemoteCertPEM))
+		var err error
+		tlsRemoteCert, err = x509.ParseCertificate(certBlock.Bytes)
+		if err != nil {
+			return nil, err
+		}
+	}
+	finalizeTLSConfig(tlsConfig, tlsRemoteCert)
 
 	return tlsConfig, nil
 }


More information about the lxc-devel mailing list