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

x/crypto/ssh: support server-side Diffie-Hellman Group Exchange #54743

Open
alextadams88 opened this issue Aug 29, 2022 · 11 comments
Open

x/crypto/ssh: support server-side Diffie-Hellman Group Exchange #54743

alextadams88 opened this issue Aug 29, 2022 · 11 comments
Labels
Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@alextadams88
Copy link

alextadams88 commented Aug 29, 2022

Support for Diffie-Hellman Group Exchange was proposed and added in the linked proposal: #17230

This was marked as closed and completed.

However, the PR that actually closed and completed this only implemented client-side kex. It did not implement server-side; only a minimal implementation to allow tests for the client-side to pass. See PR https://go-review.googlesource.com/c/crypto/+/174257 and commit golang/crypto@57b3e21.

I posit that the original issue should not have been marked as closed and completed because it was not truly completed as the Diffie-Hellman Group Exchange is still not fully supported.

@gopherbot gopherbot added this to the Proposal milestone Aug 29, 2022
@seankhliao seankhliao added the Proposal-Crypto Proposal related to crypto packages or other security issues label Aug 29, 2022
@seankhliao
Copy link
Member

cc @golang/security

@FiloSottile
Copy link
Contributor

Server-side support was intentionally left out. Diffie-Hellman Group Exchange is complex, and I wish we could have implemented none of it. People argued that it was necessary to connect to certain servers, so in the name of compatibility we added client-side support. What's the argument for adding server-side support?

@alextadams88
Copy link
Author

alextadams88 commented Aug 29, 2022

The original discussion thread did not contain a justification or discussion as to why server-side shouldn't be supported, so it appeared to me to be somewhat of an oversight as the discussion implied that this kex algorithm was fully supported, when it is not. That's why I created this proposal, just to make sure the team was aware that it was not fully supported.

As far as an argument, I can't provide anything except for information about my use-case, which doesn't really answer your question, but I'll provide the info regardless.

We have written an SFTP server using golang that allows our various downstream applications to SFTP backups to the server. It needs to do additional post-processing and forwarding to the backups, hence why we aren't just using OpenSSH on a filesystem. One of these applications supports only diffie-hellman-group-exchange-sha256, so as a result we cannot accept backups from that system. Because golang does not support this, we are looking into other solutions specifically for that application.

I suppose if I had an argument, it would simply be that this makes golang a poor choice for writing any kind of SSH server, if it means that arbitrary clients using non-deprecated algorithms will be unable to connect. In the future, it will make our team re-think using golang when solving problems like this.

@gopherbot
Copy link

Change https://go.dev/cl/532415 mentions this issue: ssh: add server side support for Diffie Hellman Group Exchange

@drakkan
Copy link
Member

drakkan commented Oct 31, 2023

@FiloSottile in DHGEX the server picks the group, so the server side should be safer than the client side. The proposed patch implements server side DHGEX to improve interoperability but hard code sending Oakley Group 14 or Oakley
Group 16 so basically this is the same as traditional DH security wise

cc @golang/proposal-review

@FiloSottile
Copy link
Contributor

We discussed this with @drakkan for https://go.dev/cl/532415, and indeed since the server selects the groups it's mostly the client side that is complex and risky, and that we already implemented.

The other advantage of implementing server-side support, too, is removing the logic for half-supported algorithms.

Let's go ahead with it.

@rsc
Copy link
Contributor

rsc commented Dec 12, 2023

Can someone say what new API this would involve, if any?

@drakkan
Copy link
Member

drakkan commented Dec 12, 2023

Can someone say what new API this would involve, if any?

No new API, we will simply remove the check to prohibit the use of server-side DHGEX and complete the server-side implementation which is currently only used in test cases and is not production ready.

In short we will add support for a new server-side algorithm but no changes to the API are needed because the algorithms are currently unexported strings

@rsc
Copy link
Contributor

rsc commented Dec 14, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Dec 21, 2023

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The proposal is to implement support for Diffie-Helman Group Exchange in the ssh server side. The client side already implements support, and that’s the more complicated one, since the server gets to pick the algorithms. There is no visible API change.

@rsc
Copy link
Contributor

rsc commented Jan 10, 2024

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The proposal is to implement support for Diffie-Helman Group Exchange in the ssh server side. The client side already implements support, and that’s the more complicated one, since the server gets to pick the algorithms. There is no visible API change.

@rsc rsc changed the title proposal: x/crypto/ssh: support server-side Diffie-Hellman Group Exchange x/crypto/ssh: support server-side Diffie-Hellman Group Exchange Jan 10, 2024
@rsc rsc modified the milestones: Proposal, Backlog Jan 10, 2024
drakkan added a commit to drakkan/crypto that referenced this issue Feb 24, 2024
We add this support for the following reasons:

- We are planning to expose recommended (secure) vs. supported (works,
  not necessarily recommended) algorithms. The DHGEX kex is currently
  only exposed as a client-side kex. To simplify the calling convention
  for this follow-on, we expose the server side too.
- Some clients are quite inflexible with reference to kex algorithms
  choice, for example they offer:

  diffie-hellman-group-exchange-sha256, diffie-hellman-group-exchange-sha1,
  diffie-hellman-group14-sha1, diffie-hellman-group1-sha1

  therefore DHGEX helps interoperability.

We do not recommend the DHGEX kex as a whole:

- the negotiation requires an extra round trip
- the server must generate parameters (slow) or hardcode them, which
  defeats the security benefit over traditional DH.

In this implementation we hardcode sending Oakley Group 14 or Oakley
Group 16.

Users that are concerned with security of classical DH kex should
migrate to kex based on EC or Ed25519.

Fixes golang/go#54743

Change-Id: I127822e90efc36821af4aca679931f40a2023021
drakkan added a commit to drakkan/crypto that referenced this issue Mar 7, 2024
We add this support for the following reasons:

- We are planning to expose recommended (secure) vs. supported (works,
  not necessarily recommended) algorithms. The DHGEX kex is currently
  only exposed as a client-side kex. To simplify the calling convention
  for this follow-on, we expose the server side too.
- Some clients are quite inflexible with reference to kex algorithms
  choice, for example they offer:

  diffie-hellman-group-exchange-sha256, diffie-hellman-group-exchange-sha1,
  diffie-hellman-group14-sha1, diffie-hellman-group1-sha1

  therefore DHGEX helps interoperability.

We do not recommend the DHGEX kex as a whole:

- the negotiation requires an extra round trip
- the server must generate parameters (slow) or hardcode them, which
  defeats the security benefit over traditional DH.

In this implementation we hardcode sending Oakley Group 14 or Oakley
Group 16.

Users that are concerned with security of classical DH kex should
migrate to kex based on EC or Ed25519.

Fixes golang/go#54743

Change-Id: I127822e90efc36821af4aca679931f40a2023021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Status: Accepted
Development

No branches or pull requests

6 participants