git » kxd » commit d3c6e3f

diff --git a/kxc/kxc.go b/kxc/kxc.go
index 66860e9..5d5e02b 100644
--- a/kxc/kxc.go
+++ b/kxc/kxc.go
@@ -9,12 +9,10 @@ package main
 import (
 	"crypto/tls"
 	"crypto/x509"
-	"encoding/pem"
 	"flag"
 	"fmt"
 	"io/ioutil"
 	"log"
-	"net"
 	"net/http"
 	"net/url"
 	"strings"
@@ -29,70 +27,18 @@ var client_cert = flag.String(
 var client_key = flag.String(
 	"client_key", "", "File containing the client private key")
 
-func LoadServerCerts() ([]*x509.Certificate, error) {
+func LoadServerCerts() (*x509.CertPool, error) {
 	pemData, err := ioutil.ReadFile(*server_cert)
 	if err != nil {
 		return nil, err
 	}
 
-	var certs []*x509.Certificate
-	for len(pemData) > 0 {
-		var block *pem.Block
-		block, pemData = pem.Decode(pemData)
-		if block == nil {
-			return certs, nil
-		}
-
-		if block.Type != "CERTIFICATE" {
-			continue
-		}
-
-		cert, err := x509.ParseCertificate(block.Bytes)
-		if err != nil {
-			return certs, err
-		}
-
-		certs = append(certs, cert)
-	}
-
-	return certs, nil
-}
-
-// We can't ask the http client for the TLS connection, so we make it
-// ourselves and save it here.
-// It is hacky but it simplifies the code.
-var tlsConn *tls.Conn
-
-func OurDial(network, addr string) (net.Conn, error) {
-	var err error
-
-	tlsConf := &tls.Config{}
-	tlsConf.Certificates = make([]tls.Certificate, 1)
-	tlsConf.Certificates[0], err = tls.LoadX509KeyPair(
-		*client_cert, *client_key)
-	if err != nil {
-		log.Fatalf("Failed to load keys: %s", err)
+	pool := x509.NewCertPool()
+	if !pool.AppendCertsFromPEM(pemData) {
+		return nil, fmt.Errorf("Error appending certificates")
 	}
 
-	// We don't want the TLS stack to check the cert, as we will compare
-	// it ourselves.
-	tlsConf.InsecureSkipVerify = true
-
-	tlsConn, err = tls.Dial(network, addr, tlsConf)
-	return tlsConn, err
-}
-
-// Check if any cert from requestedCerts matches any cert in validCerts.
-func AnyCertMatches(requestedCerts, validCerts []*x509.Certificate) bool {
-	for _, cert := range requestedCerts {
-		for _, validCert := range validCerts {
-			if cert.Equal(validCert) {
-				return true
-			}
-		}
-	}
-
-	return false
+	return pool, nil
 }
 
 // Check if the given network address has a port.
@@ -103,24 +49,17 @@ func hasPort(s string) bool {
 }
 
 func ExtractURL(rawurl string) (*url.URL, error) {
-	// Because we handle the transport ourselves, the http library has to
-	// be told to use plain http; otherwise it will attempt to do its own
-	// TLS on top of our TLS.
 	serverURL, err := url.Parse(rawurl)
 	if err != nil {
 		return nil, err
 	}
 
-	// Fix up the scheme to use http.
-	// Because we handle the transport ourselves, the http library has to
-	// be told to use plain http; otherwise it will attempt to do its own
-	// TLS on top of our TLS (and obviously fails, in this case with an
-	// "local error: record overflow" error).
+	// Make sure we're using https.
 	switch serverURL.Scheme {
-	case "http":
+	case "https":
 		// Nothing to do here.
-	case "https", "kxd":
-		serverURL.Scheme = "http"
+	case "http", "kxd":
+		serverURL.Scheme = "https"
 	default:
 		return nil, fmt.Errorf("Unsupported URL schema (try kxd://)")
 	}
@@ -139,17 +78,33 @@ func ExtractURL(rawurl string) (*url.URL, error) {
 	return serverURL, nil
 }
 
-func main() {
+func MakeTLSConf() *tls.Config {
 	var err error
-	flag.Parse()
 
+	tlsConf := &tls.Config{}
+	tlsConf.Certificates = make([]tls.Certificate, 1)
+	tlsConf.Certificates[0], err = tls.LoadX509KeyPair(
+		*client_cert, *client_key)
+	if err != nil {
+		log.Fatalf("Failed to load keys: %s", err)
+	}
+
+	// Compare against the server certificates.
 	serverCerts, err := LoadServerCerts()
 	if err != nil {
 		log.Fatalf("Failed to load server certs: %s", err)
 	}
+	tlsConf.RootCAs = serverCerts
+
+	return tlsConf
+}
+
+func main() {
+	var err error
+	flag.Parse()
 
 	tr := &http.Transport{
-		Dial: OurDial,
+		TLSClientConfig: MakeTLSConf(),
 	}
 
 	client := &http.Client{
@@ -177,10 +132,5 @@ func main() {
 			resp.Status, content)
 	}
 
-	if !AnyCertMatches(
-		tlsConn.ConnectionState().PeerCertificates, serverCerts) {
-		log.Fatalf("No server certificate matches")
-	}
-
 	fmt.Printf("%s", content)
 }
diff --git a/kxd/key_config.go b/kxd/key_config.go
index 5b9e491..01b39bd 100644
--- a/kxd/key_config.go
+++ b/kxd/key_config.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"crypto/x509"
-	"encoding/pem"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -49,7 +48,7 @@ type KeyConfig struct {
 	emailToPath        string
 
 	// Allowed certificates.
-	allowedClientCerts []*x509.Certificate
+	allowedClientCerts *x509.CertPool
 
 	// Allowed hosts.
 	allowedHosts []string
@@ -62,6 +61,7 @@ func NewKeyConfig(configPath string) *KeyConfig {
 		allowedClientsPath: configPath + "/allowed_clients",
 		allowedHostsPath:   configPath + "/allowed_hosts",
 		emailToPath:        configPath + "/email_to",
+		allowedClientCerts: x509.NewCertPool(),
 	}
 }
 
@@ -94,23 +94,8 @@ func (kc *KeyConfig) LoadClientCerts() error {
 		return err
 	}
 
-	for len(rawContents) > 0 {
-		var block *pem.Block
-		block, rawContents = pem.Decode(rawContents)
-		if block == nil {
-			return nil
-		}
-
-		if block.Type != "CERTIFICATE" {
-			continue
-		}
-
-		cert, err := x509.ParseCertificate(block.Bytes)
-		if err != nil {
-			return err
-		}
-
-		kc.allowedClientCerts = append(kc.allowedClientCerts, cert)
+	if !kc.allowedClientCerts.AppendCertsFromPEM(rawContents) {
+		return fmt.Errorf("Error parsing client certificate file")
 	}
 
 	return nil
@@ -150,11 +135,19 @@ func (kc *KeyConfig) LoadAllowedHosts() error {
 
 func (kc *KeyConfig) IsAnyCertAllowed(
 	certs []*x509.Certificate) *x509.Certificate {
+	opts := x509.VerifyOptions{
+		Roots: kc.allowedClientCerts,
+	}
 	for _, cert := range certs {
-		for _, allowedCert := range kc.allowedClientCerts {
-			if cert.Equal(allowedCert) {
-				return cert
-			}
+		chains, err := cert.Verify(opts)
+		if err != nil {
+			continue
+		}
+
+		// Our clients have only one certificate, so no need to complicate
+		// lookups.
+		if len(chains) > 0 && len(chains[0]) > 0 {
+			return chains[0][len(chains[0])-1]
 		}
 	}
 	return nil
diff --git a/scripts/create-kxd-config b/scripts/create-kxd-config
index 2513b48..d73ee0a 100755
--- a/scripts/create-kxd-config
+++ b/scripts/create-kxd-config
@@ -29,7 +29,7 @@ fi
 if ! [ -e /etc/kxd/cert.pem ]; then
 	echo "Generating certificate (/etc/kxd/cert.pem)"
 	openssl req -new -x509 -batch \
-		-subj "/organizationalUnitName=kxd@$HOSTNAME/" \
+		-subj "/commonName=*/organizationalUnitName=kxd@$HOSTNAME/" \
 		-key /etc/kxd/key.pem -out /etc/kxd/cert.pem
 else
 	echo "Certificate already exists (/etc/kxd/cert.pem)"
diff --git a/scripts/kxc-add-key b/scripts/kxc-add-key
index 6d57a81..1606e1c 100755
--- a/scripts/kxc-add-key
+++ b/scripts/kxc-add-key
@@ -40,7 +40,7 @@ fi
 if ! [ -e /etc/kxc/cert.pem ]; then
 	echo "Generating certificate (/etc/kxc/cert.pem)"
 	openssl req -new -x509 -batch \
-		-subj "/organizationalUnitName=kxc@$HOSTNAME/" \
+		-subj "/commonName=*/organizationalUnitName=kxc@$HOSTNAME/" \
 		-key /etc/kxc/key.pem -out /etc/kxc/cert.pem
 else
 	echo "Certificate already exists (/etc/kxc/cert.pem)"
diff --git a/tests/run_tests b/tests/run_tests
index cef7248..c02c722 100755
--- a/tests/run_tests
+++ b/tests/run_tests
@@ -66,8 +66,10 @@ def gen_certs(path):
         stdout=DEVNULL, stderr=DEVNULL)
     subprocess.check_call(
         ["openssl", "req", "-new", "-x509", "-batch",
-         "-subj", "/organizationalUnitName=kxd-tests:%s@%s" % (
-            os.getlogin(), platform.node()),
+         "-subj",
+            "/commonName=*" +
+            "/organizationalUnitName=kxd-tests:%s@%s" % (
+                os.getlogin(), platform.node()),
          "-key", "%s/key.pem" % path,
          "-out", "%s/cert.pem" % path],
         stdout=DEVNULL, stderr=DEVNULL)
@@ -227,7 +229,7 @@ class Simple(TestCase):
         # We tell the client to expect the server certificate to be the client
         # one, which is never going to work.
         self.assertClientFails("kxd://localhost/k1",
-                "No server certificate matches",
+                "certificate signed by unknown authority",
                 cert_path=self.client.cert_path())
 
 
@@ -350,7 +352,7 @@ class BrokenServerConfig(TestCase):
 
         # Corrupt the client certificate.
         cfd = open(self.server.path + "/data/k1/allowed_clients", "r+")
-        for _ in range(4):
+        for _ in range(5):
             cfd.readline()
         cfd.write('+/+BROKEN+/+')
         cfd.close()