Client should check TLS cert for valid dates

Checks added to make sure the TLS certificate being used has approprate dates, for example not being expired. Also, error handling improved to better indicate why a certificate might be rejected for use in TLS connection. https://jira.hyperledger.org/browse/FAB-2795 Change-Id: If6c047627ddda0ea66bd043f55bd2f2249134310 Signed-off-by: Saad Karim <[email protected]>
hyperledger · Apr 12, 2017 · f0f86b7 · f0f86b7
1 parent d67841e
commit f0f86b7
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 28 deletions.
diff --git a/cmd/fabric-ca-client/config.go b/cmd/fabric-ca-client/config.go
@@ -88,12 +88,9 @@ URL: <<<URL>>>
 MSPDir:
 
 #############################################################################
-#    TLS section for the client's listening port
+#    TLS section for secure socket connection
 #############################################################################
 tls:
-  # Enable TLS (default: false)
-  enabled: false
-
   # TLS section for secure socket connection
   certfiles:			# Comma Separated list of root certificate files (e.g. root.pem, root2.pem)
   client:
@@ -108,7 +105,7 @@ csr:
   cn: <<<ENROLLMENT_ID>>>
   names:
     - C: US
-      ST: "North Carolina"
+      ST: North Carolina
       L:
       O: Hyperledger
       OU: Fabric

