Skip to content

Commit

Permalink
Remove needlessly complex key generation scheme (#12113)
Browse files Browse the repository at this point in the history
  • Loading branch information
xacrimon authored Apr 25, 2022
1 parent cc4d4df commit 9911640
Show file tree
Hide file tree
Showing 51 changed files with 190 additions and 278 deletions.
3 changes: 0 additions & 3 deletions api/types/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ const (
// KindUser is a user resource
KindUser = "user"

// KindKeyPair is a public/private key pair
KindKeyPair = "key_pair"

// KindHostCert is a host certificate
KindHostCert = "host_cert"

Expand Down
7 changes: 4 additions & 3 deletions integration/app_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
apievents "github.com/gravitational/teleport/api/types/events"
"github.com/gravitational/teleport/lib"
"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/auth/native"
"github.com/gravitational/teleport/lib/auth/testauthority"
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/defaults"
Expand Down Expand Up @@ -1037,7 +1038,7 @@ func setupWithOptions(t *testing.T, opts appTestOptions) *pack {
p.headerAppURI = headerServer.URL
p.flushAppURI = flushServer.URL

privateKey, publicKey, err := testauthority.New().GenerateKeyPair("")
privateKey, publicKey, err := testauthority.New().GenerateKeyPair()
require.NoError(t, err)

// Create a new Teleport instance with passed in configuration.
Expand Down Expand Up @@ -1325,7 +1326,7 @@ func (p *pack) initCertPool(t *testing.T) {

// makeTLSConfig returns TLS config suitable for making an app access request.
func (p *pack) makeTLSConfig(t *testing.T, publicAddr, clusterName string) *tls.Config {
privateKey, publicKey, err := p.rootCluster.Process.GetAuthServer().GenerateKeyPair("")
privateKey, publicKey, err := native.GenerateKeyPair()
require.NoError(t, err)

ws, err := p.tc.CreateAppSession(context.Background(), types.CreateAppSessionRequest{
Expand Down Expand Up @@ -1359,7 +1360,7 @@ func (p *pack) makeTLSConfig(t *testing.T, publicAddr, clusterName string) *tls.
// makeTLSConfigNoSession returns TLS config for application access without
// creating session to simulate nonexistent session scenario.
func (p *pack) makeTLSConfigNoSession(t *testing.T, publicAddr, clusterName string) *tls.Config {
privateKey, publicKey, err := p.rootCluster.Process.GetAuthServer().GenerateKeyPair("")
privateKey, publicKey, err := native.GenerateKeyPair()
require.NoError(t, err)

certificate, err := p.rootCluster.Process.GetAuthServer().GenerateUserAppTestCert(
Expand Down
2 changes: 1 addition & 1 deletion integration/db_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,7 @@ func setupDatabaseTest(t *testing.T, options ...testOptionFunc) *databasePack {
log := utils.NewLoggerForTests()

// Generate keypair.
privateKey, publicKey, err := testauthority.New().GenerateKeyPair("")
privateKey, publicKey, err := testauthority.New().GenerateKeyPair()
require.NoError(t, err)

p := &databasePack{
Expand Down
8 changes: 4 additions & 4 deletions integration/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ func NewInstance(cfg InstanceConfig) *TeleInstance {
}

// generate instance secrets (keys):
keygen := native.New(context.TODO(), native.PrecomputeKeys(0))
keygen := native.New(context.TODO())
if cfg.Priv == nil || cfg.Pub == nil {
cfg.Priv, cfg.Pub, _ = keygen.GenerateKeyPair("")
cfg.Priv, cfg.Pub, _ = keygen.GenerateKeyPair()
}
rsaKey, err := ssh.ParseRawPrivateKey(cfg.Priv)
fatalIf(err)
Expand Down Expand Up @@ -484,7 +484,7 @@ type UserCredsRequest struct {

// GenerateUserCreds generates key to be used by client
func GenerateUserCreds(req UserCredsRequest) (*UserCreds, error) {
priv, pub, err := testauthority.New().GenerateKeyPair("")
priv, pub, err := testauthority.New().GenerateKeyPair()
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -1728,7 +1728,7 @@ func genUserKey() (*client.Key, error) {
}

keygen := testauthority.New()
priv, pub, err := keygen.GenerateKeyPair("")
priv, pub, err := keygen.GenerateKeyPair()
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
6 changes: 3 additions & 3 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func newSuite(t *testing.T) *integrationTestSuite {
suite := &integrationTestSuite{}

var err error
suite.priv, suite.pub, err = testauthority.New().GenerateKeyPair("")
suite.priv, suite.pub, err = testauthority.New().GenerateKeyPair()
require.NoError(t, err)

// Find AllocatePortsNum free listening ports to use.
Expand Down Expand Up @@ -5808,7 +5808,7 @@ func dumpGoroutineProfile() {

// TestWebProxyInsecure makes sure that proxy endpoint works when TLS is disabled.
func TestWebProxyInsecure(t *testing.T) {
privateKey, publicKey, err := testauthority.New().GenerateKeyPair("")
privateKey, publicKey, err := testauthority.New().GenerateKeyPair()
require.NoError(t, err)

rc := NewInstance(InstanceConfig{
Expand Down Expand Up @@ -5851,7 +5851,7 @@ func TestWebProxyInsecure(t *testing.T) {
func TestTraitsPropagation(t *testing.T) {
log := utils.NewLoggerForTests()

privateKey, publicKey, err := testauthority.New().GenerateKeyPair("")
privateKey, publicKey, err := testauthority.New().GenerateKeyPair()
require.NoError(t, err)

// Create root cluster.
Expand Down
5 changes: 3 additions & 2 deletions integration/kube_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/gravitational/teleport/api/profile"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib"
"github.com/gravitational/teleport/lib/auth/native"
"github.com/gravitational/teleport/lib/auth/testauthority"
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/events"
Expand Down Expand Up @@ -97,7 +98,7 @@ func newKubeSuite(t *testing.T) *KubeSuite {
var err error
SetTestTimeouts(time.Millisecond * time.Duration(100))

suite.priv, suite.pub, err = testauthority.New().GenerateKeyPair("")
suite.priv, suite.pub, err = testauthority.New().GenerateKeyPair()
require.NoError(t, err)

suite.me, err = user.Current()
Expand Down Expand Up @@ -1294,7 +1295,7 @@ func kubeProxyClient(cfg kubeProxyConfig) (*kubernetes.Clientset, *rest.Config,
if err != nil {
return nil, nil, trace.Wrap(err)
}
privPEM, _, err := authServer.GenerateKeyPair("")
privPEM, _, err := native.GenerateKeyPair()
if err != nil {
return nil, nil, trace.Wrap(err)
}
Expand Down
2 changes: 1 addition & 1 deletion integration/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ func TestALPNProxyDialProxySSHWithoutInsecureMode(t *testing.T) {
lib.SetInsecureDevMode(true)
defer lib.SetInsecureDevMode(false)

privateKey, publicKey, err := testauthority.New().GenerateKeyPair("")
privateKey, publicKey, err := testauthority.New().GenerateKeyPair()
require.NoError(t, err)

rc := NewInstance(InstanceConfig{
Expand Down
5 changes: 3 additions & 2 deletions integration/utmp_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
apidefaults "github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/auth/native"
"github.com/gravitational/teleport/lib/bpf"
"github.com/gravitational/teleport/lib/pam"
restricted "github.com/gravitational/teleport/lib/restrictedsession"
Expand Down Expand Up @@ -206,7 +207,7 @@ func newSrvCtx(ctx context.Context, t *testing.T) *SrvCtx {
require.NoError(t, err)

// set up host private key and certificate
priv, pub, err := s.server.Auth().GenerateKeyPair("")
priv, pub, err := native.GenerateKeyPair()
require.NoError(t, err)

tlsPub, err := auth.PrivateKeyToPublicKeyTLS(priv)
Expand Down Expand Up @@ -293,7 +294,7 @@ func newSrvCtx(ctx context.Context, t *testing.T) *SrvCtx {

func newUpack(ctx context.Context, s *SrvCtx, username string, allowedLogins []string, allowedLabels types.Labels) (*upack, error) {
auth := s.server.Auth()
upriv, upub, err := auth.GenerateKeyPair("")
upriv, upub, err := native.GenerateKeyPair()
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
25 changes: 0 additions & 25 deletions lib/auth/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,6 @@ func NewAPIServer(config *APIConfig) (http.Handler, error) {
srv.GET("/:version/users/:user", srv.withAuth(srv.getUser))
srv.DELETE("/:version/users/:user", srv.withAuth(srv.deleteUser)) // DELETE IN: 5.2 REST method is replaced by grpc method with context.

// Generating keypairs
srv.POST("/:version/keypair", srv.withAuth(srv.generateKeyPair))

// Passwords and sessions
srv.POST("/:version/users", srv.withAuth(srv.upsertUser))
srv.PUT("/:version/users/:user/web/password", srv.withAuth(srv.changePassword))
Expand Down Expand Up @@ -894,28 +891,6 @@ func (s *APIServer) deleteUser(auth ClientI, w http.ResponseWriter, r *http.Requ
return message(fmt.Sprintf("user %q deleted", user)), nil
}

type generateKeyPairReq struct {
Password string `json:"password"`
}

type generateKeyPairResponse struct {
PrivKey []byte `json:"privkey"`
PubKey string `json:"pubkey"`
}

func (s *APIServer) generateKeyPair(auth ClientI, w http.ResponseWriter, r *http.Request, _ httprouter.Params, version string) (interface{}, error) {
var req *generateKeyPairReq
if err := httplib.ReadJSON(r, &req); err != nil {
return nil, trace.Wrap(err)
}

priv, pub, err := auth.GenerateKeyPair(req.Password)
if err != nil {
return nil, trace.Wrap(err)
}
return &generateKeyPairResponse{PrivKey: priv, PubKey: string(pub)}, nil
}

type generateHostCertReq struct {
Key []byte `json:"key"`
HostID string `json:"hostname"`
Expand Down
5 changes: 3 additions & 2 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import (
"github.com/gravitational/teleport/api/types/wrappers"
apiutils "github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/lib/auth/keystore"
"github.com/gravitational/teleport/lib/auth/native"
wanlib "github.com/gravitational/teleport/lib/auth/webauthn"
"github.com/gravitational/teleport/lib/backend"
"github.com/gravitational/teleport/lib/defaults"
Expand Down Expand Up @@ -150,7 +151,7 @@ func NewServer(cfg *InitConfig, opts ...ServerOption) (*Server, error) {
}
}
if cfg.KeyStoreConfig.RSAKeyPairSource == nil {
cfg.KeyStoreConfig.RSAKeyPairSource = cfg.Authority.GenerateKeyPair
cfg.KeyStoreConfig.RSAKeyPairSource = native.GenerateKeyPair
}
if cfg.KeyStoreConfig.HostUUID == "" {
cfg.KeyStoreConfig.HostUUID = cfg.HostUUID
Expand Down Expand Up @@ -2294,7 +2295,7 @@ func (a *Server) NewWebSession(req types.NewWebSessionRequest) (types.WebSession
return nil, trace.Wrap(err)
}

priv, pub, err := a.GetNewKeyPairFromPool()
priv, pub, err := native.GenerateKeyPair()
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
7 changes: 0 additions & 7 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -1751,13 +1751,6 @@ func (a *ServerWithRoles) DeleteUser(ctx context.Context, user string) error {
return a.authServer.DeleteUser(ctx, user)
}

func (a *ServerWithRoles) GenerateKeyPair(pass string) ([]byte, []byte, error) {
if err := a.action(apidefaults.Namespace, types.KindKeyPair, types.VerbCreate); err != nil {
return nil, nil, trace.Wrap(err)
}
return a.authServer.GenerateKeyPair(pass)
}

func (a *ServerWithRoles) GenerateHostCert(
key []byte, hostID, nodeName string, principals []string, clusterName string, role types.SystemRole, ttl time.Duration,
) ([]byte, error) {
Expand Down
14 changes: 7 additions & 7 deletions lib/auth/auth_with_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestLocalUserCanReissueCerts(t *testing.T) {
t.Parallel()
srv := newTestTLSServer(t)

_, pub, err := srv.Auth().GenerateKeyPair("")
_, pub, err := native.GenerateKeyPair()
require.NoError(t, err)

start := srv.AuthServer.Clock().Now()
Expand Down Expand Up @@ -158,7 +158,7 @@ func TestBotCertificateGenerationCheck(t *testing.T) {
})
require.NoError(t, err)

privateKey, publicKey, err := native.GenerateKeyPair("")
privateKey, publicKey, err := native.GenerateKeyPair()
require.NoError(t, err)
sshPrivateKey, err := ssh.ParseRawPrivateKey(privateKey)
require.NoError(t, err)
Expand Down Expand Up @@ -211,7 +211,7 @@ func TestBotCertificateGenerationStolen(t *testing.T) {
})
require.NoError(t, err)

privateKey, publicKey, err := native.GenerateKeyPair("")
privateKey, publicKey, err := native.GenerateKeyPair()
require.NoError(t, err)
sshPrivateKey, err := ssh.ParseRawPrivateKey(privateKey)
require.NoError(t, err)
Expand Down Expand Up @@ -274,7 +274,7 @@ func TestSSOUserCanReissueCert(t *testing.T) {
client, err := srv.NewClient(TestUser(user.GetName()))
require.NoError(t, err)

_, pub, err := srv.Auth().GenerateKeyPair("")
_, pub, err := native.GenerateKeyPair()
require.NoError(t, err)

_, err = client.GenerateUserCerts(ctx, proto.UserCertsRequest{
Expand Down Expand Up @@ -431,7 +431,7 @@ func TestGenerateUserCertsWithRoleRequest(t *testing.T) {
client, err := srv.NewClient(TestUser(user.GetName()))
require.NoError(t, err)

_, pub, err := srv.Auth().GenerateKeyPair("")
_, pub, err := native.GenerateKeyPair()
require.NoError(t, err)

certs, err := client.GenerateUserCerts(ctx, proto.UserCertsRequest{
Expand Down Expand Up @@ -527,7 +527,7 @@ func TestRoleRequestDenyReimpersonation(t *testing.T) {
// Generate cert with a role request.
client, err := srv.NewClient(TestUser(user.GetName()))
require.NoError(t, err)
priv, pub, err := srv.Auth().GenerateKeyPair("")
priv, pub, err := native.GenerateKeyPair()
require.NoError(t, err)

// Request certs for only the `foo` role.
Expand Down Expand Up @@ -625,7 +625,7 @@ func TestGenerateDatabaseCert(t *testing.T) {
}

// Generate CSR once for speed sake.
priv, _, err := srv.Auth().GenerateKeyPair("")
priv, _, err := native.GenerateKeyPair()
require.NoError(t, err)
csr, err := tlsca.GenerateCertificateRequestPEM(pkix.Name{CommonName: "test"}, priv)
require.NoError(t, err)
Expand Down
20 changes: 0 additions & 20 deletions lib/auth/clt.go
Original file line number Diff line number Diff line change
Expand Up @@ -1037,21 +1037,6 @@ func (c *Client) DeleteWebSession(user string, sid string) error {
return trace.Wrap(err)
}

// GenerateKeyPair generates SSH private/public key pair optionally protected
// by password. If the pass parameter is an empty string, the key pair
// is not password-protected.
func (c *Client) GenerateKeyPair(pass string) ([]byte, []byte, error) {
out, err := c.PostJSON(context.TODO(), c.Endpoint("keypair"), generateKeyPairReq{Password: pass})
if err != nil {
return nil, nil, trace.Wrap(err)
}
var kp *generateKeyPairResponse
if err := json.Unmarshal(out.Bytes(), &kp); err != nil {
return nil, nil, err
}
return kp.PrivKey, []byte(kp.PubKey), err
}

// GenerateHostCert takes the public key in the Open SSH ``authorized_keys``
// plain text format, signs it using Host Certificate Authority private key and returns the
// resulting certificate.
Expand Down Expand Up @@ -1806,11 +1791,6 @@ type IdentityService interface {
// If TTL is not supplied, token will be valid until removed.
GenerateToken(ctx context.Context, req GenerateTokenRequest) (string, error)

// GenerateKeyPair generates SSH private/public key pair optionally protected
// by password. If the pass parameter is an empty string, the key pair
// is not password-protected.
GenerateKeyPair(pass string) ([]byte, []byte, error)

// GenerateHostCert takes the public key in the Open SSH ``authorized_keys``
// plain text format, signs it using Host Certificate Authority private key and returns the
// resulting certificate.
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/clt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func TestClient_RequestTimeout(t *testing.T) {

func newCertAuthority(t *testing.T, name string, caType types.CertAuthType) types.CertAuthority {
ta := testauthority.New()
priv, pub, err := ta.GenerateKeyPair("")
priv, pub, err := ta.GenerateKeyPair()
require.NoError(t, err)

// CA for cluster1 with 1 key pair.
Expand Down
5 changes: 3 additions & 2 deletions lib/auth/grpcserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
apiutils "github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/api/utils/sshutils"
"github.com/gravitational/teleport/lib/auth/mocku2f"
"github.com/gravitational/teleport/lib/auth/native"
wanlib "github.com/gravitational/teleport/lib/auth/webauthn"
"github.com/gravitational/teleport/lib/backend"
"github.com/gravitational/teleport/lib/defaults"
Expand Down Expand Up @@ -807,7 +808,7 @@ func TestGenerateUserSingleUseCert(t *testing.T) {
}
}

_, pub, err := srv.Auth().GenerateKeyPair("")
_, pub, err := native.GenerateKeyPair()
require.NoError(t, err)

tests := []struct {
Expand Down Expand Up @@ -1341,7 +1342,7 @@ func TestGenerateHostCerts(t *testing.T) {
clt, err := srv.NewClient(TestAdmin())
require.NoError(t, err)

priv, pub, err := clt.GenerateKeyPair("")
priv, pub, err := native.GenerateKeyPair()
require.NoError(t, err)

pubTLS, err := PrivateKeyToPublicKeyTLS(priv)
Expand Down
Loading

0 comments on commit 9911640

Please sign in to comment.