Do not delete integration CAs on auth init (#51938)

* Do not delete integration CAs on auth init

* change to an iterator

* fix lint
This commit is contained in:
STeve (Xin) Huang
2025-02-13 21:02:57 +00:00
committed by GitHub
parent 8a5107ceea
commit 84b66bd247
8 changed files with 309 additions and 36 deletions
+53
View File
@@ -0,0 +1,53 @@
/*
* Teleport
* Copyright (C) 2025 Gravitational, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package clientutils
import (
"context"
"github.com/gravitational/trace"
"github.com/gravitational/teleport/api/defaults"
)
// IterateResources is a helper that iterates through each resource from all
// pages and passes them one by one to the provided callback.
func IterateResources[T any](
ctx context.Context,
listPageFunc func(context.Context, int, string) ([]T, string, error),
callback func(T) error,
) error {
var pageToken string
for {
page, nextToken, err := listPageFunc(ctx, defaults.DefaultChunkSize, pageToken)
if err != nil {
return trace.Wrap(err)
}
for _, resource := range page {
if err := callback(resource); err != nil {
return trace.Wrap(err)
}
}
if nextToken == "" {
return nil
}
pageToken = nextToken
}
}
+76
View File
@@ -0,0 +1,76 @@
/*
* Teleport
* Copyright (C) 2025 Gravitational, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package clientutils
import (
"context"
"testing"
"github.com/gravitational/trace"
"github.com/stretchr/testify/require"
"github.com/gravitational/teleport/api/defaults"
)
type mockPaginator struct {
accessDenied bool
}
func (m *mockPaginator) List(_ context.Context, pageSize int, token string) ([]bool, string, error) {
if m.accessDenied {
return nil, "", trace.AccessDenied("access denied")
}
switch token {
case "":
return make([]bool, pageSize), "page1", nil
case "page1":
return make([]bool, pageSize), "page2", nil
case "page2":
return make([]bool, 5), "", nil
default:
return nil, "", trace.BadParameter("invalid token")
}
}
func TestIterateResources(t *testing.T) {
t.Run("success", func(t *testing.T) {
var count int
paginator := mockPaginator{}
err := IterateResources(context.Background(), paginator.List, func(bool) error {
count++
return nil
})
require.NoError(t, err)
require.Equal(t, defaults.DefaultChunkSize*2+5, count)
})
t.Run("paginator error", func(t *testing.T) {
paginator := mockPaginator{accessDenied: true}
err := IterateResources(context.Background(), paginator.List, func(bool) error {
return nil
})
require.Error(t, err)
})
t.Run("callback error", func(t *testing.T) {
paginator := mockPaginator{}
err := IterateResources(context.Background(), paginator.List, func(bool) error {
return trace.BadParameter("error")
})
require.Error(t, err)
})
}
+29 -4
View File
@@ -51,10 +51,12 @@ import (
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/types/clusterconfig"
apievents "github.com/gravitational/teleport/api/types/events"
"github.com/gravitational/teleport/api/utils/clientutils"
"github.com/gravitational/teleport/api/utils/keys"
"github.com/gravitational/teleport/lib"
"github.com/gravitational/teleport/lib/auth/autoupdate/autoupdatev1"
"github.com/gravitational/teleport/lib/auth/dbobjectimportrule/dbobjectimportrulev1"
igcredentials "github.com/gravitational/teleport/lib/auth/integration/credentials"
"github.com/gravitational/teleport/lib/auth/keystore"
"github.com/gravitational/teleport/lib/auth/machineid/machineidv1"
"github.com/gravitational/teleport/lib/auth/migration"
@@ -643,6 +645,24 @@ func initializeAuthorities(ctx context.Context, asrv *Server, cfg *InitConfig) e
return trace.Wrap(err)
}
// Collect CAs from integrations to avoid deleting them.
err := clientutils.IterateResources(ctx, asrv.Services.ListIntegrations, func(ig types.Integration) error {
caKeySet, err := igcredentials.GetIntegrationCertAuthorities(ctx, ig, asrv.Services)
switch {
case trace.IsNotImplemented(err):
case err != nil:
// This should not happen by design. In case integration is in a
// bad state, log a warning instead of failing this initialization.
asrv.logger.WarnContext(ctx, "Failed to fetch integration CAs", "ig", ig.GetName(), "error", err)
default:
allKeysInUse = append(allKeysInUse, collectKeysInUse(*caKeySet)...)
}
return nil
})
if err != nil {
return trace.Wrap(err)
}
// Delete any unused keys from the keyStore. This is to avoid exhausting
// (or wasting) HSM resources.
if err := asrv.keyStore.DeleteUnusedKeys(ctx, allKeysInUse); err != nil {
@@ -654,7 +674,7 @@ func initializeAuthorities(ctx context.Context, asrv *Server, cfg *InitConfig) e
return nil
}
func initializeAuthority(ctx context.Context, asrv *Server, caID types.CertAuthID) (usableKeysResult *keystore.UsableKeysResult, keysInUse [][]byte, err error) {
func initializeAuthority(ctx context.Context, asrv *Server, caID types.CertAuthID) (*keystore.UsableKeysResult, [][]byte, error) {
ca, err := asrv.Services.GetCertAuthority(ctx, caID, true)
if err != nil {
if !trace.IsNotFound(err) {
@@ -674,7 +694,7 @@ func initializeAuthority(ctx context.Context, asrv *Server, caID types.CertAuthI
// Make sure the keystore has usable keys. This is a bit redundant if the CA
// was just generated above, but cheap relative to generating the CA, and
// it's nice to get the usableKeysResult.
usableKeysResult, err = asrv.keyStore.HasUsableActiveKeys(ctx, ca)
usableKeysResult, err := asrv.keyStore.HasUsableActiveKeys(ctx, ca)
if err != nil {
return nil, nil, trace.Wrap(err)
}
@@ -734,7 +754,12 @@ func initializeAuthority(ctx context.Context, asrv *Server, caID types.CertAuthI
)
}
for _, keySet := range []types.CAKeySet{ca.GetActiveKeys(), ca.GetAdditionalTrustedKeys()} {
keysInUse := collectKeysInUse(ca.GetActiveKeys(), ca.GetAdditionalTrustedKeys())
return usableKeysResult, keysInUse, nil
}
func collectKeysInUse(cas ...types.CAKeySet) (keysInUse [][]byte) {
for _, keySet := range cas {
for _, sshKeyPair := range keySet.SSH {
keysInUse = append(keysInUse, sshKeyPair.PrivateKey)
}
@@ -745,7 +770,7 @@ func initializeAuthority(ctx context.Context, asrv *Server, caID types.CertAuthI
keysInUse = append(keysInUse, jwtKeyPair.PrivateKey)
}
}
return usableKeysResult, keysInUse, nil
return keysInUse
}
// generateAuthority creates a new self-signed authority of the provided type
@@ -132,3 +132,38 @@ func GetIntegrationRef(ctx context.Context, integration string, igGetter Integra
}
return ref, nil
}
// GetIntegrationCertAuthorities attempts to retrieve certificate authorities
// for provided integration.
func GetIntegrationCertAuthorities(ctx context.Context, ig types.Integration, getter ByLabelsGetter) (*types.CAKeySet, error) {
switch ig.GetSubKind() {
case types.IntegrationSubKindGitHub:
caKeySet, err := GetGitHubCertAuthorities(ctx, ig, getter)
return caKeySet, trace.Wrap(err)
default:
return nil, trace.NotImplemented("unsupported for integration subkind %v", ig.GetSubKind())
}
}
// GetGitHubCertAuthorities retrieves the SSH keys for a GitHub integration.
func GetGitHubCertAuthorities(ctx context.Context, ig types.Integration, getter ByLabelsGetter) (*types.CAKeySet, error) {
if ig.GetSubKind() != types.IntegrationSubKindGitHub {
return nil, trace.BadParameter("integration is not a GitHub integration")
}
if ig.GetCredentials() == nil {
return nil, trace.BadParameter("missing credentials")
}
creds, err := GetByPurpose(ctx, ig.GetCredentials().GetStaticCredentialsRef(), PurposeGitHubSSHCA, getter)
if err != nil {
return nil, trace.Wrap(err)
}
cas := creds.GetSSHCertAuthorities()
if len(cas) == 0 {
return nil, trace.BadParameter("missing SSH cert authorities from plugin static credentials")
}
return &types.CAKeySet{
SSH: cas,
}, nil
}
@@ -30,6 +30,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/fixtures"
)
type mockByLabelsGetter struct {
@@ -62,6 +63,31 @@ func mustMakeCred(t *testing.T, labels map[string]string) types.PluginStaticCred
return cred
}
func mustMakeGitHubSSHCA(t *testing.T) types.PluginStaticCredentials {
t.Helper()
cred, err := types.NewPluginStaticCredentials(
types.Metadata{
Name: uuid.NewString(),
Labels: map[string]string{
LabelStaticCredentialsPurpose: PurposeGitHubSSHCA,
},
},
types.PluginStaticCredentialsSpecV1{
Credentials: &types.PluginStaticCredentialsSpecV1_SSHCertAuthorities{
SSHCertAuthorities: &types.PluginStaticCredentialsSSHCertAuthorities{
CertAuthorities: []*types.SSHKeyPair{{
PublicKey: []byte(fixtures.SSHCAPublicKey),
PrivateKey: []byte(fixtures.SSHCAPrivateKey),
PrivateKeyType: types.PrivateKeyType_RAW,
}},
},
},
},
)
require.NoError(t, err)
return cred
}
func TestGetByPurpose(t *testing.T) {
ref := NewRef()
purpose := "test-found"
@@ -100,7 +126,7 @@ func TestGetByPurpose(t *testing.T) {
wantError: trace.IsNotFound,
},
{
name: "too mandy creds found",
name: "too many creds found",
ref: ref,
setupMock: func(m *mockByLabelsGetter) {
m.On("GetPluginStaticCredentialsByLabels", labels).
@@ -137,3 +163,80 @@ func TestGetByPurpose(t *testing.T) {
})
}
}
func metadataWithName(name string) types.Metadata {
return types.Metadata{
Name: name,
}
}
func TestGetIntegrationCertAuthorities(t *testing.T) {
notSupportedIntegration, err := types.NewIntegrationAWSOIDC(
metadataWithName("not-supported"),
&types.AWSOIDCIntegrationSpecV1{
RoleARN: "arn:aws:iam::123456789012:role/OpsTeam",
},
)
require.NoError(t, err)
githubSpec := &types.GitHubIntegrationSpecV1{
Organization: "org",
}
githubIntegrationNoCreds, err := types.NewIntegrationGitHub(
metadataWithName("github-no-creds"),
githubSpec,
)
require.NoError(t, err)
githubIntegration, err := types.NewIntegrationGitHub(
metadataWithName("github-success"),
githubSpec,
)
require.NoError(t, err)
githubIntegration.SetCredentials(&types.PluginCredentialsV1{
Credentials: &types.PluginCredentialsV1_StaticCredentialsRef{
StaticCredentialsRef: NewRef(),
},
})
m := &mockByLabelsGetter{}
m.On("GetPluginStaticCredentialsByLabels", mock.Anything).
Return([]types.PluginStaticCredentials{mustMakeGitHubSSHCA(t)}, nil)
tests := []struct {
ig types.Integration
checkError func(error) bool
wantCAKeySet *types.CAKeySet
}{
{
ig: notSupportedIntegration,
checkError: trace.IsNotImplemented,
},
{
ig: githubIntegrationNoCreds,
checkError: trace.IsBadParameter,
},
{
ig: githubIntegration,
wantCAKeySet: &types.CAKeySet{
SSH: []*types.SSHKeyPair{{
PublicKey: []byte(fixtures.SSHCAPublicKey),
PrivateKey: []byte(fixtures.SSHCAPrivateKey),
PrivateKeyType: types.PrivateKeyType_RAW,
}},
},
},
}
for _, test := range tests {
t.Run(test.ig.GetName(), func(t *testing.T) {
actualCAKeySet, err := GetIntegrationCertAuthorities(context.Background(), test.ig, m)
if test.checkError != nil {
require.True(t, test.checkError(err))
} else {
require.NoError(t, err)
}
require.Equal(t, test.wantCAKeySet, actualCAKeySet)
})
}
}
@@ -44,18 +44,16 @@ func (s *Service) ExportIntegrationCertAuthorities(ctx context.Context, in *inte
return nil, trace.Wrap(err)
}
// Currently only public keys are exported.
switch ig.GetSubKind() {
case types.IntegrationSubKindGitHub:
caKeySet, err := s.getGitHubCertAuthorities(ctx, ig)
if err != nil {
return nil, trace.Wrap(err)
}
caKeySetWithoutSecerts := caKeySet.WithoutSecrets()
return &integrationpb.ExportIntegrationCertAuthoritiesResponse{CertAuthorities: &caKeySetWithoutSecerts}, nil
default:
return nil, trace.BadParameter("unsupported for integration subkind %v", ig.GetSubKind())
caKeySet, err := credentials.GetIntegrationCertAuthorities(ctx, ig, s.cache)
if err != nil {
return nil, trace.Wrap(err)
}
// Currently only public keys are exported.
caKeySetWithoutSecrets := caKeySet.WithoutSecrets()
return &integrationpb.ExportIntegrationCertAuthoritiesResponse{
CertAuthorities: &caKeySetWithoutSecrets,
}, nil
}
func buildGitHubOAuthCredentials(idSecret *types.PluginIdSecretCredential) (*types.PluginStaticCredentialsV1, error) {
@@ -246,21 +244,3 @@ func (s *Service) getStaticCredentialsWithPurpose(ctx context.Context, ig types.
return credentials.GetByPurpose(ctx, ig.GetCredentials().GetStaticCredentialsRef(), purpose, s.cache)
}
func (s *Service) getGitHubCertAuthorities(ctx context.Context, ig types.Integration) (*types.CAKeySet, error) {
if ig.GetSubKind() != types.IntegrationSubKindGitHub {
return nil, trace.BadParameter("integration is not a GitHub integration")
}
creds, err := s.getStaticCredentialsWithPurpose(ctx, ig, credentials.PurposeGitHubSSHCA)
if err != nil {
return nil, trace.Wrap(err)
}
cas := creds.GetSSHCertAuthorities()
if len(cas) == 0 {
return nil, trace.BadParameter("missing SSH cert authorities from plugin static credentials")
}
return &types.CAKeySet{
SSH: cas,
}, nil
}
@@ -110,7 +110,7 @@ func TestExportIntegrationCertAuthorities(t *testing.T) {
t.Helper()
require.Nil(t, resp)
require.Error(t, err)
require.True(t, trace.IsBadParameter(err))
require.True(t, trace.IsNotImplemented(err))
},
},
}
+2 -1
View File
@@ -28,6 +28,7 @@ import (
integrationpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/auth/integration/credentials"
"github.com/gravitational/teleport/lib/authz"
)
@@ -91,7 +92,7 @@ func (s *Service) getGitHubSigner(ctx context.Context, integration string) (ssh.
if err != nil {
return nil, trace.Wrap(err)
}
caKeySet, err := s.getGitHubCertAuthorities(ctx, ig)
caKeySet, err := credentials.GetGitHubCertAuthorities(ctx, ig, s.cache)
if err != nil {
return nil, trace.Wrap(err)
}