diff --git a/cmd/fabric-ca-client/main_test.go b/cmd/fabric-ca-client/main_test.go
@@ -37,21 +37,22 @@ import (
 )
 
 const (
-	testYaml          = "../../testdata/test.yaml"
-	mspDir            = "../../testdata/msp"
-	myhost            = "hostname"
-	certfile          = "ec.pem"
-	keyfile           = "ec-key.pem"
-	tlsCertFile       = "tls_server-cert.pem"
-	tlsKeyFile        = "tls_server-key.pem"
-	rootCert          = "root.pem"
-	tlsClientCertFile = "tls_client-cert.pem"
-	tlsClientKeyFile  = "tls_client-key.pem"
-	tdDir             = "../../testdata"
-	db                = "fabric-ca-server.db"
-	rootCertEnvVar    = "FABRIC_CA_CLIENT_TLS_CERTFILES"
-	clientKeyEnvVar   = "FABRIC_CA_CLIENT_TLS_CLIENT_KEYFILE"
-	clientCertEnvVar  = "FABRIC_CA_CLIENT_TLS_CLIENT_CERTFILE"
+	testYaml             = "../../testdata/test.yaml"
+	mspDir               = "../../testdata/msp"
+	myhost               = "hostname"
+	certfile             = "ec.pem"
+	keyfile              = "ec-key.pem"
+	tlsCertFile          = "tls_server-cert.pem"
+	tlsKeyFile           = "tls_server-key.pem"
+	rootCert             = "root.pem"
+	tlsClientCertFile    = "tls_client-cert.pem"
+	tlsClientCertExpired = "expiredcert.pem"
+	tlsClientKeyFile     = "tls_client-key.pem"
+	tdDir                = "../../testdata"
+	db                   = "fabric-ca-server.db"
+	rootCertEnvVar       = "FABRIC_CA_CLIENT_TLS_CERTFILES"
+	clientKeyEnvVar      = "FABRIC_CA_CLIENT_TLS_CLIENT_KEYFILE"
+	clientCertEnvVar     = "FABRIC_CA_CLIENT_TLS_CLIENT_CERTFILE"
 )
 
 const jsonConfig = `{
@@ -540,6 +541,11 @@ func TestClientCommandsTLS(t *testing.T) {
 		t.Errorf("client enroll -c -u failed: %s", err)
 	}
 
+	err = RunMain([]string{cmdName, "enroll", "-c", testYaml, "--tls.certfiles", rootCert, "--tls.client.keyfile", tlsClientKeyFile, "--tls.client.certfile", tlsClientCertExpired, "-u", "https://admin:adminpw@localhost:7054", "-d"})
+	if err == nil {
+		t.Errorf("Expired certificate used for TLS connection, should have failed")
+	}
+
 	err = srv.Stop()
 	if err != nil {
 		t.Errorf("Server stop failed: %s", err)

diff --git a/lib/tls/tls.go b/lib/tls/tls.go
@@ -22,6 +22,7 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"time"
 
 	"github.com/cloudflare/cfssl/log"
 	"github.com/hyperledger/fabric-ca/util"
@@ -59,15 +60,25 @@ type KeyCertFiles struct {
 func GetClientTLSConfig(cfg *ClientTLSConfig) (*tls.Config, error) {
 	var certs []tls.Certificate
 
-	log.Debugf("CA Files: %s\n", cfg.CertFiles)
+	log.Debugf("CA Files: %s\n", cfg.CertFilesList)
 	log.Debugf("Client Cert File: %s\n", cfg.Client.CertFile)
 	log.Debugf("Client Key File: %s\n", cfg.Client.KeyFile)
-	clientCert, err := tls.LoadX509KeyPair(cfg.Client.CertFile, cfg.Client.KeyFile)
-	if err != nil {
-		log.Debugf("Client Cert or Key not provided, if server requires mutual TLS, the connection will fail: %s", err)
-	}
 
-	certs = append(certs, clientCert)
+	if cfg.Client.CertFile != "" && cfg.Client.KeyFile != "" {
+		err := checkCertDates(cfg.Client.CertFile)
+		if err != nil {
+			return nil, err
+		}
+
+		clientCert, err := tls.LoadX509KeyPair(cfg.Client.CertFile, cfg.Client.KeyFile)
+		if err != nil {
+			return nil, err
+		}
+
+		certs = append(certs, clientCert)
+	} else {
+		log.Debug("Client TLS certificate and/or key file not provided")
+	}
 
 	rootCAPool := x509.NewCertPool()
 
@@ -118,3 +129,30 @@ func AbsTLSClient(cfg *ClientTLSConfig, configDir string) error {
 
 	return nil
 }
+
+func checkCertDates(certFile string) error {
+	log.Debug("Check client TLS certificate for valid dates")
+	certPEM, err := ioutil.ReadFile(certFile)
+	if err != nil {
+		return err
+	}
+
+	cert, err := util.GetX509CertificateFromPEM(certPEM)
+	if err != nil {
+		return err
+	}
+
+	notAfter := cert.NotAfter
+	currentTime := time.Now().UTC()
+
+	if currentTime.After(notAfter) {
+		return errors.New("Certificate provided has expired")
+	}
+
+	notBefore := cert.NotBefore
+	if currentTime.Before(notBefore) {
+		return errors.New("Certificate provided not valid until later date")
+	}
+
+	return nil
+}
diff --git a/testdata/expiredcert.pem b/testdata/expiredcert.pem
@@ -0,0 +1,33 @@
+-----BEGIN CERTIFICATE-----
+MIIFpzCCA4+gAwIBAgIJAPckH7nSDuuVMA0GCSqGSIb3DQEBCwUAMGoxCzAJBgNV
+BAYTAlVTMRcwFQYDVQQIDA5Ob3J0aCBDYXJvbGluYTEQMA4GA1UEBwwHUmFsZWln
+aDEMMAoGA1UECgwDSUJNMRMwEQYDVQQLDApCbG9ja2NoYWluMQ0wCwYDVQQDDARz
+YWFkMB4XDTE3MDMyMTE0MTkwNFoXDTE3MDMyMjE0MTkwNFowajELMAkGA1UEBhMC
+VVMxFzAVBgNVBAgMDk5vcnRoIENhcm9saW5hMRAwDgYDVQQHDAdSYWxlaWdoMQww
+CgYDVQQKDANJQk0xEzARBgNVBAsMCkJsb2NrY2hhaW4xDTALBgNVBAMMBHNhYWQw
+ggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDRLs+jael13J9ieGL2q6XU
+iMbgd/YUTnPVpdFAXIEJCcRBB25/5e6jQXbevUXG/QIl3HBv9YYmtXKaQZLeL4TO
+JTdCHMMGOKaB+foGPDeXLSibn9FTBYMJd5fjC2c5cdL/8vjSMlV0BRpiWfeixJgO
+g/o/qvGxOz74S5EWPj7ox8HjfO8epadiaZ58INAuFGRjOzMwy1aDMYj/CUR80/O4
+9SmLDdMbeceZqvE5iCSx08Eu1e+kjysvTqt5B7K2NqEsOqX0bBb6ViOLTD2nnGfQ
+MVDtpJGirvV1s/rYauTfoU0CF3/pO7y/QZFukE1Kp+wHx/SGjZU5974hGgFPF9BQ
+R77ei5Sh9sNw9ia5IqRirRpOOsCTqiRwiE3guhLdvuPL7TsLashMBSqq0yjdSCIz
+I3aZCY/qkh6TgCjoztuuHaoaAsF9iCWO2jlbI963omV+inmRaiQIDu9UlhygG3XY
+qGNh1Ue7rbSK/lNiLDRnxDYCgT7OZ7JYhIZZNoyGSQHx+Y4qQnWevAE7F1ysTtHi
+824Zbjz2bmbqZq1RVLQ0mTZ9IUGTnftPe8LVBUhKbTGHsjQaT4Qm0pocnPSw9bg7
+MBraYMEDvQjp/ME6KDhpNbDbPsaKvwz3XbqP7/KFkWNhFJjntfcT7RGU15bC1xJA
+63USl5IY8nFwGIN18RVBDQIDAQABo1AwTjAdBgNVHQ4EFgQUtKsl77wYVhKatmIg
+D0SdCKxnPV8wHwYDVR0jBBgwFoAUtKsl77wYVhKatmIgD0SdCKxnPV8wDAYDVR0T
+BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAgEAeU9L+mfT1Id7K5ve/sbZ9Uh1vSZu
+oWU7EpZsAvUX6TEWL4Mjuf7Z1heSirgYwP9TYy/s6cjsKbuJyqvnD9pn8LIDAwVc
+KTWwyK3PL74OtgC5k9M4WJYsolHuSWxvrbYpWI8NFi9H84H9og+I9yEhY4chzc/P
+W7VWKkAZT8HwsZi1+aipS8jREDEI+g9MUiOnjCWYaK0C13czjeAQ9eV5wZ5WVrOR
+mtCWBHDSoN2QMgyV5SrMyN47hBPmyiku06XIdPNOIGBeucvUduFr6xo9Mn2b1WZa
+HW0pnqeG6yQzpzIujPPepg4/D4JIASSxzfhoAri2dpGzAPaa1VY9zsDkSOWsdDxy
+GmOjDdF8j2fwjXvn2nwOmVKgQaxcpzJ7Dz6o5EK6pQvo6O4GrdSvAii9jDlbDY7i
+N+qB8f7VJ3xXLBoKiiqoklYwRjRaEhIiym+BmiOh/EYVIXsU9pe5opPWNMhUnbI6
+FBlCxea5tO04GLhMGuSh1SyCbBx+XbzMv3PBpORcDshV11yjb7xiyJJk/L2b82bO
+Debwe/HBflcFCzQSwYKsZ/WdmRkKZbKgUvy6/JIEz/mvFsK2nJZBK1orQHNJPS09
+kgVh172RqWg0r4lXUVH96xxVIAsoZ/zphQ3gPPXq/MtKw7geAcNicCKKbWPLUKdv
+PYm9fooFAN1mGcg=
+-----END CERTIFICATE-----
diff --git a/testdata/fabric-ca-client-config.yaml b/testdata/fabric-ca-client-config.yaml
@@ -38,10 +38,10 @@
 URL: http://localhost:7054
 
 #############################################################################
-#    TLS section for the client's listenting port
+#    TLS section for secure socket connection
 #############################################################################
 tls:
-  # TLS for the client's listenting port
+  # TLS section for secure socket connection
   certfiles: root.pem   # Comma Separated list of root certificate files (e.g. root.pem, root2.pem)
   client:
     certfile: tls_client-cert.pem