Skip to content

Commit 0896943

Browse files
author
Andrew Lytvynov
committed
Add ca_signing_algo to the config file
This allows users to override the SHA2 signing algorithms we default to now for compatibility with the (very) old OpenSSH versions. For host and user certs, use the CA signing algo for their own handshakes. This allows us to propagate the signing algo from auth server everywhere else.
1 parent 9d7da7f commit 0896943

File tree

17 files changed

+142
-50
lines changed

17 files changed

+142
-50
lines changed

Diff for: lib/auth/init.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,7 @@ func ReadTLSIdentityFromKeyPair(keyBytes, certBytes []byte, caCertsBytes [][]byt
772772
return identity, nil
773773
}
774774

775-
// ReadSSHIdentityFromKeyPair reads identity from initialized keypair
775+
// ReadSSHIdentityFromKeyPair reads identity from initialized keypair.
776776
func ReadSSHIdentityFromKeyPair(keyBytes, certBytes []byte) (*Identity, error) {
777777
if len(keyBytes) == 0 {
778778
return nil, trace.BadParameter("PrivateKey: missing private key")
@@ -796,7 +796,10 @@ func ReadSSHIdentityFromKeyPair(keyBytes, certBytes []byte) (*Identity, error) {
796796
if err != nil {
797797
return nil, trace.BadParameter("failed to parse private key: %v", err)
798798
}
799-
signer = sshutils.CompatSigner(signer)
799+
// Inherit the cert signature algorithm from CA signature.
800+
// Whatever auth server decided to use for SSH cert signing should be used
801+
// by the resulting certs for signing.
802+
signer = sshutils.AlgSigner(signer, cert.Signature.Format)
800803
// this signer authenticates using certificate signed by the cert authority
801804
// not only by the public key
802805
certSigner, err := ssh.NewCertSigner(cert, signer)

Diff for: lib/auth/native/native.go

+12-2
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ type Keygen struct {
6161
cancel context.CancelFunc
6262
precomputeCount int
6363

64+
caSignatureAlg string
65+
6466
// clock is used to control time.
6567
clock clockwork.Clock
6668
}
@@ -85,6 +87,14 @@ func PrecomputeKeys(count int) KeygenOption {
8587
}
8688
}
8789

90+
// SetCASignatureAlg overrides the signature algorithm for SSH certificates.
91+
func SetCASignatureAlg(alg string) KeygenOption {
92+
return func(k *Keygen) error {
93+
k.caSignatureAlg = alg
94+
return nil
95+
}
96+
}
97+
8898
// New returns a new key generator.
8999
func New(ctx context.Context, opts ...KeygenOption) (*Keygen, error) {
90100
ctx, cancel := context.WithCancel(ctx)
@@ -194,7 +204,7 @@ func (k *Keygen) GenerateHostCert(c services.HostCertParams) ([]byte, error) {
194204
if err != nil {
195205
return nil, trace.Wrap(err)
196206
}
197-
signer = sshutils.CompatSigner(signer)
207+
signer = sshutils.AlgSigner(signer, k.caSignatureAlg)
198208

199209
// Build a valid list of principals from the HostID and NodeName and then
200210
// add in any additional principals passed in.
@@ -306,7 +316,7 @@ func (k *Keygen) GenerateUserCert(c services.UserCertParams) ([]byte, error) {
306316
if err != nil {
307317
return nil, trace.Wrap(err)
308318
}
309-
signer = sshutils.CompatSigner(signer)
319+
signer = sshutils.AlgSigner(signer, k.caSignatureAlg)
310320
if err := cert.SignCert(rand.Reader, signer); err != nil {
311321
return nil, trace.Wrap(err)
312322
}

Diff for: lib/auth/native/native_test.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,12 @@ func (s *NativeSuite) SetUpSuite(c *check.C) {
4747

4848
fakeClock := clockwork.NewFakeClockAt(time.Date(2016, 9, 8, 7, 6, 5, 0, time.UTC))
4949

50-
a, err := New(context.TODO(), PrecomputeKeys(1), SetClock(fakeClock))
50+
a, err := New(
51+
context.TODO(),
52+
PrecomputeKeys(1),
53+
SetClock(fakeClock),
54+
SetCASignatureAlg(ssh.SigAlgoRSASHA2256),
55+
)
5156
c.Assert(err, check.IsNil)
5257

5358
s.suite = &test.AuthSuite{
@@ -230,7 +235,8 @@ func (s *NativeSuite) TestUserCertCompatibility(c *check.C) {
230235

231236
userCertificate, ok := publicKey.(*ssh.Certificate)
232237
c.Assert(ok, check.Equals, true, comment)
233-
238+
// Check that the signature algorithm is correct.
239+
c.Assert(userCertificate.Signature.Format, check.Equals, s.suite.A.(*Keygen).caSignatureAlg)
234240
// check if we added the roles extension
235241
_, ok = userCertificate.Extensions[teleport.CertExtensionTeleportRoles]
236242
c.Assert(ok, check.Equals, tt.outHasRoles, comment)

Diff for: lib/auth/rotate.go

-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"github.com/gravitational/teleport/lib/auth/native"
2525
"github.com/gravitational/teleport/lib/defaults"
2626
"github.com/gravitational/teleport/lib/services"
27-
"github.com/gravitational/teleport/lib/sshutils"
2827
"github.com/gravitational/teleport/lib/tlsca"
2928

3029
"github.com/gravitational/trace"
@@ -479,7 +478,6 @@ func startNewRotation(req rotationReq, ca services.CertAuthority) error {
479478
if err != nil {
480479
return trace.Wrap(err)
481480
}
482-
signer = sshutils.CompatSigner(signer)
483481

484482
sshPubPEM = ssh.MarshalAuthorizedKey(signer.PublicKey())
485483
sshPrivPEM = req.privateKey

Diff for: lib/client/interfaces.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,10 @@ func (k *Key) AsAuthMethod() (ssh.AuthMethod, error) {
246246
if signer, err = ssh.NewCertSigner(keys[0].Certificate, signer); err != nil {
247247
return nil, trace.Wrap(err)
248248
}
249-
signer = sshutils.CompatSigner(signer)
249+
// Inherit the cert signature algorithm from CA signature.
250+
// Whatever auth server decided to use for SSH cert signing should be used
251+
// by the resulting certs for signing.
252+
signer = sshutils.AlgSigner(signer, keys[0].Certificate.Signature.Format)
250253
return NewAuthMethodForCert(signer), nil
251254
}
252255

Diff for: lib/config/configuration.go

+3
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,9 @@ func ApplyFileConfig(fc *FileConfig, cfg *service.Config) error {
287287
if fc.MACAlgorithms != nil {
288288
cfg.MACAlgorithms = fc.MACAlgorithms
289289
}
290+
if fc.CASignatureAlgorithm != "" {
291+
cfg.CASignatureAlgorithm = fc.CASignatureAlgorithm
292+
}
290293

291294
// Read in how nodes will validate the CA.
292295
if fc.CAPin != "" {

Diff for: lib/config/configuration_test.go

+26-12
Original file line numberDiff line numberDiff line change
@@ -329,19 +329,19 @@ func (s *ConfigTestSuite) TestTrustedClusters(c *check.C) {
329329
// TestFileConfigCheck makes sure we don't start with invalid settings.
330330
func (s *ConfigTestSuite) TestFileConfigCheck(c *check.C) {
331331
tests := []struct {
332-
inConfigString string
333-
outError bool
332+
desc string
333+
inConfig string
334+
outError bool
334335
}{
335-
// 0 - all defaults, valid
336336
{
337-
`
337+
desc: "all defaults, valid",
338+
inConfig: `
338339
teleport:
339340
`,
340-
false,
341341
},
342-
// 1 - invalid cipher, not valid
343342
{
344-
`
343+
desc: "invalid cipher, not valid",
344+
inConfig: `
345345
teleport:
346346
ciphers:
347347
- aes256-ctr
@@ -351,15 +351,29 @@ teleport:
351351
mac_algos:
352352
353353
`,
354-
true,
354+
outError: true,
355+
},
356+
{
357+
desc: "change CA signature alg, valid",
358+
inConfig: `
359+
teleport:
360+
ca_signature_algo: ssh-rsa
361+
`,
362+
},
363+
{
364+
desc: "invalid CA signature alg, not valid",
365+
inConfig: `
366+
teleport:
367+
ca_signature_algo: foobar
368+
`,
369+
outError: true,
355370
},
356371
}
357372

358-
// run tests
359-
for i, tt := range tests {
360-
comment := check.Commentf("Test %v", i)
373+
for _, tt := range tests {
374+
comment := check.Commentf(tt.desc)
361375

362-
_, err := ReadConfig(bytes.NewBufferString(tt.inConfigString))
376+
_, err := ReadConfig(bytes.NewBufferString(tt.inConfig))
363377
if tt.outError {
364378
c.Assert(err, check.NotNil, comment)
365379
} else {

Diff for: lib/config/fileconf.go

+18-3
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ var (
138138
"ciphers": false,
139139
"kex_algos": false,
140140
"mac_algos": false,
141+
"ca_signature_algo": false,
141142
"connector_name": false,
142143
"session_recording": false,
143144
"read_capacity_units": false,
@@ -164,6 +165,12 @@ var (
164165
}
165166
)
166167

168+
var validCASigAlgos = []string{
169+
ssh.SigAlgoRSA,
170+
ssh.SigAlgoRSASHA2256,
171+
ssh.SigAlgoRSASHA2512,
172+
}
173+
167174
// FileConfig structre represents the teleport configuration stored in a config file
168175
// in YAML format (usually /etc/teleport.yaml)
169176
//
@@ -329,19 +336,22 @@ func (conf *FileConfig) Check() error {
329336

330337
for _, c := range conf.Ciphers {
331338
if !utils.SliceContainsStr(sc.Ciphers, c) {
332-
return trace.BadParameter("cipher %q not supported", c)
339+
return trace.BadParameter("cipher algorithm %q is not supported; supported algorithms: %q", c, sc.Ciphers)
333340
}
334341
}
335342
for _, k := range conf.KEXAlgorithms {
336343
if !utils.SliceContainsStr(sc.KeyExchanges, k) {
337-
return trace.BadParameter("KEX %q not supported", k)
344+
return trace.BadParameter("KEX algorithm %q is not supported; supported algorithms: %q", k, sc.KeyExchanges)
338345
}
339346
}
340347
for _, m := range conf.MACAlgorithms {
341348
if !utils.SliceContainsStr(sc.MACs, m) {
342-
return trace.BadParameter("MAC %q not supported", m)
349+
return trace.BadParameter("MAC algorithm %q is not supported; supported algorithms: %q", m, sc.MACs)
343350
}
344351
}
352+
if conf.CASignatureAlgorithm != "" && !utils.SliceContainsStr(validCASigAlgos, conf.CASignatureAlgorithm) {
353+
return trace.BadParameter("CA signature algorithm %q is not supported; supported algorithms: %q", conf.CASignatureAlgorithm, validCASigAlgos)
354+
}
345355

346356
return nil
347357
}
@@ -399,6 +409,11 @@ type Global struct {
399409
// the server supports. If omitted the defaults will be used.
400410
MACAlgorithms []string `yaml:"mac_algos,omitempty"`
401411

412+
// CASignatureAlgorithm is an SSH Certificate Authority (CA) signature
413+
// algorithm that the server uses for signing user and host certificates.
414+
// If omitted, the default will be used.
415+
CASignatureAlgorithm string `yaml:"ca_signature_algo,omitempty"`
416+
402417
// CAPin is the SKPI hash of the CA used to verify the Auth Server.
403418
CAPin string `yaml:"ca_pin"`
404419
}

Diff for: lib/reversetunnel/cache.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,6 @@ func (c *certificateCache) generateHostCert(principals []string) (ssh.Signer, er
155155
if err != nil {
156156
return nil, trace.Wrap(err)
157157
}
158-
privateKey = sshutils.CompatSigner(privateKey)
159158
publicKey, _, _, _, err := ssh.ParseAuthorizedKey(certBytes)
160159
if err != nil {
161160
return nil, err
@@ -164,6 +163,10 @@ func (c *certificateCache) generateHostCert(principals []string) (ssh.Signer, er
164163
if !ok {
165164
return nil, trace.BadParameter("not a certificate")
166165
}
166+
// Inherit the cert signature algorithm from CA signature.
167+
// Whatever auth server decided to use for SSH cert signing should be used
168+
// by the resulting certs for signing.
169+
privateKey = sshutils.AlgSigner(privateKey, cert.Signature.Format)
167170

168171
// return a ssh.Signer
169172
s, err := ssh.NewCertSigner(cert, privateKey)

Diff for: lib/service/cfg.go

+6
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@ type Config struct {
141141
// the server supports. If omitted the defaults will be used.
142142
MACAlgorithms []string
143143

144+
// CASignatureAlgorithm is an SSH Certificate Authority (CA) signature
145+
// algorithm that the server uses for signing user and host certificates.
146+
// If omitted, the default will be used.
147+
CASignatureAlgorithm string
148+
144149
// DiagnosticAddr is an address for diagnostic and healthz endpoint service
145150
DiagnosticAddr utils.NetAddr
146151

@@ -479,6 +484,7 @@ func ApplyDefaults(cfg *Config) {
479484
cfg.Ciphers = sc.Ciphers
480485
cfg.KEXAlgorithms = kex
481486
cfg.MACAlgorithms = macs
487+
cfg.CASignatureAlgorithm = ssh.SigAlgoRSASHA2512
482488

483489
// Auth service defaults.
484490
cfg.Auth.Enabled = true

Diff for: lib/service/cfg_test.go

+24
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/gravitational/teleport/lib/backend/lite"
2424
"github.com/gravitational/teleport/lib/defaults"
2525
"github.com/gravitational/teleport/lib/utils"
26+
"golang.org/x/crypto/ssh"
2627

2728
"gopkg.in/check.v1"
2829
)
@@ -56,6 +57,29 @@ func (s *ConfigSuite) TestDefaultConfig(c *check.C) {
5657
c.Error("default hostname wasn't properly set")
5758
}
5859

60+
// crypto settings
61+
c.Assert(config.CipherSuites, check.DeepEquals, utils.DefaultCipherSuites())
62+
// Unfortunately the below algos don't have exported constants in
63+
// golang.org/x/crypto/ssh for us to use.
64+
c.Assert(config.Ciphers, check.DeepEquals, []string{
65+
66+
67+
"aes128-ctr",
68+
"aes192-ctr",
69+
"aes256-ctr",
70+
})
71+
c.Assert(config.KEXAlgorithms, check.DeepEquals, []string{
72+
73+
"ecdh-sha2-nistp256",
74+
"ecdh-sha2-nistp384",
75+
"ecdh-sha2-nistp521",
76+
})
77+
c.Assert(config.MACAlgorithms, check.DeepEquals, []string{
78+
79+
"hmac-sha2-256",
80+
})
81+
c.Assert(config.CASignatureAlgorithm, check.Equals, ssh.SigAlgoRSASHA2512)
82+
5983
// auth section
6084
auth := config.Auth
6185
c.Assert(auth.SSHAddr, check.DeepEquals, localAuthAddr)

Diff for: lib/service/service.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,10 @@ func NewTeleport(cfg *Config) (*TeleportProcess, error) {
625625
precomputeCount = 0
626626
}
627627
var err error
628-
cfg.Keygen, err = native.New(process.ExitContext(), native.PrecomputeKeys(precomputeCount))
628+
cfg.Keygen, err = native.New(process.ExitContext(),
629+
native.PrecomputeKeys(precomputeCount),
630+
native.SetCASignatureAlg(cfg.CASignatureAlgorithm),
631+
)
629632
if err != nil {
630633
return nil, trace.Wrap(err)
631634
}

Diff for: lib/services/authority.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ type CertAuthority interface {
202202
// Checkers returns public keys that can be used to check cert authorities
203203
Checkers() ([]ssh.PublicKey, error)
204204
// Signers returns a list of signers that could be used to sign keys
205-
Signers() ([]ssh.Signer, error)
205+
Signers(alg string) ([]ssh.Signer, error)
206206
// V1 returns V1 version of the resource
207207
V1() *CertAuthorityV1
208208
// V2 returns V2 version of the resource
@@ -546,15 +546,17 @@ func (ca *CertAuthorityV2) Checkers() ([]ssh.PublicKey, error) {
546546
return out, nil
547547
}
548548

549-
// Signers returns a list of signers that could be used to sign keys
550-
func (ca *CertAuthorityV2) Signers() ([]ssh.Signer, error) {
549+
// Signers returns a list of signers that could be used to sign keys.
550+
//
551+
// The optional alg flag can be used to override the signature algorithm.
552+
func (ca *CertAuthorityV2) Signers(alg string) ([]ssh.Signer, error) {
551553
out := make([]ssh.Signer, 0, len(ca.Spec.SigningKeys))
552554
for _, keyBytes := range ca.Spec.SigningKeys {
553555
signer, err := ssh.ParsePrivateKey(keyBytes)
554556
if err != nil {
555557
return nil, trace.Wrap(err)
556558
}
557-
signer = sshutils.CompatSigner(signer)
559+
signer = sshutils.AlgSigner(signer, alg)
558560
out = append(out, signer)
559561
}
560562
return out, nil
@@ -570,7 +572,7 @@ func (ca *CertAuthorityV2) Check() error {
570572
if err != nil {
571573
return trace.Wrap(err)
572574
}
573-
_, err = ca.Signers()
575+
_, err = ca.Signers("")
574576
if err != nil {
575577
return trace.Wrap(err)
576578
}

Diff for: lib/sshutils/fingerprint.go

-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,5 @@ func PrivateKeyFingerprint(keyBytes []byte) (string, error) {
2727
if err != nil {
2828
return "", trace.Wrap(err)
2929
}
30-
signer = CompatSigner(signer)
3130
return Fingerprint(signer.PublicKey()), nil
3231
}

0 commit comments

Comments
 (0)