Skip to content

Commit

Permalink
Avoid nil dereferencing when tlsConfig is nil. (#11614)
Browse files Browse the repository at this point in the history
  • Loading branch information
Alex McGrath authored Apr 2, 2022
1 parent 5d9c01b commit ae9e230
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 9 deletions.
7 changes: 7 additions & 0 deletions lib/kube/proxy/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@ func extractKubeCreds(ctx context.Context, cluster string, clientCfg *rest.Confi
if err != nil {
return nil, trace.Wrap(err, "failed to generate TLS config from kubeconfig: %v", err)
}
if tlsConfig == nil {
cc := rest.AnonymousClientConfig(clientCfg)
if len(cc.CAData) != 0 {
cc.CAData = []byte("REDACTED")
}
return nil, trace.BadParameter("failed to generate TLS config from kubeConfig. clientConfig: %s", cc.String())
}
transportConfig, err := clientCfg.TransportConfig()
if err != nil {
return nil, trace.Wrap(err, "failed to generate transport config from kubeconfig: %v", err)
Expand Down
87 changes: 78 additions & 9 deletions lib/kube/proxy/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,16 @@ limitations under the License.
package proxy

import (
"bufio"
"bytes"
"context"
"crypto/tls"
"errors"
"io/ioutil"
"fmt"
"io/fs"
"os"
"path/filepath"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -129,12 +135,20 @@ func failsForCluster(clusterName string) ImpersonationPermissionsChecker {
func TestGetKubeCreds(t *testing.T) {
ctx := context.TODO()
const teleClusterName = "teleport-cluster"
testDir := t.TempDir()

tmpFile, err := ioutil.TempFile("", "teleport")
cert, err := utils.GenerateSelfSignedCert([]string{"localhost"})
require.NoError(t, err)
defer os.Remove(tmpFile.Name())
kubeconfigPath := tmpFile.Name()
_, err = tmpFile.Write([]byte(`

certFilePath := filepath.Join(testDir, "certfile")
require.NoError(t, os.WriteFile(certFilePath, cert.Cert, fs.ModePerm))
keyFilePath := filepath.Join(testDir, "keyfile")
require.NoError(t, os.WriteFile(keyFilePath, cert.PrivateKey, fs.ModePerm))
tlsConfig, err := utils.CreateTLSConfiguration(certFilePath, keyFilePath, utils.DefaultCipherSuites())
require.NoError(t, err)

kubeconfigPath := filepath.Join(testDir, "kubeconf")
require.NoError(t, os.WriteFile(kubeconfigPath, []byte(fmt.Sprintf(`
apiVersion: v1
kind: Config
clusters:
Expand All @@ -156,10 +170,42 @@ contexts:
name: baz
users:
- name: creds
user:
client-certificate: %s
client-key: %s
current-context: foo
`))
require.NoError(t, err)
require.NoError(t, tmpFile.Close())
`, certFilePath, keyFilePath)), fs.ModePerm))

kubeconfigbadPath := filepath.Join(testDir, "kubeconfbad")
require.NoError(t, os.WriteFile(kubeconfigbadPath, []byte(`
apiVersion: v1
kind: Config
clusters:
- cluster:
server: https://example.com:3026
name: example
contexts:
- context:
cluster: example
user: creds
name: foo
- context:
cluster: example
user: creds
name: bar
- context:
cluster: example
user: creds
name: baz
users:
- name: creds
current-context: foo
`), fs.ModePerm))

logger := utils.NewLoggerForTests()
buf := bytes.NewBuffer([]byte{})
logger.SetOutput(buf)
sc := bufio.NewScanner(buf)

tests := []struct {
desc string
Expand Down Expand Up @@ -194,16 +240,19 @@ current-context: foo
impersonationCheck: alwaysSucceeds,
want: map[string]*kubeCreds{
"foo": {
tlsConfig: tlsConfig,
targetAddr: "example.com:3026",
transportConfig: &transport.Config{},
kubeClient: &kubernetes.Clientset{},
},
"bar": {
tlsConfig: tlsConfig,
targetAddr: "example.com:3026",
transportConfig: &transport.Config{},
kubeClient: &kubernetes.Clientset{},
},
"baz": {
tlsConfig: tlsConfig,
targetAddr: "example.com:3026",
transportConfig: &transport.Config{},
kubeClient: &kubernetes.Clientset{},
Expand All @@ -224,6 +273,7 @@ current-context: foo
impersonationCheck: alwaysSucceeds,
want: map[string]*kubeCreds{
teleClusterName: {
tlsConfig: tlsConfig,
targetAddr: "example.com:3026",
transportConfig: &transport.Config{},
kubeClient: &kubernetes.Clientset{},
Expand All @@ -237,27 +287,45 @@ current-context: foo
impersonationCheck: failsForCluster("bar"),
want: map[string]*kubeCreds{
"foo": {
tlsConfig: tlsConfig,
targetAddr: "example.com:3026",
transportConfig: &transport.Config{},
kubeClient: &kubernetes.Clientset{},
},
"bar": {
tlsConfig: tlsConfig,
targetAddr: "example.com:3026",
transportConfig: &transport.Config{},
kubeClient: &kubernetes.Clientset{},
},
"baz": {
tlsConfig: tlsConfig,
targetAddr: "example.com:3026",
transportConfig: &transport.Config{},
kubeClient: &kubernetes.Clientset{},
},
},
assertErr: require.NoError,
}, {
desc: "kubernetes_service, bad kube creds",
serviceType: KubeService,
kubeconfigPath: kubeconfigbadPath,
impersonationCheck: alwaysSucceeds,
assertErr: func(tt require.TestingT, err error, i ...interface{}) {
findErr := "failed to generate TLS config from kubeConfig. clientConfig"
for sc.Scan() {
if strings.Contains(sc.Text(), findErr) {
return
}
}
t.Fatalf("Failed to find error %q in the logs", findErr)
},
want: map[string]*kubeCreds{},
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
got, err := getKubeCreds(ctx, utils.NewLoggerForTests(), teleClusterName, "", tt.kubeconfigPath, tt.serviceType, tt.impersonationCheck)
got, err := getKubeCreds(ctx, logger, teleClusterName, "", tt.kubeconfigPath, tt.serviceType, tt.impersonationCheck)
tt.assertErr(t, err)
if err != nil {
return
Expand All @@ -266,6 +334,7 @@ current-context: foo
cmp.AllowUnexported(kubeCreds{}),
cmp.Comparer(func(a, b *transport.Config) bool { return (a == nil) == (b == nil) }),
cmp.Comparer(func(a, b *kubernetes.Clientset) bool { return (a == nil) == (b == nil) }),
cmp.Comparer(func(a, b *tls.Config) bool { return (a == nil) == (b == nil) }),
))
})
}
Expand Down

0 comments on commit ae9e230

Please sign in to comment.