control/controlclient: return correct certificate

When searching for the matching client identity, the returned
certificate chain was accidentally set to that of the last identity
returned by the certificate store instead of the one corresponding to
the selected identity.

Also, add some extra error checking for invalid certificate chains, just
in case.

Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>

control/controlclient/sign_supported.go

@@ -51,40 +51,61 @@ var (
 	errBadRequest = errors.New("malformed request")
 )
 
-// findIdentity locates an identity from the Windows or Darwin certificate
-// store. It returns the first certificate with a matching Subject anywhere in
-// its certificate chain, so it is possible to search for the leaf certificate,
-// intermediate CA or root CA. If err is nil then the returned identity will
-// never be nil (if no identity is found, the error errNoMatch will be
-// returned). If an identity is returned then its certificate chain is also
-// returned.
-func findIdentity(subject string, st certstore.Store) (certstore.Identity, []*x509.Certificate, error) {
-	ids, err := st.Identities()
-	if err != nil {
-		return nil, nil, err
+func isSupportedCertificate(cert *x509.Certificate) bool {
+	return cert.PublicKeyAlgorithm == x509.RSA
+}
+
+func isSubjectInChain(subject string, chain []*x509.Certificate) bool {
+	if len(chain) == 0 || chain[0] == nil {
+		return false
+	}
+
+	for _, c := range chain {
+		if c == nil {
+			continue
+		}
+		if c.Subject.String() == subject {
+			return true
+		}
 	}
 
-	var selected certstore.Identity
-	var chain []*x509.Certificate
+	return false
+}
 
+func selectIdentityFromSlice(subject string, ids []certstore.Identity) (certstore.Identity, []*x509.Certificate) {
 	for _, id := range ids {
-		chain, err = id.CertificateChain()
+		chain, err := id.CertificateChain()
 		if err != nil {
 			continue
 		}
 
-		if chain[0].PublicKeyAlgorithm != x509.RSA {
+		if !isSupportedCertificate(chain[0]) {
 			continue
 		}
 
-		for _, c := range chain {
-			if c.Subject.String() == subject {
-				selected = id
-				break
-			}
+		if isSubjectInChain(subject, chain) {
+			return id, chain
 		}
 	}
 
+	return nil, nil
+}
+
+// findIdentity locates an identity from the Windows or Darwin certificate
+// store. It returns the first certificate with a matching Subject anywhere in
+// its certificate chain, so it is possible to search for the leaf certificate,
+// intermediate CA or root CA. If err is nil then the returned identity will
+// never be nil (if no identity is found, the error errNoMatch will be
+// returned). If an identity is returned then its certificate chain is also
+// returned.
+func findIdentity(subject string, st certstore.Store) (certstore.Identity, []*x509.Certificate, error) {
+	ids, err := st.Identities()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	selected, chain := selectIdentityFromSlice(subject, ids)
+
 	for _, id := range ids {
 		if id != selected {
 			id.Close()