From 649add1ec6030984e933ecb67808abb82dc79fc1 Mon Sep 17 00:00:00 2001
From: Aditya Mahendrakar <a.mahendrakar@yahoo.com>
Date: Fri, 4 Jun 2021 10:26:35 -0700
Subject: [PATCH] GenCACert - do not set fields with empty value (#84)

---
 x509cert/x509.go      |  26 ++++++--
 x509cert/x509_test.go | 141 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+), 5 deletions(-)

diff --git a/x509cert/x509.go b/x509cert/x509.go
index 1ba01d19..f5d46b97 100644
--- a/x509cert/x509.go
+++ b/x509cert/x509.go
@@ -23,14 +23,30 @@ func GenCACert(config *crypki.CAConfig, signer crypto.Signer, hostname string, i
 	start := uint64(time.Now().Unix())
 	end := start + config.ValidityPeriod
 	start -= 3600
+	var country, locality, province, org, orgUnit []string
+	if config.Country != "" {
+		country = []string{config.Country}
+	}
+	if config.Locality != "" {
+		locality = []string{config.Locality}
+	}
+	if config.State != "" {
+		province = []string{config.State}
+	}
+	if config.Organization != "" {
+		org = []string{config.Organization}
+	}
+	if config.OrganizationalUnit != "" {
+		orgUnit = []string{config.OrganizationalUnit}
+	}
 
 	subj := pkix.Name{
 		CommonName:         config.CommonName,
-		Country:            []string{config.Country},
-		Locality:           []string{config.Locality},
-		Province:           []string{config.State},
-		Organization:       []string{config.Organization},
-		OrganizationalUnit: []string{config.OrganizationalUnit},
+		Country:            country,
+		Locality:           locality,
+		Province:           province,
+		Organization:       org,
+		OrganizationalUnit: orgUnit,
 	}
 	template := &x509.Certificate{
 		Subject:               subj,
diff --git a/x509cert/x509_test.go b/x509cert/x509_test.go
index 521b0e5e..3fcbdd59 100644
--- a/x509cert/x509_test.go
+++ b/x509cert/x509_test.go
@@ -3,7 +3,15 @@
 package x509cert
 
 import (
+	"crypto"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
 	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"net"
+	"reflect"
 	"testing"
 
 	"github.com/theparanoids/crypki"
@@ -64,3 +72,136 @@ func TestGetPublicKeyAlgorithm(t *testing.T) {
 		})
 	}
 }
+
+func TestGenCACert(t *testing.T) {
+	t.Parallel()
+	pka := crypki.ECDSA
+	sa := crypki.ECDSAWithSHA384
+	eckey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	tests := map[string]struct {
+		cfg         *crypki.CAConfig
+		signer      crypto.Signer
+		hostname    string
+		ips         []net.IP
+		pka         crypki.PublicKeyAlgorithm
+		sa          crypki.SignatureAlgorithm
+		wantSubj    pkix.Name
+		expectError bool
+	}{
+		"all-fields": {
+			cfg: &crypki.CAConfig{
+				Country:            "US",
+				Locality:           "Sunnyvale",
+				State:              "CA",
+				Organization:       "Foo Org",
+				OrganizationalUnit: "Foo Org Unit",
+				CommonName:         "foo.example.com",
+			},
+			signer:   eckey,
+			hostname: "hostname.example.com",
+			pka:      pka,
+			sa:       sa,
+			wantSubj: pkix.Name{
+				CommonName:         "foo.example.com",
+				Country:            []string{"US"},
+				Locality:           []string{"Sunnyvale"},
+				Province:           []string{"CA"},
+				Organization:       []string{"Foo Org"},
+				OrganizationalUnit: []string{"Foo Org Unit"},
+			},
+		},
+		"no-ST": {
+			cfg: &crypki.CAConfig{
+				Country:            "US",
+				Locality:           "Sunnyvale",
+				Organization:       "Foo Org",
+				OrganizationalUnit: "Foo Org Unit",
+				CommonName:         "foo.example.com",
+			},
+			signer:   eckey,
+			hostname: "hostname.example.com",
+			pka:      pka,
+			sa:       sa,
+			wantSubj: pkix.Name{
+				CommonName:         "foo.example.com",
+				Country:            []string{"US"},
+				Locality:           []string{"Sunnyvale"},
+				Organization:       []string{"Foo Org"},
+				OrganizationalUnit: []string{"Foo Org Unit"},
+			},
+		},
+		"no-L": {
+			cfg: &crypki.CAConfig{
+				Country:            "US",
+				State:              "CA",
+				Organization:       "Foo Org",
+				OrganizationalUnit: "Foo Org Unit",
+				CommonName:         "foo.example.com",
+			},
+			signer:   eckey,
+			hostname: "hostname.example.com",
+			pka:      pka,
+			sa:       sa,
+			wantSubj: pkix.Name{
+				CommonName:         "foo.example.com",
+				Country:            []string{"US"},
+				Province:           []string{"CA"},
+				Organization:       []string{"Foo Org"},
+				OrganizationalUnit: []string{"Foo Org Unit"},
+			},
+		},
+		"no-Org": {
+			cfg: &crypki.CAConfig{
+				Country:            "US",
+				Locality:           "Sunnyvale",
+				State:              "CA",
+				OrganizationalUnit: "Foo Org Unit",
+				CommonName:         "foo.example.com",
+			},
+			signer:   eckey,
+			hostname: "hostname.example.com",
+			pka:      pka,
+			sa:       sa,
+			wantSubj: pkix.Name{
+				CommonName:         "foo.example.com",
+				Country:            []string{"US"},
+				Locality:           []string{"Sunnyvale"},
+				Province:           []string{"CA"},
+				OrganizationalUnit: []string{"Foo Org Unit"},
+			},
+		},
+		// TODO: add tests to validate other fields in the ca cert, including the signature.
+	}
+	for name, tt := range tests {
+		name, tt := name, tt
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			got, err := GenCACert(tt.cfg, tt.signer, tt.hostname, tt.ips, tt.pka, tt.sa)
+			if err != nil {
+				if !tt.expectError {
+					t.Error("unexpected error")
+				}
+			}
+			if tt.expectError {
+				t.Error("expected error")
+			}
+			block, _ := pem.Decode(got)
+			if block == nil || block.Type != "CERTIFICATE" {
+				t.Error("unable to decode PEM block containing the certificate")
+			}
+			cert, err := x509.ParseCertificate(block.Bytes)
+			if err != nil {
+				t.Error("failed to parse certificate: " + err.Error())
+			}
+			// skip checking Names field in subject
+			tt.wantSubj.Names = cert.Subject.Names
+			if !reflect.DeepEqual(cert.Subject, tt.wantSubj) {
+				t.Errorf("subject mismatch:\n got: \n%+v\n want: \n%+v\n", cert.Subject, tt.wantSubj)
+			}
+		})
+	}
+
+}