Use common TLS parsing routine for etcd (#476)

* Use common TLS parsing routine for etcd

Change to use the new common routine, and update the docs to reflect
the different options for passing TLS configuration.

* Move middleware/tls to middleware/pkg/tls

This was put in the wrong place originally.

middleware/etcd/README.md

@@ -39,7 +39,11 @@ etcd [ZONES...] {
   pointing to external names. If you want CoreDNS to act as a proxy for clients, you'll need to add
   the proxy middleware. **ADDRESS** can be an IP address, and IP:port or a string pointing to a file
   that is structured as /etc/resolv.conf.
-* `tls` followed the cert, key and the CA's cert filenames.
+* `tls` followed by:
+  * no arguments, if the server certificate is signed by a system-installed CA and no client cert is needed
+  * a single argument that is the CA PEM file, if the server cert is not signed by a system CA and no client cert is needed
+  * two arguments - path to cert PEM file, the path to private key PEM file - if the server certificate is signed by a system-installed CA and a client certificate is needed
+  * three arguments - path to cert PEM file, path to client private key PEM file, path to CA PEM file - if the server certificate is not signed by a system-installed CA and client certificate is needed
 * `debug` allows for debug queries. Prefix the name with `o-o.debug.` to retrieve extra information in the
   additional section of the reply in the form of TXT records.
 

middleware/etcd/setup.go

@@ -2,8 +2,6 @@ package etcd
 
 import (
 	"crypto/tls"
-	"crypto/x509"
-	"io/ioutil"
 	"net"
 	"net/http"
 	"time"
@@ -12,6 +10,7 @@ import (
 	"github.com/miekg/coredns/middleware"
 	"github.com/miekg/coredns/middleware/pkg/dnsutil"
 	"github.com/miekg/coredns/middleware/pkg/singleflight"
+	mwtls "github.com/miekg/coredns/middleware/pkg/tls"
 	"github.com/miekg/coredns/middleware/proxy"
 
 	etcdc "github.com/coreos/etcd/client"
@@ -57,11 +56,10 @@ func etcdParse(c *caddy.Controller) (*Etcd, bool, error) {
 		Stubmap:    &stub,
 	}
 	var (
-		tlsCertFile   = ""
-		tlsKeyFile    = ""
-		tlsCAcertFile = ""
-		endpoints     = []string{defaultEndpoint}
-		stubzones     = false
+		tlsConfig *tls.Config
+		err	  error
+		endpoints = []string{defaultEndpoint}
+		stubzones = false
 	)
 	for c.Next() {
 		if c.Val() == "etcd" {
@@ -101,10 +99,10 @@ func etcdParse(c *caddy.Controller) (*Etcd, bool, error) {
 					etc.Proxy = proxy.New(ups)
 				case "tls": // cert key cacertfile
 					args := c.RemainingArgs()
-					if len(args) != 3 {
-						return &Etcd{}, false, c.ArgErr()
+					tlsConfig, err = mwtls.NewTLSConfigFromArgs(args...)
+					if err != nil {
+						return &Etcd{}, false, err
 					}
-					tlsCertFile, tlsKeyFile, tlsCAcertFile = args[0], args[1], args[2]
 				default:
 					if c.Val() != "}" {
 						return &Etcd{}, false, c.Errf("unknown property '%s'", c.Val())
@@ -139,10 +137,10 @@ func etcdParse(c *caddy.Controller) (*Etcd, bool, error) {
 						etc.Proxy = proxy.New(ups)
 					case "tls": // cert key cacertfile
 						args := c.RemainingArgs()
-						if len(args) != 3 {
-							return &Etcd{}, false, c.ArgErr()
+						tlsConfig, err = mwtls.NewTLSConfigFromArgs(args...)
+						if err != nil {
+							return &Etcd{}, false, err
 						}
-						tlsCertFile, tlsKeyFile, tlsCAcertFile = args[0], args[1], args[2]
 					default:
 						if c.Val() != "}" { // TODO(miek): this feels like I'm doing it completely wrong.
 							return &Etcd{}, false, c.Errf("unknown property '%s'", c.Val())
@@ -151,7 +149,7 @@ func etcdParse(c *caddy.Controller) (*Etcd, bool, error) {
 				}
 
 			}
-			client, err := newEtcdClient(endpoints, tlsCertFile, tlsKeyFile, tlsCAcertFile)
+			client, err := newEtcdClient(endpoints, tlsConfig)
 			if err != nil {
 				return &Etcd{}, false, err
 			}
@@ -164,10 +162,10 @@ func etcdParse(c *caddy.Controller) (*Etcd, bool, error) {
 	return &Etcd{}, false, nil
 }
 
-func newEtcdClient(endpoints []string, tlsCert, tlsKey, tlsCACert string) (etcdc.KeysAPI, error) {
+func newEtcdClient(endpoints []string, cc *tls.Config) (etcdc.KeysAPI, error) {
 	etcdCfg := etcdc.Config{
 		Endpoints: endpoints,
-		Transport: newHTTPSTransport(tlsCert, tlsKey, tlsCACert),
+		Transport: newHTTPSTransport(cc),
 	}
 	cli, err := etcdc.New(etcdCfg)
 	if err != nil {
@@ -176,25 +174,10 @@ func newEtcdClient(endpoints []string, tlsCert, tlsKey, tlsCACert string) (etcdc
 	return etcdc.NewKeysAPI(cli), nil
 }
 
-func newHTTPSTransport(tlsCertFile, tlsKeyFile, tlsCACertFile string) etcdc.CancelableTransport {
-	var cc *tls.Config
-
-	if tlsCertFile != "" && tlsKeyFile != "" {
-		var rpool *x509.CertPool
-		if tlsCACertFile != "" {
-			if pemBytes, err := ioutil.ReadFile(tlsCACertFile); err == nil {
-				rpool = x509.NewCertPool()
-				rpool.AppendCertsFromPEM(pemBytes)
-			}
-		}
-
-		if tlsCert, err := tls.LoadX509KeyPair(tlsCertFile, tlsKeyFile); err == nil {
-			cc = &tls.Config{
-				RootCAs:            rpool,
-				Certificates:       []tls.Certificate{tlsCert},
-				InsecureSkipVerify: true,
-			}
-		}
+func newHTTPSTransport(cc *tls.Config) etcdc.CancelableTransport {
+	// this seems like a bad idea but was here in the previous version
+	if cc != nil {
+		cc.InsecureSkipVerify = true
 	}
 
 	tr := &http.Transport{

middleware/etcd/setup_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/miekg/coredns/middleware/pkg/singleflight"
 	"github.com/miekg/coredns/middleware/proxy"
 	"github.com/miekg/coredns/middleware/test"
+	"github.com/miekg/coredns/middleware/pkg/tls"
 
 	etcdc "github.com/coreos/etcd/client"
 	"github.com/mholt/caddy"
@@ -28,7 +29,8 @@ func newEtcdMiddleware() *Etcd {
 	ctxt, _ = context.WithTimeout(context.Background(), etcdTimeout)
 
 	endpoints := []string{"http://localhost:2379"}
-	client, _ := newEtcdClient(endpoints, "", "", "")
+	tlsc, _ := tls.NewTLSConfigFromArgs()
+	client, _ := newEtcdClient(endpoints, tlsc)
 
 	return &Etcd{
 		Proxy:      proxy.New([]string{"8.8.8.8:53"}),