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

ssh_config needs to allow ordering of host entries #5

Closed
wants to merge 0 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@raphink
Member

raphink commented Mar 30, 2015

I was implementing the SSH changes suggested here: http://stribika.github.io/2015/01/04/secure-secure-shell.html and ran into an interesting issue, when the provider generated the /etc/ssh/ssh_config file in this order:

 Host *
 KexAlgorithms diffie-hellman-group-exchange-sha256
 Host github.com
 KexAlgorithms diffie-hellman-group-exchange-sha256,diffie-hellman-group-exchange-sha1,diffie-  hellman-group14-sha1

Despite what you might think, the global * entry wins for the ssh config, causing github to not work; if you flip the order, it works perfectly. I think we need to be able to specify the order of hosts in the provider.

The same should probably apply to Match entries in sshd_config.

@raphink

This comment has been minimized.

Member

raphink commented Jan 12, 2015

So you're saying the first matching group is used, not the most specific one, which means Host * should always be last, and we should be able to place groups in the file.

@robbat2

This comment has been minimized.

robbat2 commented Jan 13, 2015

While Host * should be last, we should be able to specify the order.
Consider, this, from my per-user config; probably an extreme example, but what is normally possible in ssh_config.

Match user git
  BatchMode yes
Host specific.example.com
  User user1
  IdentityFile id_dsa_specific
Host *.example.com
  User user1
  IdentityFile id_dsa_example
Host *
...
@raphink

This comment has been minimized.

Member

raphink commented Jan 13, 2015

OK. Currently, the ssh_config type manages single entries, not Host groups.

Host groups are created if they don't exist, but they are not managed on their own. Should this be a new type to manage where Host groups are created, or should the positioning be an attribute of the ssh_config type (but then there might be conflicts between two ssh_config resources in the same group)?

@robbat2

This comment has been minimized.

robbat2 commented Jan 13, 2015

I think if there are conflicts between two ssh_config entries in the same group, the author is at fault anyway.

For multi-valued config options (*Forward, IdentityFile, SendEnv) detecting the conflicts is really hard anyway; and the order of those settings individually is not idempotent either :-(

@raphink

This comment has been minimized.

Member

raphink commented Jan 14, 2015

I still believe the group positioning should not belong with the ssh_config entry. Probably an ssh_entry_host type would be better, with an autorequire to make sure it's managed before the ssh_config entries in it. @domcleal, any opinion on this?

About multi-values options, does order matter?

@robbat2

This comment has been minimized.

robbat2 commented Jan 14, 2015

Yes, order matters on multi-valued options:

  • IdentityFile, if you have multiple ones, the server can decide you've tried too many (past sshd_config MaxAuthTries).
  • LocalForward/RemoteForward if they have conflicts, the first one wins, ssh spits a non-fatal error and continues on the rest.
  • You can chain some combinations of Local/RemoteForwards
@raphink

This comment has been minimized.

Member

raphink commented Jan 29, 2015

@domcleal What's your take on this issue?

@jcpunk

This comment has been minimized.

jcpunk commented Mar 23, 2015

+1 for interest in having this fixed

@raphink

This comment has been minimized.

Member

raphink commented Mar 30, 2015

Started attaching some code.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2015

Coverage Status

Coverage decreased (-7.45%) to 90.67% when pulling 1b5fab5 on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2015

Coverage Status

Coverage decreased (-5.32%) to 92.8% when pulling e98696d on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2015

Coverage Status

Coverage decreased (-5.32%) to 92.8% when pulling e98696d on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2015

Coverage Status

Coverage decreased (-5.32%) to 92.8% when pulling e98696d on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2015

Coverage Status

Coverage decreased (-5.02%) to 93.09% when pulling e98696d on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2015

Coverage Status

Coverage decreased (-3.52%) to 94.59% when pulling 10b2144 on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2015

Coverage Status

Coverage decreased (-4.03%) to 94.09% when pulling 1811d33 on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2015

Coverage Status

Coverage decreased (-4.03%) to 94.09% when pulling 1811d33 on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2015

Coverage Status

Coverage decreased (-6.12%) to 91.99% when pulling 1811d33 on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 30, 2015

Coverage Status

Coverage decreased (-4.03%) to 94.09% when pulling 1811d33 on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 31, 2015

Coverage Status

Coverage decreased (-4.03%) to 94.09% when pulling 1811d33 on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 31, 2015

Coverage Status

Coverage decreased (-4.03%) to 94.09% when pulling 1811d33 on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 31, 2015

Coverage Status

Coverage decreased (-4.03%) to 94.09% when pulling 1811d33 on raphink:issue/5 into bfd69b4 on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Mar 31, 2015

Coverage Status

Coverage decreased (-4.03%) to 94.09% when pulling 1811d33 on raphink:issue/5 into bfd69b4 on hercules-team:master.

@jcpunk

This comment has been minimized.

jcpunk commented Mar 31, 2015

This PR works for me, a few quick examples in README.md would be awesome for showing of just how to use this and how to sort out these blocks!

@raphink raphink force-pushed the raphink:issue/5 branch from 1811d33 to 04b6259 Apr 1, 2015

@coveralls

This comment has been minimized.

coveralls commented Apr 1, 2015

Coverage Status

Coverage increased (+0.13%) to 97.89% when pulling cc4acbe on raphink:issue/5 into 1e8731a on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Apr 1, 2015

Coverage Status

Coverage increased (+0.13%) to 97.89% when pulling cc4acbe on raphink:issue/5 into 1e8731a on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Apr 2, 2015

Coverage Status

Coverage increased (+0.13%) to 97.89% when pulling 857f6dd on raphink:issue/5 into 1e8731a on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Apr 2, 2015

Coverage Status

Coverage increased (+0.13%) to 97.89% when pulling 0321e65 on raphink:issue/5 into 1e8731a on hercules-team:master.

@coveralls

This comment has been minimized.

coveralls commented Apr 2, 2015

Coverage Status

Coverage decreased (-1.28%) to 96.48% when pulling 0321e65 on raphink:issue/5 into 1e8731a on hercules-team:master.

@raphink raphink force-pushed the raphink:issue/5 branch from 0321e65 to 4900e25 Apr 2, 2015

@coveralls

This comment has been minimized.

coveralls commented Apr 2, 2015

Coverage Status

Coverage increased (+0.13%) to 97.89% when pulling 4900e25 on raphink:issue/5 into fc87462 on hercules-team:master.

newparam(:target) do
isnamevar
desc "The file in which to the pg_hba rule"

This comment has been minimized.

@jcpunk

jcpunk Apr 13, 2015

perhaps ssh_config_match rather than pg_hba rule?

@raphink

This comment has been minimized.

Member

raphink commented May 25, 2015

I'm going to merge this for now, and move on to writing an ssh_config_match on the same model later.

@raphink raphink force-pushed the raphink:issue/5 branch 2 times, most recently from ceb3041 to 3678858 May 25, 2015

@coveralls

This comment has been minimized.

coveralls commented May 25, 2015

Coverage Status

Coverage decreased (-0.0%) to 98.21% when pulling 3678858 on raphink:issue/5 into e185af2 on hercules-team:master.

@raphink raphink closed this May 25, 2015

@raphink raphink force-pushed the raphink:issue/5 branch from 469bcdf to 3d80611 May 25, 2015

@coveralls

This comment has been minimized.

coveralls commented May 25, 2015

Coverage Status

Coverage increased (+0.02%) to 98.225% when pulling 469bcdf on raphink:issue/5 into e185af2 on hercules-team:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment