Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce SHA-512 for RSA SSH signatures #3777

Merged
merged 9 commits into from
Jun 24, 2020
Merged

Conversation

awly
Copy link
Contributor

@awly awly commented May 27, 2020

Motivation:

x/crypto/ssh defaults to using SHA-1 for signatures:
https://github.com/golang/crypto/blob/master/ssh/keys.go#L963-L982
Because Teleport uses RSA for user, host and CA keys, we end up with
SHA-1 by default.

SHA-1 is now considered weak and OpenSSH plans to deprecate it:
https://www.openssh.com/txt/release-8.3

Fix:

Wrap all RSA `ssh.Signer`s and override `SignWithAlgorithm` to
provide `SigAlgoRSASHA2512` if not otherwise specified. This will
only affect new certs, existing certs will use `SigAlgoRSA` until
rotated. For CA certs (e.g. exported with `tctl auth export`) users
might need to manually rotate.

I somewhat blindly wrapped all non-test places where we create ssh.Signers. If you think there's unnecessary calls or missing ones, please comment.

Limited local testing with openssh 8.2 client and
-oHostKeyAlgorithms=-ssh-rsa confirms that this works with a new
cluster and fails with an old one.

Fixes #3742

@russjones
Copy link
Contributor

russjones commented Jun 3, 2020

@awly Releases of OpenSSH below 7.2 will not support SHA-512 signed certificates. The good news is that Ubuntu 16.04+, CentOS 7+, and macOS 10.12 (Sierra) all have releases greater than OpenSSH 7.2.

The bad news is that CentOS 6 has an older release and we have customers still on CentOS 6. This means we'll have to add a flag to tell the Auth Server to continue issuing ssh-rsa signatures.

I like making this the default behavior, but we should do some more testing around compatibility before we make it the default behavior. We don't want someone to upgrade their cluster and no longer have access to it.

@awly
Copy link
Contributor Author

awly commented Jun 4, 2020

The bad news is that CentOS 6 has an older release and we have customers still on CentOS 6. This means we'll have to add a flag to tell the Auth Server to continue issuing ssh-rsa signatures.

Ahh, ok. It's unfortunate that we support outdated OSs and integration with outdated userspace software on them.

I'll add a config file field, next to the other crypto settings we have. OpenSSH has a similar field called CASignatureAlgorithms.

@awly awly force-pushed the andrew/ssh-rsa-sha2-signer branch from 07a2150 to 0896943 Compare June 4, 2020 23:06
@awly
Copy link
Contributor Author

awly commented Jun 5, 2020

@russjones PTAL

I added a config field for this, which defaults to SHA-512.
This field only needs to be set on the auth server. Then it propagates like so:

  • all new user/host certs get signed by the CA using the specified algo
  • user/host cert consumers use the same signature algo as was used by the CA to sign certs (taken from ssh.Certificate.Signature.Format field)

