Merge pull request #1549 from vmware-tanzu/jtc/tiny-fixups-to-support-1548

Tiny fixups to support #1548
This commit is contained in:
Joshua Casey 2023-07-19 16:40:59 -05:00 committed by GitHub
commit 6c329ba56f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 287 additions and 77 deletions

View File

@ -1,4 +1,4 @@
// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
// Package certauthority implements a simple x509 certificate authority suitable for use in an aggregated API service.
@ -179,13 +179,13 @@ func (c *CA) IssueServerCert(dnsNames []string, ips []net.IP, ttl time.Duration)
return c.issueCert(x509.ExtKeyUsageServerAuth, pkix.Name{}, dnsNames, ips, ttl)
}
// Similar to IssueClientCert, but returning the new cert as a pair of PEM-formatted byte slices
// IssueClientCertPEM is similar to IssueClientCert, but returns the new cert as a pair of PEM-formatted byte slices
// for the certificate and private key.
func (c *CA) IssueClientCertPEM(username string, groups []string, ttl time.Duration) ([]byte, []byte, error) {
return toPEM(c.IssueClientCert(username, groups, ttl))
}
// Similar to IssueServerCert, but returning the new cert as a pair of PEM-formatted byte slices
// IssueServerCertPEM is similar to IssueServerCert, but returns the new cert as a pair of PEM-formatted byte slices
// for the certificate and private key.
func (c *CA) IssueServerCertPEM(dnsNames []string, ips []net.IP, ttl time.Duration) ([]byte, []byte, error) {
return toPEM(c.IssueServerCert(dnsNames, ips, ttl))
@ -260,7 +260,7 @@ func toPEM(cert *tls.Certificate, err error) ([]byte, []byte, error) {
return certPEM, keyPEM, nil
}
// Encode a tls.Certificate into a private key PEM and a cert chain PEM.
// ToPEM encodes a tls.Certificate into a private key PEM and a cert chain PEM.
func ToPEM(cert *tls.Certificate) ([]byte, []byte, error) {
// Encode the certificate(s) to PEM.
certPEMBlocks := make([][]byte, 0, len(cert.Certificate))

View File

@ -7,10 +7,10 @@ import (
"crypto"
"crypto/tls"
"crypto/x509"
_ "embed"
"fmt"
"io"
"net"
"os"
"strings"
"testing"
"time"
@ -20,60 +20,65 @@ import (
"go.pinniped.dev/internal/testutil"
)
func loadFromFiles(t *testing.T, certPath string, keyPath string) (*CA, error) {
t.Helper()
certPEM, err := os.ReadFile(certPath)
require.NoError(t, err)
keyPEM, err := os.ReadFile(keyPath)
require.NoError(t, err)
ca, err := Load(string(certPEM), string(keyPEM))
return ca, err
}
var (
//go:embed testdata/empty
empty string
//go:embed testdata/invalid
invalid string
//go:embed testdata/multiple.crt
multiple string
//go:embed testdata/test.crt
testCert string
//go:embed testdata/test.key
testKey string
//go:embed testdata/test2.key
testKey2 string
)
func TestLoad(t *testing.T) {
tests := []struct {
name string
certPath string
keyPath string
wantErr string
name string
cert string
key string
wantErr string
test []byte
}{
{
name: "empty key",
certPath: "./testdata/test.crt",
keyPath: "./testdata/empty",
wantErr: "could not load CA: tls: failed to find any PEM data in key input",
name: "empty key",
cert: testCert,
key: empty,
wantErr: "could not load CA: tls: failed to find any PEM data in key input",
},
{
name: "invalid key",
certPath: "./testdata/test.crt",
keyPath: "./testdata/invalid",
wantErr: "could not load CA: tls: failed to find any PEM data in key input",
name: "invalid key",
cert: testCert,
key: invalid,
wantErr: "could not load CA: tls: failed to find any PEM data in key input",
},
{
name: "mismatched cert and key",
certPath: "./testdata/test.crt",
keyPath: "./testdata/test2.key",
wantErr: "could not load CA: tls: private key does not match public key",
name: "mismatched cert and key",
cert: testCert,
key: testKey2,
wantErr: "could not load CA: tls: private key does not match public key",
},
{
name: "multiple certs",
certPath: "./testdata/multiple.crt",
keyPath: "./testdata/test.key",
wantErr: "invalid CA certificate: expected a single certificate, found 2 certificates",
name: "multiple certs",
cert: multiple,
key: testKey,
wantErr: "invalid CA certificate: expected a single certificate, found 2 certificates",
},
{
name: "success",
certPath: "./testdata/test.crt",
keyPath: "./testdata/test.key",
name: "success",
cert: testCert,
key: testKey,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
ca, err := loadFromFiles(t, tt.certPath, tt.keyPath)
t.Parallel()
ca, err := Load(tt.cert, tt.key)
if tt.wantErr != "" {
require.EqualError(t, err, tt.wantErr)
return
@ -226,7 +231,7 @@ func TestIssue(t *testing.T) {
now := time.Date(2020, 7, 10, 12, 41, 12, 1234, time.UTC)
realCA, err := loadFromFiles(t, "./testdata/test.crt", "./testdata/test.key")
realCA, err := Load(testCert, testKey)
require.NoError(t, err)
tests := []struct {

View File

@ -1,4 +1,4 @@
// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
// Package server is the command line entry point for pinniped-concierge.
@ -118,17 +118,17 @@ func (a *App) runServer(ctx context.Context) error {
// This cert provider will provide certs to the API server and will
// be mutated by a controller to keep the certs up to date with what
// is stored in a k8s Secret. Therefore it also effectively acting as
// an in-memory cache of what is stored in the k8s Secret, helping to
// keep incoming requests fast.
// is stored in a k8s Secret. Therefore, it acts as an in-memory cache
// of what is stored in the k8s Secret, helping to keep incoming requests
// fast.
dynamicServingCertProvider := dynamiccert.NewServingCert("concierge-serving-cert")
// This cert provider will be used to provide the Kube signing key to the
// cert issuer used to issue certs to Pinniped clients wishing to login.
// cert issuer used to issue certs to Pinniped clients wishing to log in.
dynamicSigningCertProvider := dynamiccert.NewCA("concierge-kube-signing-cert")
// This cert provider will be used to provide the impersonation proxy signing key to the
// cert issuer used to issue certs to Pinniped clients wishing to login.
// cert issuer used to issue certs to Pinniped clients wishing to log in.
impersonationProxySigningCertProvider := dynamiccert.NewCA("impersonation-proxy-signing-cert")
// Get the "real" name of the login concierge API group (i.e., the API group name with the
@ -256,7 +256,8 @@ func getAggregatedAPIServerConfig(
return apiServerConfig, nil
}
func main() error { // return an error instead of plog.Fatal to allow defer statements to run
// main returns an error instead of calling plog.Fatal to allow defer statements to run.
func main() error {
defer plog.Setup()()
// Dump out the time since compile (mostly useful for benchmarking our local development cycle latency).

View File

@ -1,4 +1,4 @@
// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
// Package concierge contains functionality to load/store Config's from/to
@ -35,7 +35,7 @@ const (
impersonationProxyPortDefault = 8444
)
// FromPath loads an Config from a provided local file path, inserts any
// FromPath loads a Config from a provided local file path, inserts any
// defaults (from the Config documentation), and verifies that the config is
// valid (per the Config documentation).
//

View File

@ -1,11 +1,11 @@
// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package concierge
import "go.pinniped.dev/internal/plog"
// Config contains knobs to setup an instance of the Pinniped Concierge.
// Config contains knobs to set up an instance of the Pinniped Concierge.
type Config struct {
DiscoveryInfo DiscoveryInfoSpec `json:"discovery"`
APIConfig APIConfigSpec `json:"api"`
@ -21,7 +21,7 @@ type Config struct {
}
// DiscoveryInfoSpec contains configuration knobs specific to
// pinniped's publishing of discovery information. These values can be
// Pinniped's publishing of discovery information. These values can be
// viewed as overrides, i.e., if these are set, then Pinniped will
// publish these values in its discovery document instead of the ones it finds.
type DiscoveryInfoSpec struct {

View File

@ -1,4 +1,4 @@
// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package impersonatorconfig
@ -285,12 +285,10 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre
return nil, err
}
var impersonationCA *certauthority.CA
var impersonationCABundle []byte
if c.shouldHaveImpersonator(impersonationSpec) {
if impersonationCA, err = c.ensureCASecretIsCreated(ctx); err != nil {
return nil, err
}
if err = c.ensureTLSSecret(ctx, nameInfo, impersonationCA); err != nil {
impersonationCABundle, err = c.ensureCAAndTLSSecrets(ctx, nameInfo)
if err != nil {
return nil, err
}
} else {
@ -300,7 +298,7 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre
c.clearTLSSecret()
}
credentialIssuerStrategyResult := c.doSyncResult(nameInfo, impersonationSpec, impersonationCA)
credentialIssuerStrategyResult := c.doSyncResult(nameInfo, impersonationSpec, impersonationCABundle)
if c.shouldHaveImpersonator(impersonationSpec) {
if err = c.loadSignerCA(); err != nil {
@ -313,6 +311,25 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre
return credentialIssuerStrategyResult, nil
}
func (c *impersonatorConfigController) ensureCAAndTLSSecrets(ctx context.Context, nameInfo *certNameInfo) ([]byte, error) {
var (
impersonationCA *certauthority.CA
err error
)
if impersonationCA, err = c.ensureCASecretIsCreated(ctx); err != nil {
return nil, err
}
if err = c.ensureTLSSecret(ctx, nameInfo, impersonationCA); err != nil {
return nil, err
}
if impersonationCA != nil {
return impersonationCA.Bundle(), nil
}
return nil, nil
}
func (c *impersonatorConfigController) loadImpersonationProxyConfiguration(credIssuer *v1alpha1.CredentialIssuer) (*v1alpha1.ImpersonationProxySpec, error) {
// Make a copy of the spec since we got this object from informer cache.
spec := credIssuer.Spec.DeepCopy().ImpersonationProxy
@ -707,7 +724,7 @@ func (c *impersonatorConfigController) deleteTLSSecretWhenCertificateDoesNotMatc
}
if !nameInfo.ready {
// We currently have a secret but we are waiting for a load balancer to be assigned an ingress, so
// We currently have a secret, but we are waiting for a load balancer to be assigned an ingress, so
// our current secret must be old/unwanted.
if err = c.ensureTLSSecretIsRemoved(ctx); err != nil {
return false, err
@ -1018,7 +1035,7 @@ func (c *impersonatorConfigController) clearSignerCA() {
c.impersonationSigningCertProvider.UnsetCertKeyContent()
}
func (c *impersonatorConfigController) doSyncResult(nameInfo *certNameInfo, config *v1alpha1.ImpersonationProxySpec, ca *certauthority.CA) *v1alpha1.CredentialIssuerStrategy {
func (c *impersonatorConfigController) doSyncResult(nameInfo *certNameInfo, config *v1alpha1.ImpersonationProxySpec, caBundle []byte) *v1alpha1.CredentialIssuerStrategy {
switch {
case c.disabledExplicitly(config):
return &v1alpha1.CredentialIssuerStrategy{
@ -1055,7 +1072,7 @@ func (c *impersonatorConfigController) doSyncResult(nameInfo *certNameInfo, conf
Type: v1alpha1.ImpersonationProxyFrontendType,
ImpersonationProxyInfo: &v1alpha1.ImpersonationProxyInfo{
Endpoint: "https://" + nameInfo.clientEndpoint,
CertificateAuthorityData: base64.StdEncoding.EncodeToString(ca.Bundle()),
CertificateAuthorityData: base64.StdEncoding.EncodeToString(caBundle),
},
},
}

View File

@ -1,4 +1,4 @@
// Copyright 2020 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package controllerlib
@ -39,8 +39,8 @@ func (c *controllerManager) WithController(controller Controller, workers int) M
return c
}
// Start will run all managed controllers and block until all controllers shutdown.
// When the context passed is cancelled, all controllers are signalled to shutdown.
// Start will run all managed controllers and block until all controllers have shut down.
// When the context passed is cancelled, all controllers are signalled to shut down.
func (c *controllerManager) Start(ctx context.Context) {
var wg sync.WaitGroup
wg.Add(len(c.controllers))

View File

@ -1,4 +1,4 @@
// Copyright 2021 the Pinniped contributors. All Rights Reserved.
// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package issuer
@ -38,15 +38,14 @@ func (c ClientCertIssuers) Name() string {
}
func (c ClientCertIssuers) IssueClientCertPEM(username string, groups []string, ttl time.Duration) ([]byte, []byte, error) {
var errs []error
errs := make([]error, 0, len(c))
for _, issuer := range c {
certPEM, keyPEM, err := issuer.IssueClientCertPEM(username, groups, ttl)
if err != nil {
errs = append(errs, fmt.Errorf("%s failed to issue client cert: %w", issuer.Name(), err))
continue
if err == nil {
return certPEM, keyPEM, nil
}
return certPEM, keyPEM, nil
errs = append(errs, fmt.Errorf("%s failed to issue client cert: %w", issuer.Name(), err))
}
if err := errors.NewAggregate(errs); err != nil {

View File

@ -0,0 +1,169 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package issuer
import (
"errors"
"testing"
"time"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
"go.pinniped.dev/internal/mocks/issuermocks"
)
func TestName(t *testing.T) {
ctrl := gomock.NewController(t)
tests := []struct {
name string
buildIssuerMocks func() ClientCertIssuers
want string
}{
{
name: "empty issuers",
buildIssuerMocks: func() ClientCertIssuers { return ClientCertIssuers{} },
want: "empty-client-cert-issuers",
},
{
name: "foo issuer",
buildIssuerMocks: func() ClientCertIssuers {
fooClientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
fooClientCertIssuer.EXPECT().Name().Return("foo")
return ClientCertIssuers{fooClientCertIssuer}
},
want: "foo",
},
{
name: "foo and bar issuers",
buildIssuerMocks: func() ClientCertIssuers {
fooClientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
fooClientCertIssuer.EXPECT().Name().Return("foo")
barClientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
barClientCertIssuer.EXPECT().Name().Return("bar")
return ClientCertIssuers{fooClientCertIssuer, barClientCertIssuer}
},
want: "foo,bar",
},
}
for _, tTemp := range tests {
testcase := tTemp
t.Run(testcase.name, func(t *testing.T) {
t.Parallel()
name := testcase.buildIssuerMocks().Name()
require.Equal(t, testcase.want, name)
})
}
}
func TestIssueClientCertPEM(t *testing.T) {
ctrl := gomock.NewController(t)
tests := []struct {
name string
buildIssuerMocks func() ClientCertIssuers
wantErrorMessage string
wantCert []byte
wantKey []byte
}{
{
name: "empty issuers",
buildIssuerMocks: func() ClientCertIssuers { return ClientCertIssuers{} },
wantErrorMessage: "failed to issue cert",
},
{
name: "issuers with error",
buildIssuerMocks: func() ClientCertIssuers {
errClientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
errClientCertIssuer.EXPECT().Name().Return("error cert issuer")
errClientCertIssuer.EXPECT().
IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second).
Return(nil, nil, errors.New("error from wrapped cert issuer"))
return ClientCertIssuers{errClientCertIssuer}
},
wantErrorMessage: "error cert issuer failed to issue client cert: error from wrapped cert issuer",
},
{
name: "valid issuer",
buildIssuerMocks: func() ClientCertIssuers {
validClientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
validClientCertIssuer.EXPECT().
IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second).
Return([]byte("cert"), []byte("key"), nil)
return ClientCertIssuers{validClientCertIssuer}
},
wantCert: []byte("cert"),
wantKey: []byte("key"),
},
{
name: "fallthrough issuer",
buildIssuerMocks: func() ClientCertIssuers {
errClientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
errClientCertIssuer.EXPECT().Name().Return("error cert issuer")
errClientCertIssuer.EXPECT().
IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second).
Return(nil, nil, errors.New("error from wrapped cert issuer"))
validClientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
validClientCertIssuer.EXPECT().
IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second).
Return([]byte("cert"), []byte("key"), nil)
return ClientCertIssuers{
errClientCertIssuer,
validClientCertIssuer,
}
},
wantCert: []byte("cert"),
wantKey: []byte("key"),
},
{
name: "multiple error issuers",
buildIssuerMocks: func() ClientCertIssuers {
err1ClientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
err1ClientCertIssuer.EXPECT().Name().Return("error1 cert issuer")
err1ClientCertIssuer.EXPECT().
IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second).
Return(nil, nil, errors.New("error1 from wrapped cert issuer"))
err2ClientCertIssuer := issuermocks.NewMockClientCertIssuer(ctrl)
err2ClientCertIssuer.EXPECT().Name().Return("error2 cert issuer")
err2ClientCertIssuer.EXPECT().
IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second).
Return(nil, nil, errors.New("error2 from wrapped cert issuer"))
return ClientCertIssuers{
err1ClientCertIssuer,
err2ClientCertIssuer,
}
},
wantErrorMessage: "[error1 cert issuer failed to issue client cert: error1 from wrapped cert issuer, error2 cert issuer failed to issue client cert: error2 from wrapped cert issuer]",
},
}
for _, tTemp := range tests {
testcase := tTemp
t.Run(testcase.name, func(t *testing.T) {
t.Parallel()
certPEM, keyPEM, err := testcase.buildIssuerMocks().
IssueClientCertPEM("username", []string{"group1", "group2"}, 32*time.Second)
if testcase.wantErrorMessage != "" {
require.ErrorContains(t, err, testcase.wantErrorMessage)
require.Empty(t, certPEM)
require.Empty(t, keyPEM)
} else {
require.NoError(t, err)
require.Equal(t, testcase.wantCert, certPEM)
require.Equal(t, testcase.wantKey, keyPEM)
}
})
}
}

View File

@ -1,4 +1,4 @@
// Copyright 2020 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package provider
@ -11,7 +11,7 @@ import (
"go.pinniped.dev/internal/constable"
)
// FederationDomainIssuer represents all of the settings and state for a downstream OIDC provider
// FederationDomainIssuer represents all the settings and state for a downstream OIDC provider
// as defined by a FederationDomain.
type FederationDomainIssuer struct {
issuer string
@ -19,6 +19,8 @@ type FederationDomainIssuer struct {
issuerPath string
}
// NewFederationDomainIssuer returns a FederationDomainIssuer.
// Performs validation, and returns any error from validation.
func NewFederationDomainIssuer(issuer string) (*FederationDomainIssuer, error) {
p := FederationDomainIssuer{issuer: issuer}
err := p.validate()
@ -42,6 +44,10 @@ func (p *FederationDomainIssuer) validate() error {
return constable.Error(`issuer must have "https" scheme`)
}
if issuerURL.Hostname() == "" {
return constable.Error(`issuer must have a hostname`)
}
if issuerURL.User != nil {
return constable.Error(`issuer must not have username or password`)
}
@ -64,14 +70,17 @@ func (p *FederationDomainIssuer) validate() error {
return nil
}
// Issuer returns the issuer.
func (p *FederationDomainIssuer) Issuer() string {
return p.issuer
}
// IssuerHost returns the issuerHost.
func (p *FederationDomainIssuer) IssuerHost() string {
return p.issuerHost
}
// IssuerPath returns the issuerPath.
func (p *FederationDomainIssuer) IssuerPath() string {
return p.issuerPath
}

View File

@ -1,4 +1,4 @@
// Copyright 2020 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package provider
@ -20,6 +20,16 @@ func TestFederationDomainIssuerValidations(t *testing.T) {
issuer: "",
wantError: "federation domain must have an issuer",
},
{
name: "returns url.Parse errors",
issuer: "https://example.com" + string(byte(0x7f)),
wantError: "could not parse issuer as URL: parse \"https://example.com\\x7f\": net/url: invalid control character in URL",
},
{
name: "no hostname",
issuer: "https://",
wantError: `issuer must have a hostname`,
},
{
name: "no scheme",
issuer: "tuna.com",

View File

@ -59,7 +59,7 @@ Pinniped uses [ytt](https://carvel.dev/ytt/) from [Carvel](https://carvel.dev/)
1. Customize configuration parameters:
- See the [default values](http://github.com/vmware-tanzu/pinniped/tree/main/deploy/supervisor/values.yaml) for documentation about individual configuration parameters.
For example, you can change the number of Concierge pods by setting `replicas` or apply custom annotations to the impersonation proxy service using `impersonation_proxy_spec`.
For example, you can change the number of Supervisor pods by setting `replicas` or install into a non-default namespace using `into_namespace`.
- In a different directory, create a new YAML file to contain your site-specific configuration. For example, you might call this file `site/dev-env.yaml`.