cleaned-up x509util

3 files changed, 63 insertions, 66 deletions

diff --git a/client/client.go b/client/client.go
index 9462e0c..6eb99c2 100644
--- a/client/client.go
+++ b/client/client.go
@@ -46,12 +46,20 @@ func NewClient(log *descriptor.Log, client *http.Client, useHttp bool, chain []*
 // private key, and a json-encoded list of log operators (see descriptor).
 // Chain and key paths may be left out by passing the empty string: "".
 func NewClientFromPath(logId, chainPath, keyPath, operatorsPath string, cli *http.Client, useHttp bool) (*Client, error) {
-	c, err := x509util.LoadCertificates(chainPath)
+	pem, err := ioutil.ReadFile(chainPath)
 	if err != nil && chainPath != "" {
+		return nil, fmt.Errorf("failed reading %s: %v", chainPath, err)
+	}
+	c, err := x509util.NewCertificateList(pem)
+	if err != nil {
 		return nil, err
 	}
 
-	k, err := x509util.LoadEd25519SigningKey(keyPath)
+	pem, err = ioutil.ReadFile(keyPath)
+	if err != nil && keyPath != "" {
+		return nil, fmt.Errorf("failed reading %s: %v", keyPath, err)
+	}
+	k, err := x509util.NewEd25519PrivateKey(pem)
 	if err != nil && keyPath != "" {
 		return nil, err
 	}
diff --git a/instance.go b/instance.go
index 7a353fe..6732698 100644
--- a/instance.go
+++ b/instance.go
@@ -8,6 +8,7 @@ import (
 	"crypto/sha256"
 	"crypto/x509"
 	"encoding/base64"
+	"io/ioutil"
 	"net/http"
 
 	"github.com/golang/glog"
@@ -61,12 +62,16 @@ func NewInstance(lp *LogParameters, client trillian.TrillianLogClient, deadline
 
 // NewLogParameters initializes log parameters, assuming ed25519 signatures.
 func NewLogParameters(treeId int64, prefix string, anchorPath, keyPath string, maxRange, maxChain int64) (*LogParameters, error) {
-	anchorList, anchorPool, err := x509util.LoadTrustAnchors(anchorPath)
+	anchorList, anchorPool, err := loadTrustAnchors(anchorPath)
 	if err != nil {
 		return nil, err
 	}
 
-	key, err := x509util.LoadEd25519SigningKey(keyPath)
+	pem, err := ioutil.ReadFile(keyPath)
+	if err != nil {
+		return nil, fmt.Errorf("failed reading %s: %v", keyPath, err)
+	}
+	key, err := x509util.NewEd25519PrivateKey(pem)
 	if err != nil {
 		return nil, err
 	}
@@ -109,3 +114,16 @@ func (i *Instance) registerHandlers(mux *http.ServeMux) {
 		mux.Handle(endpoint.path, endpoint.handler)
 	}
 }
+
+// loadTrustAnchors loads a list of PEM-encoded certificates from file
+func loadTrustAnchors(path string) ([]*x509.Certificate, *x509.CertPool, error) {
+	pem, err := ioutil.ReadFile(path)
+	if err != nil {
+		return nil, nil, fmt.Errorf("failed reading trust anchors: %v", err)
+	}
+	anchorList, err := x509util.NewCertificateList(pem)
+	if err != nil || len(anchorList) == 0 {
+		return nil, nil, fmt.Errorf("failed parsing trust anchors: %v", err)
+	}
+	return anchorList, x509util.NewCertPool(anchorList), nil
+}
diff --git a/x509util/x509util.go b/x509util/x509util.go
index c3ebd4b..57d97ca 100644
--- a/x509util/x509util.go
+++ b/x509util/x509util.go
@@ -6,44 +6,34 @@ import (
 	"crypto/ed25519"
 	"crypto/x509"
 	"encoding/pem"
-	"io/ioutil"
 )
 
-// TODO: remove LoadCertificates
-// LoadCertificates loads a PEM-encoded list of certificates from file
-func LoadCertificates(path string) ([]*x509.Certificate, error) {
-	pem, err := ioutil.ReadFile(path)
-	if err != nil {
-		return nil, fmt.Errorf("failed reading certificate chain: %v", err)
+// NewEd25519PrivateKey creates a new Ed25519 private-key from a PEM block
+func NewEd25519PrivateKey(data []byte) (ed25519.PrivateKey, error) {
+	block, rest := pem.Decode(data)
+	if block == nil {
+		return nil, fmt.Errorf("pem block: is empty")
 	}
-	return NewCertificateList(pem)
-}
-
-// TODO: remove LoadTrustAnchors
-// LoadTrustAnchors loads a list of PEM-encoded certificates from file
-func LoadTrustAnchors(path string) ([]*x509.Certificate, *x509.CertPool, error) {
-	pem, err := ioutil.ReadFile(path)
-	if err != nil {
-		return nil, nil, fmt.Errorf("failed reading trust anchors: %v", err)
+	if block.Type != "PRIVATE KEY" {
+		return nil, fmt.Errorf("bad pem block type: %v", block.Type)
 	}
-	anchorList, err := NewCertificateList(pem)
-	if err != nil || len(anchorList) == 0 {
-		return nil, nil, fmt.Errorf("failed parsing trust anchors: %v", err)
+	if len(rest) != 0 {
+		return nil, fmt.Errorf("pem block: trailing data")
 	}
-	return anchorList, NewCertPool(anchorList), nil
-}
 
-// TODO: remove LoadEd25519SigningKey
-// LoadEd25519SigningKey loads an Ed25519 private key from a given path
-func LoadEd25519SigningKey(path string) (ed25519.PrivateKey, error) {
-	data, err := ioutil.ReadFile(path)
+	key, err := x509.ParsePKCS8PrivateKey(block.Bytes)
 	if err != nil {
-		return nil, fmt.Errorf("failed reading private key: %v", err)
+		fmt.Errorf("x509 parser failed: %v", err)
+	}
+	switch t := key.(type) {
+	case ed25519.PrivateKey:
+		return key.(ed25519.PrivateKey), nil
+	default:
+		return nil, fmt.Errorf("unexpected signing key type: %v", t)
 	}
-	return NewEd25519PrivateKey(data)
 }
 
-// NewCertificateList parses a block of PEM-encoded certificates
+// NewCertificateList parses a block of PEM-encoded X.509 certificates
 func NewCertificateList(rest []byte) ([]*x509.Certificate, error) {
 	var certificates []*x509.Certificate
 	for len(rest) > 0 {
@@ -74,34 +64,26 @@ func NewCertPool(certificates []*x509.Certificate) *x509.CertPool {
 	return pool
 }
 
-// NewEd25519PrivateKey creates a new ed25519 private-key from a PEM block
-func NewEd25519PrivateKey(data []byte) (ed25519.PrivateKey, error) {
-	block, rest := pem.Decode(data)
-	if block == nil {
-		return nil, fmt.Errorf("pem block: is empty")
-	}
-	if block.Type != "PRIVATE KEY" {
-		return nil, fmt.Errorf("bad pem block type: %v", block.Type)
-	}
-	if len(rest) != 0 {
-		return nil, fmt.Errorf("pem block: trailing data")
-	}
-
-	key, err := x509.ParsePKCS8PrivateKey(block.Bytes)
-	if err != nil {
-		fmt.Errorf("x509 parser failed: %v", err)
-	}
-	switch t := key.(type) {
-	case ed25519.PrivateKey:
-		return key.(ed25519.PrivateKey), nil
-	default:
-		return nil, fmt.Errorf("unexpected signing key type: %v", t)
+// VerifyChain checks whether the listed certificates are chained such
+// that the first is signed by the second, the second by the third, etc.
+//
+// Note: it is up to the caller to determine whether the final certificate
+// is a valid trust anchor.
+func VerifyChain(chain []*x509.Certificate) error {
+	for i := 0; i < len(chain)-1; i++ {
+		if err := chain[i].CheckSignatureFrom(chain[i+1]); err != nil {
+			return err
+		}
 	}
+	return nil
 }
 
 // ParseDerChain parses a list of DER-encoded X.509 certificates, such that the
 // first (zero-index) blob is interpretted as an end-entity certificate and
 // the remaining ones as its intermediate CertPool.
+//
+// Note: these are the parameters you will need to use x509.Certificate.Verify()
+// with x509.VerifyOptions that include both a pool of roots and intermediates.
 func ParseDerChain(chain [][]byte) (*x509.Certificate, *x509.CertPool, error) {
 	certificates, err := ParseDerList(chain)
 	if err != nil {
@@ -117,7 +99,7 @@ func ParseDerChain(chain [][]byte) (*x509.Certificate, *x509.CertPool, error) {
 	return certificates[0], intermediatePool, nil
 }
 
-// ParseDerList parses a list of DER-encoded certificates
+// ParseDerList parses a list of DER-encoded X.509 certificates
 func ParseDerList(certificates [][]byte) ([]*x509.Certificate, error) {
 	ret := make([]*x509.Certificate, 0, len(certificates))
 	for _, der := range certificates {
@@ -129,14 +111,3 @@ func ParseDerList(certificates [][]byte) ([]*x509.Certificate, error) {
 	}
 	return ret, nil
 }
-
-// VerifyChain checks whether the listed certificates are chained such
-// that the first is signed by the second, the second by the third, etc.
-func VerifyChain(chain []*x509.Certificate) error {
-	for i := 0; i < len(chain)-1; i++ {
-		if err := chain[i].CheckSignatureFrom(chain[i+1]); err != nil {
-			return err
-		}
-	}
-	return nil
-}