feedback

scionproto · Dec 6, 2019 · 3822ae3 · 3822ae3
1 parent 8d33b02
commit 3822ae3
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 70 deletions.
diff --git a/go/tools/scion-pki/internal/v2/certs/chain.go b/go/tools/scion-pki/internal/v2/certs/chain.go
@@ -45,11 +45,11 @@ func (g chainGen) Run(asMap pkicmn.ASMap) error {
 	if err != nil {
 		return serrors.WrapStr("unable to load AS certificate configs", err)
 	}
-	certs, err := g.Generate(cfgs)
+	certs, err := g.generateAll(cfgs)
 	if err != nil {
 		return serrors.WrapStr("unable to generate AS certificates", err)
 	}
-	if err := g.Sign(certs, cfgs); err != nil {
+	if err := g.signAll(certs, cfgs); err != nil {
 		return serrors.WrapStr("unable to sign AS certificates", err)
 	}
 	if err := g.verify(certs); err != nil {
@@ -64,7 +64,7 @@ func (g chainGen) Run(asMap pkicmn.ASMap) error {
 	return nil
 }
 
-func (g chainGen) Generate(cfgs map[addr.IA]conf.AS) (map[addr.IA]chainMeta, error) {
+func (g chainGen) generateAll(cfgs map[addr.IA]conf.AS) (map[addr.IA]chainMeta, error) {
 	certs := make(map[addr.IA]chainMeta)
 	for ia, cfg := range cfgs {
 		signed, err := g.generate(ia, cfg)
@@ -82,7 +82,7 @@ func (g chainGen) generate(ia addr.IA, cfg conf.AS) (chainMeta, error) {
 	if err != nil {
 		return chainMeta{}, serrors.WrapStr("unable to load all public keys", err)
 	}
-	enc, err := cert.EncodeAS(g.newCert(ia, cfg, pubKeys))
+	enc, err := cert.EncodeAS(newASCert(ia, cfg, pubKeys))
 	if err != nil {
 		return chainMeta{}, serrors.WrapStr("unable to encode AS certificate", err)
 	}
@@ -153,27 +153,7 @@ func (g chainGen) loadPubKey(id keyconf.ID) (keyconf.Key, error) {
 	return key, nil
 }
 
-func (g chainGen) newCert(ia addr.IA, cfg conf.AS, pubKeys map[cert.KeyType]keyconf.Key) *cert.AS {
-	val := cfg.Validity.Eval(time.Now())
-	c := &cert.AS{
-		Base: cert.Base{
-			Subject:                    ia,
-			Version:                    cfg.Version,
-			FormatVersion:              1,
-			Description:                cfg.Description,
-			OptionalDistributionPoints: cfg.OptDistPoints,
-			Validity:                   &val,
-			Keys:                       translateKeys(pubKeys),
-		},
-		Issuer: cert.IssuerCertID{
-			IA:                 cfg.IssuerIA,
-			CertificateVersion: cfg.IssuerCertVersion,
-		},
-	}
-	return c
-}
-
-func (g chainGen) Sign(protos map[addr.IA]chainMeta, cfgs map[addr.IA]conf.AS) error {
+func (g chainGen) signAll(protos map[addr.IA]chainMeta, cfgs map[addr.IA]conf.AS) error {
 	for ia, meta := range protos {
 		var err error
 		if meta.Chain, err = g.sign(cfgs[ia], meta.Chain); err != nil {
@@ -252,3 +232,23 @@ func (g chainGen) write(certs map[addr.IA]chainMeta) error {
 	}
 	return nil
 }
+
+func newASCert(ia addr.IA, cfg conf.AS, pubKeys map[cert.KeyType]keyconf.Key) *cert.AS {
+	val := cfg.Validity.Eval(time.Now())
+	c := &cert.AS{
+		Base: cert.Base{
+			Subject:                    ia,
+			Version:                    cfg.Version,
+			FormatVersion:              1,
+			Description:                cfg.Description,
+			OptionalDistributionPoints: cfg.OptDistPoints,
+			Validity:                   &val,
+			Keys:                       translateKeys(pubKeys),
+		},
+		Issuer: cert.IssuerCertID{
+			IA:                 cfg.IssuerIA,
+			CertificateVersion: cfg.IssuerCertVersion,
+		},
+	}
+	return c
+}
diff --git a/go/tools/scion-pki/internal/v2/certs/chain_test.go b/go/tools/scion-pki/internal/v2/certs/chain_test.go
@@ -33,6 +33,21 @@ var (
 	chainASMap = pkicmn.ASMap{1: {ia111}}
 )
 
+// TestChainGenRun checks that the chain generator creates verifiable chains.
+//
+// Given the folders:
+// - ISD1                   with the trc config
+// - ISD1/trcs              with the issuing TRC
+// - ISD1/ASff00_0_110      with the issuer AS, all its configs, keys and issuer certificate
+// - ISD1/ASff00_0_111      with the AS certifcate config
+//
+// When running chain.Run with the AS map that contains AS 1-ff00:0:111.
+//
+// Then a certificate chain is generated under ISD1/ASff00_0_111/certs.
+// The certificate chain is:
+// - valid
+// - verifiable using TRC ISD1-V1.trc that can be found at ISD1/trcs
+// - Byte for byte the same as the golden file.
 func TestChainGenRun(t *testing.T) {
 	tmpDir, cleanF := xtest.MustTempDir("", "test-certs-chain")
 	defer cleanF()

diff --git a/go/tools/scion-pki/internal/v2/certs/cmd.go b/go/tools/scion-pki/internal/v2/certs/cmd.go
@@ -90,10 +90,7 @@ var genChainCmd = &cobra.Command{
 		if err != nil {
 			return serrors.WrapStr("unable to select target ISDs", err, "selector", args[0])
 		}
-		if err := g.Run(asMap); err != nil {
-			return serrors.WrapStr("unable to generate certificate chains", err)
-		}
-		return nil
+		return g.Run(asMap)
 	},
 }
 

diff --git a/go/tools/scion-pki/internal/v2/certs/issuer.go b/go/tools/scion-pki/internal/v2/certs/issuer.go
@@ -81,7 +81,7 @@ func (g issGen) generate(ia addr.IA, cfg conf.Issuer) (issMeta, error) {
 	if err != nil {
 		return issMeta{}, serrors.WrapStr("unable to load all public keys", err)
 	}
-	enc, err := cert.EncodeIssuer(g.newCert(ia, cfg, pubKeys))
+	enc, err := cert.EncodeIssuer(newIssuerCert(ia, cfg, pubKeys))
 	if err != nil {
 		return issMeta{}, serrors.WrapStr("unable to encode issuer certificate", err)
 	}
@@ -131,27 +131,6 @@ func (g issGen) loadPubKey(id keyconf.ID) (keyconf.Key, error) {
 	return key, nil
 }
 
-func (g issGen) newCert(ia addr.IA, cfg conf.Issuer,
-	pubKeys map[cert.KeyType]keyconf.Key) *cert.Issuer {
-
-	val := cfg.Validity.Eval(time.Now())
-	c := &cert.Issuer{
-		Base: cert.Base{
-			Subject:                    ia,
-			Version:                    cfg.Version,
-			FormatVersion:              1,
-			Description:                cfg.Description,
-			OptionalDistributionPoints: cfg.OptDistPoints,
-			Validity:                   &val,
-			Keys:                       translateKeys(pubKeys),
-		},
-		Issuer: cert.IssuerTRC{
-			TRCVersion: cfg.TRCVersion,
-		},
-	}
-	return c
-}
-
 func (g issGen) signAll(protos map[addr.IA]issMeta, cfgs map[addr.IA]conf.Issuer) error {
 	for ia, meta := range protos {
 		var err error
@@ -234,3 +213,23 @@ func (g issGen) write(certs map[addr.IA]issMeta) error {
 	}
 	return nil
 }
+
+func newIssuerCert(ia addr.IA, cfg conf.Issuer, pubKeys map[cert.KeyType]keyconf.Key) *cert.Issuer {
+
+	val := cfg.Validity.Eval(time.Now())
+	c := &cert.Issuer{
+		Base: cert.Base{
+			Subject:                    ia,
+			Version:                    cfg.Version,
+			FormatVersion:              1,
+			Description:                cfg.Description,
+			OptionalDistributionPoints: cfg.OptDistPoints,
+			Validity:                   &val,
+			Keys:                       translateKeys(pubKeys),
+		},
+		Issuer: cert.IssuerTRC{
+			TRCVersion: cfg.TRCVersion,
+		},
+	}
+	return c
+}
diff --git a/go/tools/scion-pki/internal/v2/certs/loader.go b/go/tools/scion-pki/internal/v2/certs/loader.go
@@ -55,14 +55,14 @@ func (l loader) LoadIssuerConfigs(asMap pkicmn.ASMap) (map[addr.IA]conf.Issuer,
 }
 
 func (l loader) LoadASConfigs(asMap pkicmn.ASMap) (map[addr.IA]conf.AS, error) {
+	s := selector{
+		File:  conf.ASFile,
+		All:   conf.AllASFiles,
+		Regex: `as-v(\d*)\.toml$`,
+	}
 	cfgs := make(map[addr.IA]conf.AS)
 	for _, ias := range asMap {
 		for _, ia := range ias {
-			s := selector{
-				File:  conf.ASFile,
-				All:   conf.AllASFiles,
-				Regex: `as-v(\d*)\.toml$`,
-			}
 			file, err := l.selectConfig(ia, s)
 			if err != nil {
 				return nil, serrors.WrapStr("unable to select config", err, "ia", ia)

diff --git a/go/tools/scion-pki/internal/v2/certs/loader_test.go b/go/tools/scion-pki/internal/v2/certs/loader_test.go
@@ -55,8 +55,8 @@ func TestLoaderLoadASConfigs(t *testing.T) {
 		"v1":  {Version: 1, Expected: 1},
 		"max": {Version: 0, Expected: 1},
 	}
-	for name, test := range tests {
-		name, test := name, test
+	for n, tc := range tests {
+		name, test := n, tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 			l := loader{

diff --git a/go/tools/scion-pki/internal/v2/certs/verify.go b/go/tools/scion-pki/internal/v2/certs/verify.go
@@ -37,21 +37,21 @@ func (v verifier) VerifyIssuer(raw []byte) error {
 	if err != nil {
 		return serrors.WrapStr("unable to parse signed issuer certificate", err)
 	}
-	if _, err = v.verifyIssuer(signed); err != nil {
-		return err
-	}
-	return nil
+	return v.verifyIssuer(signed)
 }
 
 func (v verifier) VerifyChain(raw []byte) error {
 	chain, err := cert.ParseChain(raw)
 	if err != nil {
 		return serrors.WrapStr("unable to parse signed certificate chain", err)
 	}
-	issCert, err := v.verifyIssuer(chain.Issuer)
-	if err != nil {
+	if err := v.verifyIssuer(chain.Issuer); err != nil {
 		return err
 	}
+	issCert, err := chain.Issuer.Encoded.Decode()
+	if err != nil {
+		return serrors.WrapStr("unable to parse issuer certificate payload", err)
+	}
 	asCert, err := chain.AS.Encoded.Decode()
 	if err != nil {
 		return serrors.WrapStr("unable to parse AS certificate payload", err)
@@ -70,27 +70,27 @@ func (v verifier) VerifyChain(raw []byte) error {
 	return nil
 }
 
-func (v verifier) verifyIssuer(signed cert.SignedIssuer) (*cert.Issuer, error) {
+func (v verifier) verifyIssuer(signed cert.SignedIssuer) error {
 	c, err := signed.Encoded.Decode()
 	if err != nil {
-		return nil, serrors.WrapStr("unable to parse issuer certificate payload", err)
+		return serrors.WrapStr("unable to parse issuer certificate payload", err)
 	}
 	if err := c.Validate(); err != nil {
-		return nil, serrors.WrapStr("unable to validate issuer certificate", err)
+		return serrors.WrapStr("unable to validate issuer certificate", err)
 	}
 	t, err := v.loadTRC(c.Subject.I, c.Issuer.TRCVersion)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	issVer := cert.IssuerVerifier{
 		Issuer:       c,
 		SignedIssuer: &signed,
 		TRC:          t,
 	}
 	if err := issVer.Verify(); err != nil {
-		return nil, serrors.WrapStr("unable to verify issuer certificate", err)
+		return serrors.WrapStr("unable to verify issuer certificate", err)
 	}
-	return c, nil
+	return nil
 }
 
 func (v verifier) loadTRC(isd addr.ISD, version scrypto.Version) (*trc.TRC, error) {