Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provide and use compiled in test certs (only) in tests #757

Merged
merged 1 commit into from
Apr 23, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions client/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2015 The Cockroach 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. See the AUTHORS file
// for names of contributors.
//
// Author: Tobias Schottdorf ([email protected])

package client

import (
"github.com/cockroachdb/cockroach/security"
"github.com/cockroachdb/cockroach/security/securitytest"
)

func init() {
security.SetReadFileFn(securitytest.Asset)
}
27 changes: 27 additions & 0 deletions kv/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2015 The Cockroach 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. See the AUTHORS file
// for names of contributors.
//
// Author: Tobias Schottdorf ([email protected])

package kv

import (
"github.com/cockroachdb/cockroach/security"
"github.com/cockroachdb/cockroach/security/securitytest"
)

func init() {
security.SetReadFileFn(securitytest.Asset)
}
27 changes: 27 additions & 0 deletions rpc/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2015 The Cockroach 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. See the AUTHORS file
// for names of contributors.
//
// Author: Tobias Schottdorf ([email protected])

package rpc

import (
"github.com/cockroachdb/cockroach/security"
"github.com/cockroachdb/cockroach/security/securitytest"
)

func init() {
security.SetReadFileFn(securitytest.Asset)
}
2 changes: 1 addition & 1 deletion rpc/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (

// NewTestContext returns a rpc.Context for testing.
func NewTestContext(t *testing.T) *Context {
tlsConfig, err := security.LoadTLSConfigFromDir(security.EmbeddedPrefix + "test_certs")
tlsConfig, err := security.LoadTLSConfigFromDir("test_certs")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of leaking this constant "test_certs" directory, why not just have another security method called: security.LoadTLSConfigForTests? Then in security:

func LoadTLSConfigForTests() (*TLSConfig, error) {
  return LoadTLSConfigFromDir("test_certs")
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or make it an exported constant in the security package.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm currently refactoring some of this with much fewer TLS setups. I'll move the test path to an exported const.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mberhault is refactoring the stuff anyways, I'll wait for the dust to settle. What's more worrying is that we currently do compile in the test certs everywhere, because it's used in the rpc/testing package. Will change that though.

if err != nil {
t.Fatal(err)
}
Expand Down
7 changes: 7 additions & 0 deletions security/certs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ import (

// This is just the mechanics of certs generation.
func TestGenerateCerts(t *testing.T) {
// Do not mock cert access for this test.
security.ResetReadFileFn()
defer security.ResetTest()

certsDir := util.CreateTempDir(t, "certs_test")
defer util.CleanupDir(certsDir)

Expand Down Expand Up @@ -63,6 +67,9 @@ func TestGenerateCerts(t *testing.T) {
// This is a fairly high-level test of CA and node certificates.
// We construct SSL server and clients and use the generated certs.
func TestUseCerts(t *testing.T) {
// Do not mock cert access for this test.
security.ResetReadFileFn()
defer security.ResetTest()
certsDir := util.CreateTempDir(t, "certs_test")
defer util.CleanupDir(certsDir)

Expand Down
30 changes: 30 additions & 0 deletions security/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2015 The Cockroach 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. See the AUTHORS file
// for names of contributors.
//
// Author: Tobias Schottdorf ([email protected])

package security

import "github.com/cockroachdb/cockroach/security/securitytest"

func init() {
ResetTest()
}

// ResetTest sets up the test environment. In particular, it embeds the
// test_certs folder and makes the tls package load from there.
func ResetTest() {
SetReadFileFn(securitytest.Asset)
}
62 changes: 18 additions & 44 deletions security/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,25 @@ import (
"crypto/x509"
"io/ioutil"
"path"
"strings"
"sync"

"github.com/cockroachdb/cockroach/security/securitytest"
"github.com/cockroachdb/cockroach/util"
)

const (
// EmbeddedPrefix is the prefix to the -certs arg that indicates embedded certs.
EmbeddedPrefix = "embedded="
)
// readFileFn is used to mock out file system access during tests.
var readFileFn = ioutil.ReadFile

// SetReadFileFn allows to switch out ioutil.ReadFile by a mock
// for testing purposes.
func SetReadFileFn(f func(string) ([]byte, error)) {
readFileFn = f
}

// ResetReadFileFn is the counterpart to SetReadFileFn, restoring the
// original behaviour for loading certificate related data from disk.
func ResetReadFileFn() {
readFileFn = ioutil.ReadFile
}

// TLSConfig contains the TLS settings for a Cockroach node. Currently it's
// just a wrapper for tls.Config. If config is nil, we don't use TLS.
Expand All @@ -62,36 +70,15 @@ func (c *TLSConfig) Config() *tls.Config {
// - node.key -- the private key of this node
// If the path is prefixed with "embedded=", load the embedded certs.
func LoadTLSConfigFromDir(certDir string) (*TLSConfig, error) {
if strings.HasPrefix(certDir, EmbeddedPrefix) {
return LoadTestTLSConfig(certDir[len(EmbeddedPrefix):])
}
certPEM, err := ioutil.ReadFile(path.Join(certDir, "node.crt"))
certPEM, err := readFileFn(path.Join(certDir, "node.crt"))
if err != nil {
return nil, err
}
keyPEM, err := ioutil.ReadFile(path.Join(certDir, "node.key"))
keyPEM, err := readFileFn(path.Join(certDir, "node.key"))
if err != nil {
return nil, err
}
caPEM, err := ioutil.ReadFile(path.Join(certDir, "ca.crt"))
if err != nil {
return nil, err
}
return LoadTLSConfig(certPEM, keyPEM, caPEM)
}

// LoadTestTLSConfig loads the embedded certs. This is only called from
// LoadTLSConfigFromDir when the certdir path starts with "embedded=".
func LoadTestTLSConfig(certDir string) (*TLSConfig, error) {
certPEM, err := securitytest.Asset(path.Join(certDir, "node.crt"))
if err != nil {
return nil, err
}
keyPEM, err := securitytest.Asset(path.Join(certDir, "node.key"))
if err != nil {
return nil, err
}
caPEM, err := securitytest.Asset(path.Join(certDir, "ca.crt"))
caPEM, err := readFileFn(path.Join(certDir, "ca.crt"))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -147,20 +134,7 @@ func LoadInsecureTLSConfig() *TLSConfig {
// LoadClientTLSConfigFromDir creates a client TLSConfig by loading the root CA certs from the
// specified directory. The directory must contain ca.crt.
func LoadClientTLSConfigFromDir(certDir string) (*TLSConfig, error) {
if strings.HasPrefix(certDir, EmbeddedPrefix) {
return LoadTestClientTLSConfig(certDir[len(EmbeddedPrefix):])
}
caPEM, err := ioutil.ReadFile(path.Join(certDir, "ca.crt"))
if err != nil {
return nil, err
}
return LoadClientTLSConfig(caPEM)
}

// LoadTestClientTLSConfig loads the embedded certs. This is only called from
// LoadClientTLSConfigFromDir when the certdir path starts with "embedded=".
func LoadTestClientTLSConfig(certDir string) (*TLSConfig, error) {
caPEM, err := securitytest.Asset(path.Join(certDir, "ca.crt"))
caPEM, err := readFileFn(path.Join(certDir, "ca.crt"))
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion security/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
)

func TestLoadTLSConfig(t *testing.T) {
wrapperConfig, err := LoadTLSConfigFromDir(EmbeddedPrefix + "test_certs")
wrapperConfig, err := LoadTLSConfigFromDir("test_certs")
if err != nil {
t.Fatalf("Failed to load TLS config: %v", err)
}
Expand Down
5 changes: 2 additions & 3 deletions server/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"os"
"strings"

"github.com/cockroachdb/cockroach/security"
"github.com/cockroachdb/cockroach/server"
)

Expand All @@ -49,8 +48,8 @@ func (c cliTest) Run(line string) {
var args []string
args = append(args, a[0])
args = append(args, fmt.Sprintf("-addr=%s", c.ServingAddr()))
// Always load server certs. Not sure if this path is a good assumption though.
args = append(args, fmt.Sprintf("-certs=%s", security.EmbeddedPrefix+"test_certs"))
// Always load test certs.
args = append(args, fmt.Sprintf("-certs=%s", "test_certs"))
args = append(args, a[1:]...)

fmt.Printf("%s\n", line)
Expand Down
27 changes: 27 additions & 0 deletions server/cli/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2015 The Cockroach 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. See the AUTHORS file
// for names of contributors.
//
// Author: Tobias Schottdorf ([email protected])

package cli

import (
"github.com/cockroachdb/cockroach/security"
"github.com/cockroachdb/cockroach/security/securitytest"
)

func init() {
security.SetReadFileFn(securitytest.Asset)
}
27 changes: 27 additions & 0 deletions server/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2015 The Cockroach 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. See the AUTHORS file
// for names of contributors.
//
// Author: Tobias Schottdorf ([email protected])

package server

import (
"github.com/cockroachdb/cockroach/security"
"github.com/cockroachdb/cockroach/security/securitytest"
)

func init() {
security.SetReadFileFn(securitytest.Asset)
}
5 changes: 4 additions & 1 deletion server/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ import (
// not nil, the gossip bootstrap address is set to gossipBS.
func createTestNode(addr net.Addr, engines []engine.Engine, gossipBS net.Addr, t *testing.T) (
*rpc.Server, *hlc.Clock, *Node, *util.Stopper) {
tlsConfig, err := security.LoadTLSConfigFromDir(security.EmbeddedPrefix + "test_certs")
// Load the TLS config from our test certs. They're embedded in the
// test binary and calls to the file system have been mocked out,
// see main_test.go.
tlsConfig, err := security.LoadTLSConfigFromDir("test_certs")
if err != nil {
t.Fatal(err)
}
Expand Down
3 changes: 2 additions & 1 deletion server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ import (

func newTestContext() *Context {
newContext := NewContext()
newContext.Certs = security.EmbeddedPrefix + "test_certs"
// The certs are compiled in, see main_test.go.
newContext.Certs = "test_certs"
return newContext
}

Expand Down
9 changes: 6 additions & 3 deletions server/testserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/cockroachdb/cockroach/gossip"
"github.com/cockroachdb/cockroach/proto"
"github.com/cockroachdb/cockroach/security"
"github.com/cockroachdb/cockroach/storage"
"github.com/cockroachdb/cockroach/storage/engine"
"github.com/cockroachdb/cockroach/util"
Expand Down Expand Up @@ -55,8 +54,12 @@ func NewTestContext() *Context {
// uncertainty intervals.
ctx.MaxOffset = 0

// Load certs from embedded files.
ctx.Certs = security.EmbeddedPrefix + "test_certs"
// Load test certs. In addition, the tests requiring certs
// need to call security.SetReadFileFn(securitytest.Asset)
// in their init to mock out the file system calls for calls to AssetFS,
// which has the test certs compiled in. Typically this is done
// once per package, in main_test.go.
ctx.Certs = "test_certs"
// Addr defaults to localhost with port set at time of call to
// Start() to an available port.
// Call TestServer.ServingAddr() for the full address (including bound port).
Expand Down
6 changes: 6 additions & 0 deletions storage/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,17 @@ package storage
import (
"testing"

"github.com/cockroachdb/cockroach/security"
"github.com/cockroachdb/cockroach/security/securitytest"
"github.com/cockroachdb/cockroach/util/leaktest"
)

//go:generate ../util/leaktest/add-leaktest.sh *_test.go

func init() {
security.SetReadFileFn(securitytest.Asset)
}

func TestMain(m *testing.M) {
leaktest.TestMainWithLeakCheck(m)
}