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

enable GCM and Chacha20 ciphers #192

Merged
merged 2 commits into from
Jul 3, 2018

Conversation

drizzd
Copy link
Contributor

@drizzd drizzd commented Jun 20, 2018

These are offered by ssh by default, but they are not included in the
default list of Ciphers in ssh_config.

Instead of setting the entire list of ciphers, selectively re-enable the
CBC ciphers using the "Cipher +somecipher" notation.

Closes git-for-windows/git#1723.

@dscho
Copy link
Member

dscho commented Jun 20, 2018

Thank you!

I do have two requests, still, before merging time:

  1. could you Sign-off on that patch?
  2. existing Git for Windows SDKs will already have an /etc/ssh_config that was patched by the current git-extra (or an earlier one). Could you make it so that this case is handled, too, by reverting the previous edit (something like ! grep '^Cipher .*,aes256-cbc,aes192-cbc' /etc/ssh/ssh_config || sed -i -e 's|^\(Cipher .*\),aes256-cbc,aes192-cbc|# \1|' /etc/ssh/ssh_config)?

@drizzd
Copy link
Contributor Author

drizzd commented Jun 21, 2018

Good point. I can do this, but if a user modifies /etc/ssh/ssh_config, then we may overwrite their modifications too. I think the right way is to patch the openssh package instead. Since we already have some patches there, that's no extra maintenance overhead. I created a pull request there:
git-for-windows/MSYS2-packages#24

The only downside is that due to the modification by git-extra, the new ssh_config will be installed as /etc/ssh/ssh_config.new with a warning.

@dscho
Copy link
Member

dscho commented Jun 22, 2018

The only downside is that due to the modification by git-extra, the new ssh_config will be installed as /etc/ssh/ssh_config.new with a warning.

That's not the only downside. It would make it quite a bit harder to revert the supposedly temporary workaround of re-enabling weak ciphers to accommodate a few hosters (all of which have updated their SSH to handle strong ciphers in the meantime AFAICT).

{
# Revert change by prior versions of git-extra.
sed -i -e 's/^Ciphers aes128-ctr,aes192-ctr,aes256-ctr,aes128-cbc,3des-cbc,aes256-cbc,aes192-cbc$/# Ciphers aes128-ctr,aes192-ctr,aes256-ctr,aes128-cbc,3des-cbc/' \
/etc/ssh/ssh_config

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

grep -q '^Ciphers .*aes256-cbc,aes192-cbc' /etc/ssh/ssh_config ||
sed -i -e '/^[# ]*Ciphers /{s/^# *//;s/$/,aes256-cbc,aes192-cbc/}' \
/etc/ssh/ssh_config
{

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

These are offered by ssh by default, but they are not included in the
default list of Ciphers in ssh_config.

Instead of setting the entire list of ciphers, selectively re-enable the
CBC ciphers using the "Cipher +somecipher" notation.

Closes git-for-windows/git#1723.

Signed-off-by: Clemens Buchacher <[email protected]>
@drizzd drizzd force-pushed the ssh-enable-ciphers branch from c33fb6e to 3cadc4c Compare July 1, 2018 16:45
@dscho dscho merged commit 7252ea8 into git-for-windows:master Jul 3, 2018
@dscho
Copy link
Member

dscho commented Jul 3, 2018

Thank you, @drizzd!

dscho added a commit that referenced this pull request Jul 3, 2018
The included OpenSSH client [now enables modern
ciphers](#192).

Signed-off-by: Johannes Schindelin <[email protected]>
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.

2 participants