Always use PEM PKIX DER representation of public keys for BotInstances (

#43845)

* Always use PEM PKIX DER representation of public keys for BotInstances

* Prefer using `keys.MarshalPublicKey`

api/proto/teleport/machineid/v1/bot_instance.proto

@@ -101,7 +101,9 @@ message BotInstanceStatusAuthentication {
   // the counter in the certificate does not match the counter of the last
   // authentication.
   int32 generation = 5;
-  // The public key of the Bot instance.
+  // The public key of the Bot instance. This must be a PEM wrapped, PKIX DER
+  // encoded public key. This provides consistency and supports multiple types
+  // of public key algorithm.
   bytes public_key = 6;
 
   reserved 7;

lib/auth/bot.go

@@ -35,12 +35,14 @@ import (
 	"github.com/gravitational/teleport/api/types"
 	apievents "github.com/gravitational/teleport/api/types/events"
 	apiutils "github.com/gravitational/teleport/api/utils"
+	"github.com/gravitational/teleport/api/utils/keys"
 	"github.com/gravitational/teleport/lib/auth/machineid/machineidv1"
 	experiment "github.com/gravitational/teleport/lib/auth/machineid/machineidv1/bot_instance_experiment"
 	"github.com/gravitational/teleport/lib/authz"
 	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/events"
 	"github.com/gravitational/teleport/lib/services"
+	"github.com/gravitational/teleport/lib/sshutils"
 )
 
 // validateGenerationLabel validates and updates a generation label.
@@ -174,6 +176,14 @@ func (a *Server) validateGenerationLabel(ctx context.Context, username string, c
 	return nil
 }
 
+func sshPublicKeyToPKIXPEM(pubKey []byte) ([]byte, error) {
+	cryptoPubKey, err := sshutils.CryptoPublicKey(pubKey)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	return keys.MarshalPublicKey(cryptoPubKey)
+}
+
 // updateBotInstance updates the bot instance associated with the context
 // identity, if any.
 func (a *ServerWithRoles) updateBotInstance(ctx context.Context, req *certRequest) error {
@@ -189,9 +199,17 @@ func (a *ServerWithRoles) updateBotInstance(ctx context.Context, req *certReques
 		return nil
 	}
 
+	// req.PublicKey is in SSH Authorized Keys format. For consistency, we
+	// only store public keys in PEM wrapped PKIX, DER format within
+	// BotInstances.
+	pkixPEM, err := sshPublicKeyToPKIXPEM(req.publicKey)
+	if err != nil {
+		return trace.Wrap(err, "converting key")
+	}
+
 	authRecord := &machineidv1pb.BotInstanceStatusAuthentication{
 		AuthenticatedAt: timestamppb.New(a.authServer.GetClock().Now()),
-		PublicKey:       req.publicKey,
+		PublicKey:       pkixPEM,
 
 		// TODO: for now, this copy of the certificate generation only is
 		// informational. Future changes will transition to trusting (and
@@ -234,7 +252,7 @@ func (a *ServerWithRoles) updateBotInstance(ctx context.Context, req *certReques
 		return nil
 	}
 
-	_, err := a.authServer.BotInstance.PatchBotInstance(ctx, ident.BotName, ident.BotInstanceID, func(bi *machineidv1pb.BotInstance) (*machineidv1pb.BotInstance, error) {
+	_, err = a.authServer.BotInstance.PatchBotInstance(ctx, ident.BotName, ident.BotInstanceID, func(bi *machineidv1pb.BotInstance) (*machineidv1pb.BotInstance, error) {
 		if bi.Status == nil {
 			bi.Status = &machineidv1pb.BotInstanceStatus{}
 		}

lib/auth/join.go

@@ -367,14 +367,21 @@ func (a *Server) generateCertsBot(
 		},
 	}
 
+	// req.PublicSSHKey is in SSH Authorized Keys format. For consistency, we
+	// only store public keys in PEM wrapped PKIX, DER format within
+	// BotInstances.
+	pkixPEM, err := sshPublicKeyToPKIXPEM(req.PublicSSHKey)
+	if err != nil {
+		return nil, trace.Wrap(err, "converting key")
+	}
 	auth := &machineidv1pb.BotInstanceStatusAuthentication{
 		AuthenticatedAt: timestamppb.New(a.GetClock().Now()),
 		// TODO: GetSafeName may not return an appropriate value for later
 		// comparison / locking purposes, and this also shouldn't contain
 		// secrets. Should we hash it?
 		JoinToken:  provisionToken.GetSafeName(),
 		JoinMethod: string(provisionToken.GetJoinMethod()),
-		PublicKey:  req.PublicTLSKey,
+		PublicKey:  pkixPEM,
 
 		// Note: Generation will be set during `generateInitialBotCerts()` as
 		// needed.