a more robust sorting solution

certs.go

@@ -381,60 +381,118 @@ func IsRootCA(cert *x509.Certificate) bool {
 
 // SortCerts sorts a []*x509.Certificate from leaf to root CA, or the other
 // way around if a the supplied boolean is set to true. Double elements are
-// removed. Sort will look for a single leaf in the []*x509.Certificate and
-// a chain from there. If no single leaf can be found, the chain will start
-// with the first element of the given []*x509.Certificate.
+// removed.
 func SortCerts(certs []*x509.Certificate, reverse bool) []*x509.Certificate {
-	var ordered []*x509.Certificate
+	chainAsCerts, certByName, order := SortCertsAsChains(certs, reverse)
 
-	// Find leaf
-	leaf, err := FindLeaf(certs)
-	if err == nil {
-		certs = append([]*x509.Certificate{leaf}, certs...)
+	var orderedFromLeaves []*x509.Certificate
+	var orderedNoLeaves []*x509.Certificate
+	for _, subj := range order {
+		if ! certByName[subj].IsCA {
+			orderedFromLeaves = append(orderedFromLeaves, chainAsCerts[subj]...)
+		} else {
+			orderedNoLeaves = append(orderedNoLeaves, chainAsCerts[subj]...)
+		}
 	}
 
+	var ordered []*x509.Certificate
+	tmpOrdered := append(orderedFromLeaves, orderedNoLeaves...)
+	seen := make(map[string]bool)
+	for _, cert := range tmpOrdered {
+		if _, ok := seen[cert.Subject.String()]; ok {
+			continue
+		}
+		ordered = append(ordered, cert)
+		seen[cert.Subject.String()] = true
+	}
+
+	return ordered
+}
+
+// SortCertsAsChains sorts a []*x509.Certificate from leaf to root CA, or the other
+// way around if a the boolean parameter is set to true. The function returns three
+// elements: a map[string][]*x509.Certificate with the subject as key and the chain as
+// value, a map[string]*x509.Certificate with the the subject as key and the
+// corresponding as value *x509.Certificate and a []string with Subjects that start the
+// chain in the order the certificates where given.
+// TODO: write test
+func SortCertsAsChains(
+	certs []*x509.Certificate, reverse bool) (map[string][]*x509.Certificate, map[string]*x509.Certificate, []string) {
 	// Get the information needed to follow the chain
-	parentName := make(map[string]string)
+	var certByNameOrder []string
+	issuerName := make(map[string]string)
 	certByName := make(map[string]*x509.Certificate)
+	isLeaf := make(map[string]bool)
 	for _, cert := range certs {
-		if _, ok := certByName[cert.Subject.String()]; ok {
+		subj := cert.Subject.String()
+		issuer := cert.Issuer.String()
+		if _, ok := certByName[subj]; ok {
 			continue
 		}
-		certByName[cert.Subject.String()] = cert
-		parentName[cert.Subject.String()] = cert.Issuer.String()
+		if !cert.IsCA {
+			isLeaf[subj] = true
+		}
+		certByName[subj] = cert
+		issuerName[subj] = issuer
+		certByNameOrder = append(certByNameOrder, subj)
 	}
 
-	seen := make(map[string]bool)
-	for _, cert := range certs {
-		if _, ok := seen[cert.Subject.String()]; ok {
+	// Create chains
+	var order []string
+	chain := make(map[string][]string)
+	skip := make(map[string]bool)
+	for subj, issuer := range issuerName {
+		if _, ok := skip[subj]; ok {
 			continue
 		}
-		ordered = append(ordered, cert)
-		seen[cert.Subject.String()] = true
-		for { // follow the chain
-			_, ok := certByName[parentName[cert.Subject.String()]] // we have that cert
-			_, ok2 := seen[parentName[cert.Subject.String()]]      // the parent has not been seen
-			if ok && !ok2 {
-				// do we have the next Issuer (e.g. incomplete chain
-				if _, ok := certByName[parentName[cert.Subject.String()]]; ok {
-					ordered = append(ordered, certByName[parentName[cert.Subject.String()]])
-					seen[parentName[cert.Subject.String()]] = true
-					cert = certByName[parentName[cert.Subject.String()]]
-					continue
+
+		skip[issuer] = true // we follow the issuers below
+		chain[subj] = []string{subj}
+		order = append(order, subj)
+		presentIssuer := issuer
+		for {
+			if _, ok := certByName[subj]; !ok {
+				continue
+			}
+
+			tmpChain := []string{}
+			tmpChain = append(tmpChain, chain[subj]...)
+			tmpChain = append(tmpChain, presentIssuer)
+			skip[presentIssuer] = true
+			chain[subj] = tmpChain
+			delete(chain, presentIssuer)
+
+			if nextIssuer, ok := issuerName[presentIssuer]; ok {
+				if nextIssuer == presentIssuer { // end of this chain
+					break
 				}
+
+				presentIssuer = nextIssuer
+				continue
 			}
 			break
 		}
 	}
 
-	if reverse {
-		var reversed []*x509.Certificate
-		for idx := len(ordered) - 1; idx >= 0; idx-- {
-			reversed = append(reversed, ordered[idx])
+	chainAsCerts := make(map[string][]*x509.Certificate)
+	for subj, chainElems := range chain {
+		var ordered []*x509.Certificate
+		for _, chainElem := range chainElems {
+			if cert, ok := certByName[chainElem]; ok {
+				ordered = append(ordered, cert)
+			}
+		}
+		if reverse {
+			var reversed []*x509.Certificate
+			for idx := len(ordered) - 1; idx >= 0; idx-- {
+				reversed = append(reversed, ordered[idx])
+			}
+			ordered = reversed
 		}
-		return reversed
+		chainAsCerts[subj] = ordered
 	}
-	return ordered
+
+	return chainAsCerts, certByName, order
 }
 
 // SplitCertsAsTree returns a *CertTree where the given certificates
@@ -539,4 +597,4 @@ func getPKCS8PEMBlock(parsedKey interface{}) (*pem.Block, error) {
 		Bytes: parsedBytes,
 	}
 	return &pemBlock, nil
-}
+}

certs_test.go

@@ -278,14 +278,16 @@ func TestSortCerts(t *testing.T) {
 	assert.NoError(t, err)
 
 	ordered := SortCerts(certs, false)
-	assert.NotNil(t, ordered)
-	assert.Equal(t, 7, len(ordered))
-	assert.Equal(t, testGeantSerial, ordered[1].SerialNumber.String())
+	if assert.NotNil(t, ordered) {
+		assert.Equal(t, 7, len(ordered))
+		assert.Contains(t, ordered[0].Subject.CommonName, "exporl.med.kuleuven.be")
+	}
 
 	ordered = SortCerts(certs, true)
-	assert.NotNil(t, ordered)
-	assert.Equal(t, 7, len(ordered))
-	assert.Equal(t, testGeantSerial, ordered[len(ordered)-2].SerialNumber.String())
+	if assert.NotNil(t, ordered) {
+		assert.Equal(t, 7, len(ordered))
+		assert.Contains(t, ordered[0].Subject.CommonName, "AAA Certificate Services")
+	}
 }
 
 func TestSplitCertsAsTree(t *testing.T) {
@@ -402,4 +404,4 @@ func TestGetPKCS8PEMBlock(t *testing.T) {
 	assert.NoError(t, err)
 	assert.NotNil(t, pemBlock)
 	assert.Equal(t, "RSA PRIVATE KEY", pemBlock.Type)
-}
+}

cmd/certmin/actions.go

@@ -48,11 +48,30 @@ func skimCerts(locations []string, params Params) (string, error) {
 			}
 		}
 
-		switch {
-		case params.sort:
-			certs = certmin.SortCerts(certs, false)
-		case params.rsort:
-			certs = certmin.SortCerts(certs, true)
+		if params.sort || params.rsort {
+			if params.once {
+				switch {
+				case params.sort:
+					certs = certmin.SortCerts(certs, false)
+				case params.rsort:
+					certs = certmin.SortCerts(certs, true)
+				}
+			} else {
+				var chainAsCerts map[string][]*x509.Certificate
+				var order []string
+				switch {
+				case params.sort:
+					chainAsCerts, _, order = certmin.SortCertsAsChains(certs, false)
+				case params.rsort:
+					chainAsCerts, _, order = certmin.SortCertsAsChains(certs, true)
+				}
+
+				var tmpCerts []*x509.Certificate
+				for _, subj := range order {
+					tmpCerts = append(tmpCerts, chainAsCerts[subj]...)
+				}
+				certs = tmpCerts
+			}
 		}
 
 		for idx, cert := range certs {

cmd/certmin/cli.go

@@ -16,7 +16,7 @@ See ` + website + ` for more information.
 Usage:
   certmin skim cert-location1 [cert-location2...] 
     [--leaf|--follow] [--no-roots]
-    [--sort|--rsort] [--keep] [--no-colour]
+    [--sort|--rsort] [--once] [--keep] [--no-colour]
   certmin verify-chain cert-location [cert-location2...]
     [--root=ca-file1 --root=ca-file2...]
     [--inter=inter-file1 --inter=inter-file2...]
@@ -47,6 +47,10 @@ Global options (optional):
   --inter     | -i  : intermediate certificate file(s).
   --sort      | -s  : sort the certificates and chains from leaf to root.
   --rsort     | -z  : sort the certificates and chains from root to leaf.
+  --once      | -o  : if within a location several certificates share an
+                      intermediate/root, don't show certificates more than
+                      once to visually complete the chain. If "rsort" not
+                      given it enables "sort".
   --keep      | -k  : write the requested certificates and chains to files
                       as PKCS1 PEM files (converting if necessary).
   --no-colour | -c  : don't colourise the output.
@@ -55,8 +59,8 @@ Global options (optional):
 `
 
 type Params struct {
-	help, progVersion, leaf, follow, noRoots, sort, rsort, keep bool
-	roots, inters                                               []string
+	help, progVersion, leaf, follow, noRoots, sort, rsort, once, keep bool
+	roots, inters                                                     []string
 }
 
 // getAction returns an action function, a msg for early exit and an error.
@@ -78,6 +82,7 @@ func getAction() (actionFunc, string, error) {
 	noRoots := flags.BoolP("no-roots", "n", false, "")
 	sort := flags.BoolP("sort", "s", false, "")
 	rsort := flags.BoolP("rsort", "z", false, "")
+	once := flags.BoolP("once", "o", false, "")
 	keep := flags.BoolP("keep", "k", false, "")
 	noColour := flags.BoolP("no-colour", "c", false, "")
 
@@ -90,6 +95,10 @@ func getAction() (actionFunc, string, error) {
 		color.NoColor = true // disables colorized output
 	}
 
+	if *once && !*rsort {
+		*sort = true
+	}
+
 	all := append(*roots, *inters...)
 	var notFound []string
 	for _, cert := range all {
@@ -109,6 +118,7 @@ func getAction() (actionFunc, string, error) {
 		noRoots:     *noRoots,
 		sort:        *sort,
 		rsort:       *rsort,
+		once:        *once,
 		keep:        *keep,
 		roots:       *roots,
 		inters:      *inters,
@@ -147,6 +157,8 @@ func verifyAndDispatch(params Params, args []string) (actionFunc, string, error)
 		return nil, "", errors.New("--leaf and --follow are mutually exclusive")
 	case params.sort && params.rsort:
 		return nil, "", errors.New("--sort and --rsort are mutually exclusive")
+	case params.once && !(params.sort || params.rsort):
+		return nil, "", errors.New("--once requires --sort and --rsort")
 	case len(args) < 3:
 		return nil, "", errors.New("no certificate location given")