Skip to content

Commit

Permalink
Remove custom signer
Browse files Browse the repository at this point in the history
The x/crypto didn't support RSA-SHA2 algos, so we developed our own algorithm
signer.

This is no longer the case, and after upgrading x/crypto to 20220513 we can
safely remove the custom code we have.
  • Loading branch information
marcoandredinis committed May 17, 2022
1 parent 0df4893 commit 0d129e4
Show file tree
Hide file tree
Showing 5 changed files with 3 additions and 148 deletions.
3 changes: 0 additions & 3 deletions lib/auth/keystore/hsm.go
Expand Up @@ -25,7 +25,6 @@ import (

"github.com/gravitational/teleport/api/constants"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/trace"

"github.com/ThalesIgnite/crypto11"
Expand Down Expand Up @@ -226,7 +225,6 @@ func (c *hsmKeyStore) GetSSHSigner(ca types.CertAuthority) (ssh.Signer, error) {
if err != nil {
return nil, trace.Wrap(err)
}
sshSigner = sshutils.AlgSigner(sshSigner, sshutils.GetSigningAlgName(ca))
return sshSigner, nil
}

Expand Down Expand Up @@ -415,7 +413,6 @@ func (c *hsmKeyStore) GetAdditionalTrustedSSHSigner(ca types.CertAuthority) (ssh
if err != nil {
return nil, trace.Wrap(err)
}
sshSigner = sshutils.AlgSigner(sshSigner, sshutils.GetSigningAlgName(ca))
return sshSigner, nil
}

Expand Down
3 changes: 0 additions & 3 deletions lib/auth/keystore/raw.go
Expand Up @@ -20,7 +20,6 @@ import (
"golang.org/x/crypto/ssh"

"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/teleport/lib/utils"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -107,7 +106,6 @@ func (c *rawKeyStore) GetSSHSigner(ca types.CertAuthority) (ssh.Signer, error) {
if err != nil {
return nil, trace.Wrap(err)
}
signer = sshutils.AlgSigner(signer, sshutils.GetSigningAlgName(ca))
return signer, nil
}
}
Expand All @@ -124,7 +122,6 @@ func (c *rawKeyStore) GetAdditionalTrustedSSHSigner(ca types.CertAuthority) (ssh
if err != nil {
return nil, trace.Wrap(err)
}
signer = sshutils.AlgSigner(signer, sshutils.GetSigningAlgName(ca))
return signer, nil
}
}
Expand Down
8 changes: 2 additions & 6 deletions lib/auth/native/native.go
Expand Up @@ -34,7 +34,6 @@ import (
"github.com/gravitational/teleport/api/types/wrappers"
apiutils "github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/teleport/lib/utils"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -177,8 +176,6 @@ func (k *Keygen) GenerateHostCertWithoutValidation(c services.HostCertParams) ([
return nil, trace.Wrap(err)
}

signer := sshutils.AlgSigner(c.CASigner, c.CASigningAlg)

// Build a valid list of principals from the HostID and NodeName and then
// add in any additional principals passed in.
principals := BuildPrincipals(c.HostID, c.NodeName, c.ClusterName, types.SystemRoles{c.Role})
Expand Down Expand Up @@ -207,7 +204,7 @@ func (k *Keygen) GenerateHostCertWithoutValidation(c services.HostCertParams) ([
cert.Permissions.Extensions[utils.CertExtensionAuthority] = c.ClusterName

// sign host certificate with private signing key of certificate authority
if err := cert.SignCert(rand.Reader, signer); err != nil {
if err := cert.SignCert(rand.Reader, c.CASigner); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -318,8 +315,7 @@ func (k *Keygen) GenerateUserCertWithoutValidation(c services.UserCertParams) ([
}
}

signer := sshutils.AlgSigner(c.CASigner, c.CASigningAlg)
if err := cert.SignCert(rand.Reader, signer); err != nil {
if err := cert.SignCert(rand.Reader, c.CASigner); err != nil {
return nil, trace.Wrap(err)
}
return ssh.MarshalAuthorizedKey(cert), nil
Expand Down
47 changes: 1 addition & 46 deletions lib/sshutils/signer.go
Expand Up @@ -18,7 +18,6 @@ package sshutils

import (
"crypto"
"io"

"github.com/gravitational/teleport/api/utils/sshutils"

Expand All @@ -27,51 +26,6 @@ import (
"github.com/gravitational/trace"
)

// AlgSigner wraps the provided ssh.Signer to ensure signature algorithm
// compatibility with OpenSSH.
//
// Right now it allows forcing SHA-2 signatures with RSA keys, instead of the
// default SHA-1 used by x/crypto/ssh. See
// https://www.openssh.com/txt/release-8.2 for context.
//
// If the provided Signer is not an RSA key or does not implement
// ssh.AlgorithmSigner, it's returned as is.
//
// DELETE IN 5.0: assuming https://github.com/golang/go/issues/37278 is fixed
// by then and we pull in the fix. Also delete all call sites.
func AlgSigner(s ssh.Signer, alg string) ssh.Signer {
if alg == "" {
return s
}
if s.PublicKey().Type() != ssh.KeyAlgoRSA && s.PublicKey().Type() != ssh.CertAlgoRSAv01 {
return s
}
as, ok := s.(ssh.AlgorithmSigner)
if !ok {
return s
}
return fixedAlgorithmSigner{
AlgorithmSigner: as,
alg: alg,
}
}

type fixedAlgorithmSigner struct {
ssh.AlgorithmSigner
alg string
}

func (s fixedAlgorithmSigner) SignWithAlgorithm(rand io.Reader, data []byte, alg string) (*ssh.Signature, error) {
if alg == "" {
alg = s.alg
}
return s.AlgorithmSigner.SignWithAlgorithm(rand, data, alg)
}

func (s fixedAlgorithmSigner) Sign(rand io.Reader, data []byte) (*ssh.Signature, error) {
return s.AlgorithmSigner.SignWithAlgorithm(rand, data, s.alg)
}

// NewSigner returns new ssh Signer from private key + certificate pair. The
// signer can be used to create "auth methods" i.e. login into Teleport SSH
// servers.
Expand All @@ -96,6 +50,7 @@ func CryptoPublicKey(publicKey []byte) (crypto.PublicKey, error) {
if err != nil {
return nil, trace.Wrap(err)
}

cryptoPubKey, ok := pubKey.(ssh.CryptoPublicKey)
if !ok {
return nil, trace.BadParameter("expected ssh.CryptoPublicKey, got %T", pubKey)
Expand Down
90 changes: 0 additions & 90 deletions lib/sshutils/signer_test.go

This file was deleted.

0 comments on commit 0d129e4

Please sign in to comment.