Skip to content

Commit

Permalink
Only check certificate algorithms in FIPS mode.
Browse files Browse the repository at this point in the history
Update utils.CertChecker to only check key and certificate algorithms
when in FIPS mode. Otherwise accept keys and certificates generated with
any algorithm.
  • Loading branch information
russjones committed Jul 26, 2019
1 parent 838e754 commit 630d2bf
Show file tree
Hide file tree
Showing 14 changed files with 196 additions and 35 deletions.
7 changes: 7 additions & 0 deletions lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import (
"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/session"
"github.com/gravitational/teleport/lib/shell"
Expand Down Expand Up @@ -2162,3 +2163,9 @@ func ParseDynamicPortForwardSpec(spec []string) (DynamicForwardedPorts, error) {
func InsecureSkipHostKeyChecking(host string, remote net.Addr, key ssh.PublicKey) error {
return nil
}

// isFIPS returns if the binary was build with BoringCrypto, which implies
// FedRAMP/FIPS 140-2 mode for tsh.
func isFIPS() bool {
return modules.GetModules().IsBoringBinary()
}
5 changes: 4 additions & 1 deletion lib/client/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,12 @@ func (k *Key) CheckCert() error {
if err != nil {
return trace.Wrap(err)
}

// A valid principal is always passed in because the principals are not being
// checked here, but rather the validity period, signature, and algorithms.
certChecker := utils.CertChecker{}
certChecker := utils.CertChecker{
FIPS: isFIPS(),
}
err = certChecker.CheckCert(cert.ValidPrincipals[0], cert)
if err != nil {
return trace.Wrap(err)
Expand Down
1 change: 1 addition & 0 deletions lib/client/keyagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ func (a *LocalKeyAgent) CheckHostSignature(addr string, remote net.Addr, key ssh
IsHostAuthority: a.checkHostCertificate,
HostKeyFallback: a.checkHostKey,
},
FIPS: isFIPS(),
}
err := certChecker.CheckHostKey(addr, remote, key)
if err != nil {
Expand Down
36 changes: 29 additions & 7 deletions lib/client/keystore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,11 @@ func (s *KeyStoreTestSuite) makeSignedKey(c *check.C, makeExpired bool) *Key {
c.Assert(err, check.IsNil)

cert, err = s.keygen.GenerateUserCert(services.UserCertParams{
PrivateCASigningKey: CAPriv,
PublicUserKey: pub,
Username: username,
AllowedLogins: allowedLogins,
TTL: ttl,
PrivateCASigningKey: CAPriv,
PublicUserKey: pub,
Username: username,
AllowedLogins: allowedLogins,
TTL: ttl,
PermitAgentForwarding: false,
PermitPortForwarding: true,
})
Expand All @@ -244,8 +244,8 @@ func (s *KeyStoreTestSuite) makeSignedKey(c *check.C, makeExpired bool) *Key {
}
}

// TestCheckKey make sure Teleport clients don't load invalid user
// certificates. The main check is the certificate algorithms.
// TestCheckKey make sure Teleport clients can load non-RSA algorithms in
// normal operating mode.
func (s *KeyStoreTestSuite) TestCheckKey(c *check.C) {
key := s.makeSignedKey(c, false)

Expand All @@ -257,6 +257,28 @@ func (s *KeyStoreTestSuite) TestCheckKey(c *check.C) {
err = s.store.AddKey("host.a", "bob", key)
c.Assert(err, check.IsNil)

_, err = s.store.GetKey("host.a", "bob")
c.Assert(err, check.IsNil)
}

// TestCheckKey make sure Teleport clients don't load invalid
// certificates while in FIPS mode.
func (s *KeyStoreTestSuite) TestCheckKeyFIPS(c *check.C) {
// This test only runs in FIPS mode.
if !isFIPS() {
return
}

key := s.makeSignedKey(c, false)

// Swap out the key with a ECDSA SSH key.
ellipticCertificate, _, err := utils.CreateEllipticCertificate("foo", ssh.UserCert)
c.Assert(err, check.IsNil)
key.Cert = ssh.MarshalAuthorizedKey(ellipticCertificate)

err = s.store.AddKey("host.a", "bob", key)
c.Assert(err, check.IsNil)

_, err = s.store.GetKey("host.a", "bob")
c.Assert(err, check.NotNil)
}
Expand Down
1 change: 1 addition & 0 deletions lib/reversetunnel/remotesite.go
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,7 @@ func (s *remoteSite) dialWithAgent(params DialParams) (net.Conn, error) {
DataDir: s.srv.Config.DataDir,
Address: params.Address,
UseTunnel: targetConn.UseTunnel(),
FIPS: s.srv.FIPS,
}
remoteServer, err := forward.New(serverConfig)
if err != nil {
Expand Down
10 changes: 9 additions & 1 deletion lib/reversetunnel/srv.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,17 @@ type Config struct {

// DataDir is a local server data directory
DataDir string

// PollingPeriod specifies polling period for internal sync
// goroutines, used to speed up sync-ups in tests.
PollingPeriod time.Duration

// Component is a component used in logs
Component string

// FIPS means Teleport was started in a FedRAMP/FIPS 140-2 compliant
// configuration.
FIPS bool
}

// CheckAndSetDefaults checks parameters and sets default values
Expand Down Expand Up @@ -288,6 +293,7 @@ func NewServer(cfg Config) (Server, error) {
sshutils.SetCiphers(cfg.Ciphers),
sshutils.SetKEXAlgorithms(cfg.KEXAlgorithms),
sshutils.SetMACAlgorithms(cfg.MACAlgorithms),
sshutils.SetFIPS(cfg.FIPS),
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -778,7 +784,9 @@ func (s *server) checkHostCert(logger *log.Entry, user string, clusterName strin
return trace.NotFound("cluster %v has no matching CA keys", clusterName)
}

checker := utils.CertChecker{}
checker := utils.CertChecker{
FIPS: s.FIPS,
}
if err := checker.CheckCert(user, cert); err != nil {
return trace.BadParameter(err.Error())
}
Expand Down
3 changes: 3 additions & 0 deletions lib/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -1435,6 +1435,7 @@ func (process *TeleportProcess) initSSH() error {
regular.SetPAMConfig(cfg.SSH.PAM),
regular.SetRotationGetter(process.getRotation),
regular.SetUseTunnel(conn.UseTunnel()),
regular.SetFIPS(cfg.FIPS),
)
if err != nil {
return trace.Wrap(err)
Expand Down Expand Up @@ -1979,6 +1980,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error {
MACAlgorithms: cfg.MACAlgorithms,
DataDir: process.Config.DataDir,
PollingPeriod: process.Config.PollingPeriod,
FIPS: cfg.FIPS,
})
if err != nil {
return trace.Wrap(err)
Expand Down Expand Up @@ -2088,6 +2090,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error {
regular.SetMACAlgorithms(cfg.MACAlgorithms),
regular.SetNamespace(defaults.Namespace),
regular.SetRotationGetter(process.getRotation),
regular.SetFIPS(cfg.FIPS),
)
if err != nil {
return trace.Wrap(err)
Expand Down
6 changes: 6 additions & 0 deletions lib/srv/authhandlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ type AuthHandlers struct {

// AccessPoint is used to access the Auth Server.
AccessPoint auth.AccessPoint

// FIPS mode means Teleport started in a FedRAMP/FIPS 140-2 compliant
// configuration.
FIPS bool
}

// CreateIdentityContext returns an IdentityContext populated with information
Expand Down Expand Up @@ -178,6 +182,7 @@ func (h *AuthHandlers) UserKeyAuth(conn ssh.ConnMetadata, key ssh.PublicKey) (*s
CertChecker: ssh.CertChecker{
IsUserAuthority: h.IsUserAuthority,
},
FIPS: h.FIPS,
}
permissions, err := certChecker.Authenticate(conn, key)
if err != nil {
Expand Down Expand Up @@ -254,6 +259,7 @@ func (h *AuthHandlers) HostKeyAuth(addr string, remote net.Addr, key ssh.PublicK
IsHostAuthority: h.IsHostAuthority,
HostKeyFallback: h.hostKeyCallback,
},
FIPS: h.FIPS,
}
err := certChecker.CheckHostKey(addr, remote, key)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions lib/srv/forward/sshserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ type ServerConfig struct {

// Clock is an optoinal clock to override default real time clock
Clock clockwork.Clock

// FIPS mode means Teleport started in a FedRAMP/FIPS 140-2 compliant
// configuration.
FIPS bool
}

// CheckDefaults makes sure all required parameters are passed in.
Expand Down Expand Up @@ -261,6 +265,7 @@ func New(c ServerConfig) (*Server, error) {
Component: teleport.ComponentForwardingNode,
AuditLog: c.AuthClient,
AccessPoint: c.AuthClient,
FIPS: c.FIPS,
}

// Common term handlers.
Expand Down
16 changes: 15 additions & 1 deletion lib/srv/regular/sshserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ type Server struct {
// useTunnel is used to inform other components that this server is
// requesting connections to it come over a reverse tunnel.
useTunnel bool

// fips means Teleport started in a FedRAMP/FIPS 140-2 compliant
// configuration.
fips bool
}

// GetClock returns server clock implementation
Expand Down Expand Up @@ -408,6 +412,13 @@ func SetUseTunnel(useTunnel bool) ServerOption {
}
}

func SetFIPS(fips bool) ServerOption {
return func(s *Server) error {
s.fips = fips
return nil
}
}

// New returns an unstarted server
func New(addr utils.NetAddr,
hostname string,
Expand Down Expand Up @@ -485,6 +496,7 @@ func New(addr utils.NetAddr,
Component: component,
AuditLog: s.alog,
AccessPoint: s.authService,
FIPS: s.fips,
}

// common term handlers
Expand All @@ -500,7 +512,9 @@ func New(addr utils.NetAddr,
sshutils.SetRequestHandler(s),
sshutils.SetCiphers(s.ciphers),
sshutils.SetKEXAlgorithms(s.kexAlgorithms),
sshutils.SetMACAlgorithms(s.macAlgorithms))
sshutils.SetMACAlgorithms(s.macAlgorithms),
sshutils.SetFIPS(s.fips),
)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
19 changes: 16 additions & 3 deletions lib/sshutils/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ type Server struct {
// insecureSkipHostValidation does not validate the host signers to make sure
// they are a valid certificate. Used in tests.
insecureSkipHostValidation bool

// fips means Teleport started in a FedRAMP/FIPS 140-2 compliant
// configuration.
fips bool
}

const (
Expand Down Expand Up @@ -222,6 +226,13 @@ func SetMACAlgorithms(macAlgorithms []string) ServerOption {
}
}

func SetFIPS(fips bool) ServerOption {
return func(s *Server) error {
s.fips = fips
return nil
}
}

func (s *Server) Addr() string {
return s.listener.Addr().String()
}
Expand Down Expand Up @@ -493,7 +504,7 @@ func (s *Server) checkArguments(a utils.NetAddr, h NewChanHandler, hostSigners [
return trace.BadParameter("host signer can not be nil")
}
if !s.insecureSkipHostValidation {
err := validateHostSigner(signer)
err := validateHostSigner(s.fips, signer)
if err != nil {
return trace.Wrap(err)
}
Expand All @@ -506,7 +517,7 @@ func (s *Server) checkArguments(a utils.NetAddr, h NewChanHandler, hostSigners [
}

// validateHostSigner make sure the signer is a valid certificate.
func validateHostSigner(signer ssh.Signer) error {
func validateHostSigner(fips bool, signer ssh.Signer) error {
cert, ok := signer.PublicKey().(*ssh.Certificate)
if !ok {
return trace.BadParameter("only host certificates supported")
Expand All @@ -515,7 +526,9 @@ func validateHostSigner(signer ssh.Signer) error {
return trace.BadParameter("at least one valid principal is required in host certificate")
}

certChecker := utils.CertChecker{}
certChecker := utils.CertChecker{
FIPS: fips,
}
err := certChecker.CheckCert(cert.ValidPrincipals[0], cert)
if err != nil {
return trace.Wrap(err)
Expand Down
53 changes: 43 additions & 10 deletions lib/sshutils/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,23 +178,56 @@ func (s *ServerSuite) TestConfigureCiphers(c *check.C) {

// TestHostSigner makes sure Teleport can not be started with a invalid host
// certificate. The main check is the certificate algorithms.
func (s *ServerSuite) TestHostSigner(c *check.C) {
func (s *ServerSuite) TestHostSignerFIPS(c *check.C) {
_, ellipticSigner, err := utils.CreateEllipticCertificate("foo", ssh.HostCert)
c.Assert(err, check.IsNil)

newChanHandler := NewChanHandlerFunc(func(_ net.Conn, conn *ssh.ServerConn, nch ssh.NewChannel) {
nch.Reject(ssh.Prohibited, "nothing to see here")
})

_, err = NewServer(
"test",
utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"},
newChanHandler,
[]ssh.Signer{ellipticSigner},
AuthMethods{Password: pass("abc123")},
SetCiphers([]string{"aes128-ctr"}),
)
c.Assert(err, check.NotNil)
var tests = []struct {
inSigner ssh.Signer
inFIPS bool
outError bool
}{
// ECDSA when in FIPS mode should fail.
{
inSigner: ellipticSigner,
inFIPS: true,
outError: true,
},
// RSA when in FIPS mode is okay.
{
inSigner: s.signer,
inFIPS: true,
outError: false,
},
// ECDSA when in not FIPS mode should succeed.
{
inSigner: ellipticSigner,
inFIPS: false,
outError: false,
},
// RSA when in not FIPS mode should succeed.
{
inSigner: s.signer,
inFIPS: false,
outError: false,
},
}
for _, tt := range tests {
_, err := NewServer(
"test",
utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"},
newChanHandler,
[]ssh.Signer{tt.inSigner},
AuthMethods{Password: pass("abc123")},
SetCiphers([]string{"aes128-ctr"}),
SetFIPS(tt.inFIPS),
)
c.Assert(err != nil, check.Equals, tt.outError)
}
}

func wait(c *check.C, srv *Server) {
Expand Down
Loading

0 comments on commit 630d2bf

Please sign in to comment.