From c9372be1269ded11a90b95d1e9067e3eea7992c5 Mon Sep 17 00:00:00 2001 From: Saad Karim Date: Tue, 6 Jun 2017 15:26:19 -0400 Subject: [PATCH] [FAB-4404] Adding CA to server restricted on DN Currently adding a CA required having a unique common name (CN) for each CA in the same server process. However, a less restrictive approach is required because a CSR can be signed by different root CAs. Logic was modified to check that if the issuer distinguished name (DN) is different than an intermediate CA can have the same subject distinguished name as another CA in the same server process. However, if the issuer DN is the same than the intermediate CA must use a unique subject DN. Also fixed missing CA configuration file path, currently error message was printing empty string. See [FAB-4404] for more info Change-Id: I77bfe48af35e599c3fd9898513d05da042e62add Signed-off-by: Saad Karim --- cmd/fabric-ca-server/main.go | 3 +- lib/ca.go | 7 ++- lib/server.go | 113 ++++++++++++++++++++++++++++++++--- lib/server_test.go | 93 ++++++++++++++++++++++++++++ 4 files changed, 205 insertions(+), 11 deletions(-) diff --git a/cmd/fabric-ca-server/main.go b/cmd/fabric-ca-server/main.go index bbf2db0d2..2760f84b4 100644 --- a/cmd/fabric-ca-server/main.go +++ b/cmd/fabric-ca-server/main.go @@ -107,7 +107,8 @@ func getServer() *lib.Server { Config: serverCfg, BlockingStart: blockingStart, CA: lib.CA{ - Config: &serverCfg.CAcfg, + Config: &serverCfg.CAcfg, + ConfigFilePath: cfgFileName, }, } } diff --git a/lib/ca.go b/lib/ca.go index 6fa3b2e43..9728c0313 100644 --- a/lib/ca.go +++ b/lib/ca.go @@ -69,7 +69,7 @@ type CA struct { // The CA's configuration Config *CAConfig // The file path of the config file - configFilePath string + ConfigFilePath string // The database handle used to store certificates and optionally // the user registry information, unless LDAP it enabled for the // user registry function. @@ -96,9 +96,10 @@ const ( // NewCA creates a new CA with the specified // home directory, parent server URL, and config -func NewCA(homeDir string, config *CAConfig, server *Server, renew bool) (*CA, error) { +func NewCA(caFile string, config *CAConfig, server *Server, renew bool) (*CA, error) { ca := new(CA) - err := initCA(ca, homeDir, config, server, renew) + ca.ConfigFilePath = caFile + err := initCA(ca, filepath.Dir(caFile), config, server, renew) if err != nil { return nil, err } diff --git a/lib/server.go b/lib/server.go index a2c07f85e..f6f398026 100644 --- a/lib/server.go +++ b/lib/server.go @@ -19,6 +19,7 @@ package lib import ( "crypto/tls" "crypto/x509" + "crypto/x509/pkix" "errors" "fmt" "io/ioutil" @@ -26,7 +27,6 @@ import ( "net/http" _ "net/http/pprof" // import to support profiling "os" - "path" "path/filepath" "strconv" "strings" @@ -294,7 +294,7 @@ func (s *Server) loadCA(caFile string, renew bool) error { log.Debugf("CA configuration after checking for missing values: %+v", cfg) - ca, err := NewCA(path.Dir(caFile), cfg, s, renew) + ca, err := NewCA(caFile, cfg, s, renew) if err != nil { return err } @@ -302,19 +302,24 @@ func (s *Server) loadCA(caFile string, renew bool) error { return s.addCA(ca) } +// DN is the distinguished name inside a certificate +type DN struct { + issuer string + subject string +} + // addCA adds a CA to the server if there are no conflicts func (s *Server) addCA(ca *CA) error { // check for conflicts caName := ca.Config.CA.Name - cn := ca.Config.CSR.CN for _, c := range s.caMap { if c.Config.CA.Name == caName { return fmt.Errorf("CA name '%s' is used in '%s' and '%s'", - caName, ca.configFilePath, c.configFilePath) + caName, ca.ConfigFilePath, c.ConfigFilePath) } - if c.Config.CSR.CN == cn { - return fmt.Errorf("Common name '%s' is used in '%s' and '%s'", - cn, ca.configFilePath, c.configFilePath) + err := s.compareDN(c.Config.CA.Certfile, ca.Config.CA.Certfile) + if err != nil { + return err } } // no conflicts, so add it @@ -567,3 +572,97 @@ func (s *Server) closeListener() error { log.Debugf("Stopped waiting for server to stop") return nil } + +func (s *Server) compareDN(existingCACertFile, newCACertFile string) error { + log.Debugf("Comparing DNs from certificates: %s and %s", existingCACertFile, newCACertFile) + existingDN, err := s.loadDNFromCertFile(existingCACertFile) + if err != nil { + return err + } + + newDN, err := s.loadDNFromCertFile(newCACertFile) + if err != nil { + return err + } + + err = existingDN.equal(newDN) + if err != nil { + return fmt.Errorf("Please modify CSR in %s and try adding CA again: %s", newCACertFile, err) + } + return nil +} + +func (s *Server) loadDNFromCertFile(certFile string) (*DN, error) { + log.Debugf("Loading DNs from certificate %s", certFile) + cert, err := util.GetX509CertificateFromPEMFile(certFile) + if err != nil { + return nil, err + } + issuerDN, err := s.getDNFromCert(cert.Issuer, "/") + if err != nil { + return nil, err + } + subjectDN, err := s.getDNFromCert(cert.Subject, "/") + if err != nil { + return nil, err + } + distinguishedName := &DN{ + issuer: issuerDN, + subject: subjectDN, + } + return distinguishedName, nil +} + +func (dn *DN) equal(checkDN *DN) error { + log.Debugf("Check to see if two DNs are equal - %+v and %+v", dn, checkDN) + if dn.issuer == checkDN.issuer { + log.Debug("Issuer distinguished name already in use, checking for unique subject distinguished name") + if dn.subject == checkDN.subject { + return errors.New("Both issuer and subject distinguished name are already in use") + } + } + return nil +} + +func (s *Server) getDNFromCert(namespace pkix.Name, sep string) (string, error) { + subject := []string{} + for _, s := range namespace.ToRDNSequence() { + for _, i := range s { + if v, ok := i.Value.(string); ok { + if name, ok := oid[i.Type.String()]; ok { + // = + subject = append(subject, fmt.Sprintf("%s=%s", name, v)) + } else { + // = if no is found + subject = append(subject, fmt.Sprintf("%s=%s", i.Type.String(), v)) + } + } else { + // = if value is not string + subject = append(subject, fmt.Sprintf("%s=%v", i.Type.String(), v)) + } + } + } + return sep + strings.Join(subject, sep), nil +} + +var oid = map[string]string{ + "2.5.4.3": "CN", + "2.5.4.4": "SN", + "2.5.4.5": "serialNumber", + "2.5.4.6": "C", + "2.5.4.7": "L", + "2.5.4.8": "ST", + "2.5.4.9": "streetAddress", + "2.5.4.10": "O", + "2.5.4.11": "OU", + "2.5.4.12": "title", + "2.5.4.17": "postalCode", + "2.5.4.42": "GN", + "2.5.4.43": "initials", + "2.5.4.44": "generationQualifier", + "2.5.4.46": "dnQualifier", + "2.5.4.65": "pseudonym", + "0.9.2342.19200300.100.1.25": "DC", + "1.2.840.113549.1.9.1": "emailAddress", + "0.9.2342.19200300.100.1.1": "userid", +} diff --git a/lib/server_test.go b/lib/server_test.go index aa0b076f3..38ea1ae15 100644 --- a/lib/server_test.go +++ b/lib/server_test.go @@ -683,6 +683,82 @@ func TestDefaultMultiCA(t *testing.T) { } } +// Test the combination of multiple CAs in both root and intermediate servers. +func TestMultiCAIntermediates(t *testing.T) { + myTestDir := "multicaIntermediates" + os.RemoveAll(myTestDir) + defer os.RemoveAll(myTestDir) + // Start the root server with 2 non-default CAs + rootServer := TestGetServer(rootPort, path.Join(myTestDir, "root"), "", -1, t) + rootServer.Config.CAcount = 2 + err := rootServer.Start() + if err != nil { + t.Fatalf("Failed to start root CA: %s", err) + } + home := path.Join(myTestDir, "intermediate") + parentURL := fmt.Sprintf("http://admin:adminpw@localhost:%d", rootPort) + intServer := TestGetServer(0, home, parentURL, -1, t) + intServer.Config.CAfiles = []string{"ca1/ca1.yaml", "ca2/ca2.yaml"} + ca1home := filepath.Join(home, "ca1") + ca2home := filepath.Join(home, "ca2") + + // Negative Case - same CA name from two intermediate CAs sent to the same root CA + // Check that CA file paths are getting printed + // Create ca1.yaml and ca2.yaml for the intermediate server CAs + writeCAFile("ca1", "ca1", "ca1", ca1home, rootPort, t) + writeCAFile("ca1", "ca2", "ca2", ca2home, rootPort, t) + err = intServer.Init(false) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), ca1home) + assert.Contains(t, err.Error(), ca2home) + } + err = rootServer.Stop() + if err != nil { + t.Error("Failed to stop server: ", err) + } + + os.RemoveAll(myTestDir) + + // Negative Case - same subject distinguished name from two intermediate CAs sent to the same root CA + // Create ca1.yaml and ca2.yaml for the intermediate server CAs + rootServer = TestGetServer(rootPort, path.Join(myTestDir, "root"), "", -1, t) // reset server from last run above + rootServer.Config.CAcount = 2 + err = rootServer.Start() + if err != nil { + t.Fatalf("Failed to start root CA: %s", err) + } + writeCAFile("ca1", "ca1", "ca1", ca1home, rootPort, t) + writeCAFile("ca2", "ca1", "ca2", ca2home, rootPort, t) + err = intServer.Init(false) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "Both issuer and subject distinguished name are already in use") + } + err = rootServer.Stop() + if err != nil { + t.Error("Failed to stop server: ", err) + } + os.RemoveAll(myTestDir) + + // Positive Case - same subject distinguished names from two intermediate CAs sent to two different root CAs + rootServer = TestGetServer(rootPort, path.Join(myTestDir, "root"), "", -1, t) // reset server from last run above + rootServer.Config.CAcount = 2 + err = rootServer.Start() + if err != nil { + t.Fatalf("Failed to start root CA: %s", err) + } + writeCAFile("ca1", "ca1", "ca1", ca1home, rootPort, t) + writeCAFile("ca2", "ca2", "ca2", ca2home, rootPort, t) + // Init the intermediate server + err = intServer.Init(false) + if err != nil { + t.Fatalf("Failed to initialize intermediate server: %s", err) + } + err = rootServer.Stop() + if err != nil { + t.Error("Failed to stop server: ", err) + } +} + func TestMaxEnrollmentInfinite(t *testing.T) { os.RemoveAll(rootDir) t.Log("Test max enrollment infinite") @@ -1297,3 +1373,20 @@ func makeAttrs(t *testing.T, args ...string) []api.Attribute { } return attrs } + +func writeCAFile(name, parentcaname, filename, home string, port int, t *testing.T) { + contents := fmt.Sprintf(` +ca: + name: %s +intermediate: + parentserver: + url: http://admin:adminpw@localhost:%d + caname: %s +`, name, port, parentcaname) + os.MkdirAll(home, 0755) + fpath := path.Join(home, fmt.Sprintf("%s.yaml", filename)) + err := ioutil.WriteFile(fpath, []byte(contents), 0644) + if err != nil { + t.Fatalf("Failed to create ca1.yaml: %s", err) + } +}