Skip to content

Commit

Permalink
Add Subject Key Id field to CA certificate
Browse files Browse the repository at this point in the history
The SKI field is mandatory for CA certificates, but Go crypto library for versions < 1.15
is not filling in the SKI field for CAs [1] this PR replicate the golang >= 1.15 fix [2].

Replicated from qinqon/kube-admission-webhook#42

[1] golang/go#26676
[2] https://go-review.googlesource.com/c/go/+/227098/

Signed-off-by: Quique Llorente <ellorent@redhat.com>
  • Loading branch information
qinqon committed Oct 14, 2020
1 parent 4ace288 commit 6fd8b42
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 0 deletions.
15 changes: 15 additions & 0 deletions pkg/certificates/triple/cert/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"crypto/rand"
cryptorand "crypto/rand"
"crypto/rsa"
"crypto/sha1"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
Expand Down Expand Up @@ -63,6 +64,14 @@ func NewPrivateKey() (*rsa.PrivateKey, error) {
return rsa.GenerateKey(cryptorand.Reader, rsaKeySize)
}

func generateSKIFromRSAPublicKey(key crypto.Signer) []byte {
rsaPublicKey := key.Public().(*rsa.PublicKey)

publicKeyBytes := x509.MarshalPKCS1PublicKey(rsaPublicKey)
h := sha1.Sum(publicKeyBytes)
return h[:]
}

// NewSelfSignedCACert creates a CA certificate
func NewSelfSignedCACert(cfg Config, key crypto.Signer, duration time.Duration) (*x509.Certificate, error) {
now := time.Now()
Expand All @@ -79,6 +88,12 @@ func NewSelfSignedCACert(cfg Config, key crypto.Signer, duration time.Duration)
IsCA: true,
}

// Golang < 1.15 is not filling in the SKI field for CAs [1] so we replicate the golang >= 1.15 fix [2]
//
// [1] https://github.com/golang/go/issues/26676
// [2] https://go-review.googlesource.com/c/go/+/227098/
tmpl.SubjectKeyId = generateSKIFromRSAPublicKey(key)

certDERBytes, err := x509.CreateCertificate(cryptorand.Reader, &tmpl, &tmpl, key.Public(), key)
if err != nil {
return nil, err
Expand Down
15 changes: 15 additions & 0 deletions pkg/certificates/triple/triple_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package triple

import (
"testing"

. "github.com/onsi/ginkgo"
"github.com/onsi/ginkgo/reporters"
. "github.com/onsi/gomega"
)

func TestTriple(t *testing.T) {
RegisterFailHandler(Fail)
junitReporter := reporters.NewJUnitReporter("junit.triple_suite_test.xml")
RunSpecsWithDefaultAndCustomReporters(t, "Certificate Test Suite", []Reporter{junitReporter})
}
44 changes: 44 additions & 0 deletions pkg/certificates/triple/triple_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package triple

import (
"crypto/x509"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

var _ = Describe("Cert library", func() {
Context("when NewCA is called", func() {
var (
name string
duration time.Duration
now time.Time
)
BeforeEach(func() {
now = time.Now()
name = "foo-bar-name"
duration = time.Minute
})
It("should generate key and CA cert with expected fields", func() {

keyAndCert, err := NewCA(name, duration)
Expect(err).ToNot(HaveOccurred(), "should succeed generating CA")

privateKey := keyAndCert.Key
caCert := keyAndCert.Cert

Expect(privateKey).ToNot(BeNil(), "should generate a private key")
Expect(caCert).ToNot(BeNil(), "should generate a CA certificate")
Expect(caCert.SerialNumber.Int64()).To(Equal(int64(0)), "should have zero as serial number")
Expect(caCert.Subject.CommonName).To(Equal(name), "should take CommonName from name field")
Expect(caCert.NotBefore).To(BeTemporally("~", now.UTC(), time.Second), "should set NotBefore to now")
Expect(caCert.NotAfter).To(BeTemporally("~", now.Add(duration).UTC(), time.Second), "should set NotAfter to now + duration")
Expect(caCert.KeyUsage).To(Equal(x509.KeyUsageKeyEncipherment|x509.KeyUsageDigitalSignature|x509.KeyUsageCertSign), "should set proper KeyUsage")
Expect(caCert.BasicConstraintsValid).To(BeTrue(), "should mark it as BasicConstraintsValid")
Expect(caCert.IsCA).To(BeTrue(), "should mark it as CA")
Expect(caCert.SubjectKeyId).ToNot(BeEmpty(), "should include a SKI")
})

})
})

0 comments on commit 6fd8b42

Please sign in to comment.