@@ -479,6 +484,7 @@ func ApplyDefaults(cfg *Config) {
cfg.Ciphers = sc.Ciphers
cfg.KEXAlgorithms = kex
cfg.MACAlgorithms = macs
cfg.CASignatureAlgorithm = ssh.SigAlgoRSASHA2512
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like if you don't specify this argument, you get opted into rsa-sha2-512?

I'm concerned about the long tail of clients and breaking peoples clusters. I'm thinking the safest path forward would be the following:

  • For new clusters, start out with rsa-sha2-512. Support downgrading to ssh-rsa if needed for compatibility reasons.
  • For existing clusters, continue to use the existing algorithm, but log a warning about the dangers of using an insecure algorithm with a link to documentation explaining how to upgrade cluster.

This way we don't surprise any cluster administrators. What do you think?

@awly
Copy link
Contributor Author

awly commented Jun 10, 2020

For existing clusters - preserve signature type. Log warnings in tctl in teleport.
For new clusters default to SHA512 (unless explicitly opted out)

@awly awly force-pushed the andrew/ssh-rsa-sha2-signer branch 3 times, most recently from c662637 to 4caf9cb Compare June 12, 2020 23:48
@awly
Copy link
Contributor Author

awly commented Jun 12, 2020

OK, reworked this per our convo with @russjones
High-level overview:

  • signature algorithm is stored along with CA in the backend
    • if missing, assume SHA1 (the old behavior)
  • startup/upgrade:
    • existing CAs are unchanged
    • new CAs default to SHA2-256
    • setting ca_signature_algo in teleport.yaml can override the behavior for new CAs
  • rotation:
    • existing CAs are unchanged by default
    • setting ca_signature_algo in teleport.yaml will change the algorithm on next rotation
  • to migrate to SHA2, users need to update their config and do a rotation

PTAL

@awly awly requested a review from russjones June 12, 2020 23:55
@awly awly force-pushed the andrew/ssh-rsa-sha2-signer branch from 4caf9cb to 9ec4435 Compare June 18, 2020 18:21
@awly
Copy link
Contributor Author

awly commented Jun 18, 2020

Good thing we had those integration tests!
I was actually silently skipping the new signing algo when using a user cert to do handshakes, which is why things worked in manual testing. Host-to-host authn (with trusted clusters) exposed the bug.

This check on the server side of the handshake is preventing our new signing algorithm. There doesn't seem to be a way around it.

Which means that:

  • we can sign user/host certs with SHA2-512
  • we can NOT use SHA2-512 for SSH handshakes

And I'm pretty sure the way I change cert signing is against the spec (SSH certs should be using formats like rsa-sha2-512-cert-v01@openssh.com, not rsa-sha2-512).

This leaves us with 3 options:

  1. fork x/crypto/ssh with a one-line change to allow our signing algorithms and use this PR
  2. fork x/crypto/ssh with https://go-review.googlesource.com/c/crypto/+/220037 and use only the parts of this PR that deal with switching alg on rotation
  3. keep that part of this PR that switches to SHA2 for cert signing ONLY and wait until OpenSSH deprecates SHA-1 for actual handshakes or x/crypto/ssh patch lands upstream

Need to think through this some more...

@awly awly force-pushed the andrew/ssh-rsa-sha2-signer branch from 9ec4435 to c7837e4 Compare June 23, 2020 22:35
@awly
Copy link
Contributor Author

awly commented Jun 23, 2020

OK, I reverted the changes that affect SSH handshakes. This is option No.3.
This PR now only uses SHA2 for cert signing, which makes OpenSSH happy. Upcoming OpenSSH releases will deprecate SHA2 for handshakes as well, so we'll have to revisit this problem.

This also unblocks 4.3 testing, since currently ssh will always fail against our SHA1-signed certs.

Andrew Lytvynov added 9 commits June 24, 2020 11:48
Motivation:

    x/crypto/ssh defaults to using SHA-1 for signatures:
    https://github.com/golang/crypto/blob/master/ssh/keys.go#L963-L982
    Because Teleport uses RSA for user, host and CA keys, we end up with
    SHA-1 by default.

    SHA-1 is now considered weak and OpenSSH plans to deprecate it:
    https://www.openssh.com/txt/release-8.3

Fix:

    Wrap all RSA `ssh.Signer`s and override `SignWithAlgorithm` to
    provide `SigAlgoRSASHA2512` if not otherwise specified. This will
    only affect new certs, existing certs will use `SigAlgoRSA` until
    rotated. For CA certs (e.g. exported with `tctl auth export`) users
    might need to manually rotate.

Limited local testing with openssh 8.2 client and
`-oHostKeyAlgorithms=-ssh-rsa` confirms that this works with a new
cluster and fails with an old one.
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.
Store the signing algorithm along the CA private key. When reading old
CAs that don't have it set, default to UNKNOWN proto enum which
corresponds to the old SHA1-based signing alg.

The only time you get a SHA2 signature is when creating a fresh cluster
and generating a new CA. This can be disabled in the config.
This allows users to manually switch to a different algorithm by:
- setting the config file field
- running "tctl auth rotate"

If config file field is not set, existing signing algorithm of the CA is
preserved.
connectedPeer is used as reversetunnel.Site outside of the package. Its
connInfo field must be synchronized.
It's no longer needed, since CertAuthority contains the signing
algorithm internally.
Also fix a few bugs along the way.
Previously we matched the public key type for only plain public key
authn.
x/crypto/ssh does not support SHA2 signatures for handshakes yet. We'll
keep using SHA2 for cert signing, but handshakes have to wait.
@awly awly force-pushed the andrew/ssh-rsa-sha2-signer branch from c7837e4 to 0d49b29 Compare June 24, 2020 18:56
@awly awly merged commit d326010 into master Jun 24, 2020
@awly awly deleted the andrew/ssh-rsa-sha2-signer branch June 24, 2020 21:25
awly pushed a commit that referenced this pull request Jun 24, 2020
awly pushed a commit that referenced this pull request Jun 25, 2020
awly pushed a commit that referenced this pull request Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Teleport server certs are signed with ssh-rsa-cert-v01@openssh.com, which was deprecated in OpenSSH 8.2
2 participants