Skip to content

Commit

Permalink
Avoid generating a CA keypair on-demand
Browse files Browse the repository at this point in the history
Instead we must explicitly create it; this avoids races where we are
reading the private key and creating CA certs.

Issue kubernetes#3875
  • Loading branch information
justinsb committed Nov 26, 2017
1 parent 875b416 commit e3c7f03
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 14 deletions.
10 changes: 3 additions & 7 deletions upup/pkg/fi/clientset_castore.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ func NewClientsetCAStore(cluster *kops.Cluster, clientset kopsinternalversion.Ko
return c
}

// readCAKeypairs retrieves the CA keypair, generating a new keypair if not found
// readCAKeypairs retrieves the CA keypair.
// (No longer generates a keypair if not found.)
func (c *ClientsetCAStore) readCAKeypairs(id string) (*keyset, error) {
c.mutex.Lock()
defer c.mutex.Unlock()
Expand All @@ -78,14 +79,9 @@ func (c *ClientsetCAStore) readCAKeypairs(id string) (*keyset, error) {
}

if keyset == nil {
keyset, err = c.generateCACertificate(id)
if err != nil {
return nil, err
}

return nil, nil
}
c.cachedCaKeysets[id] = keyset

return keyset, nil
}

Expand Down
11 changes: 10 additions & 1 deletion upup/pkg/fi/fitasks/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "go_default_library",
Expand Down Expand Up @@ -27,3 +27,12 @@ go_library(
"//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
],
)

go_test(
name = "go_default_test",
size = "small",
srcs = ["keypair_test.go"],
importpath = "k8s.io/kops/upup/pkg/fi/fitasks",
library = ":go_default_library",
deps = ["//upup/pkg/fi:go_default_library"],
)
44 changes: 44 additions & 0 deletions upup/pkg/fi/fitasks/keypair_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
Copyright 2017 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package fitasks

import (
"k8s.io/kops/upup/pkg/fi"
"strings"
"testing"
)

func TestKeypairDeps(t *testing.T) {
ca := &Keypair{}
cert := &Keypair{
Signer: ca,
}

tasks := make(map[string]fi.Task)
tasks["ca"] = ca
tasks["cert"] = cert

deps := fi.FindTaskDependencies(tasks)

if strings.Join(deps["ca"], ",") != "" {
t.Errorf("unexpected dependencies for ca: %v", deps["ca"])
}

if strings.Join(deps["cert"], ",") != "ca" {
t.Errorf("unexpected dependencies for cert: %v", deps["cert"])
}
}
11 changes: 5 additions & 6 deletions upup/pkg/fi/vfs_castore.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (s *VFSCAStore) VFSPath() vfs.Path {
return s.basedir
}

// Retrieves the CA keypair, generating a new keypair if not found
// Retrieves the CA keypair. No longer generates keypairs if not found.
func (s *VFSCAStore) readCAKeypairs(id string) (*certificates, *privateKeys, error) {
s.mutex.Lock()
defer s.mutex.Unlock()
Expand Down Expand Up @@ -98,16 +98,15 @@ func (s *VFSCAStore) readCAKeypairs(id string) (*certificates, *privateKeys, err
}

if caPrivateKeys == nil {
caCertificates, caPrivateKeys, err = s.generateCACertificate(id)
if err != nil {
return nil, nil, err
}

// We no longer generate CA certificates automatically - too race-prone
return caCertificates, caPrivateKeys, nil
}

cached = &cachedEntry{certificates: caCertificates, privateKeys: caPrivateKeys}
s.cachedCAs[id] = cached

return cached.certificates, cached.privateKeys, nil

}

func BuildCAX509Template() *x509.Certificate {
Expand Down

0 comments on commit e3c7f03

Please sign in to comment.