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

StrictHostKeyChecking=no is insecure #696

Closed
elitak opened this issue Jun 29, 2017 · 13 comments
Closed

StrictHostKeyChecking=no is insecure #696

elitak opened this issue Jun 29, 2017 · 13 comments

Comments

@elitak
Copy link

elitak commented Jun 29, 2017

The hardcoded -o StrictHostKeyChecking=no everywhere is a big SecOps no-no. It's quite feasible an attacker could wind up with an IP address you neglect to change after relinquishing, and have an entire host config hand-delivered to him to inspect for vulnerabilities. He wouldn't be able to MITM the the deployment, but obtaining what is essentially a dump of the host's whole filesystem is still pretty disastrous, from a defensive standpoint.

Ideallly, as @oconnorr suggested in #262, nixops would have its own known_hosts mapping cached, but short of that, I'd like to at least have the option of turning on StrictHostKeyChecking=yes permanently somehow. It really should be at least =ask by default, too, but I expect handling the user interaction could cause some grief. I'd much rather have to ssh-keygen -R $host; ssh $host : every time a host changes its keys, than compromise a big part of ssh's integrity for a small convenience.

@domenkozar
Copy link
Member

For context:

     StrictHostKeyChecking
	     If	this flag is set to ``yes'', ssh(1) will never automatically
	     add host keys to the ~/.ssh/known_hosts file, and refuses to con-
	     nect to hosts whose host key has changed.	This provides maximum
	     protection	against	trojan horse attacks, though it	can be annoy-
	     ing when the /etc/ssh/ssh_known_hosts file	is poorly maintained
	     or	when connections to new	hosts are frequently made.  This
	     option forces the user to manually	add all	new hosts.  If this
	     flag is set to ``no'', ssh	will automatically add new host	keys
	     to	the user known hosts files.  If	this flag is set to ``ask'',
	     new host keys will	be added to the	user known host	files only
	     after the user has	confirmed that is what they really want	to do,
	     and ssh will refuse to connect to hosts whose host	key has
	     changed.  The host	keys of	known hosts will be verified automati-
	     cally in all cases.  The argument must be ``yes'',	``no'',	or
	     ``ask''.  The default is ``ask''.

@elitak
Copy link
Author

elitak commented Jun 29, 2017

Incidentally, the wording of that paragraph is quite misleading, especially: The host keys of known hosts will be verified automatically in all cases.. That sounds to me like ssh will refuse to connect on a mismatch with the setting =no, but it does anyway. Only password auth and agent forwarding are forcefully disabled when there's a mismatch.

Also, semantically, what is a "strict check" anyway? The check is already done every time; strictness applies to policy enforcement. The option conflates two distinct concerns and should be divided into respective options: something like AutoAddKnownHosts=always|if-new|never|ask and KnownHostKeyMatchPolicy=strict|no-mismatch|lax|ask . Perhaps I'll file this upstream.

@nh2
Copy link
Contributor

nh2 commented Jul 17, 2017

Can we narrow down the "everywhere" a bit so that it's clear what parts are problematic?

$ git grep StrictHostKey
nix/ssh-tunnel.nix:       + " -o StrictHostKeyChecking=no"
nixops/backends/digital_ocean.py:            '-o', 'StrictHostKeyChecking=no',
nixops/backends/hetzner.py:             "-o", "StrictHostKeyChecking=no"]
nixops/backends/hetzner.py:            ["-o", "StrictHostKeyChecking=no",
nixops/backends/libvirtd.py:        return super_flags + ["-o", "StrictHostKeyChecking=no",
nixops/backends/none.py:            return super_state_flags + ["-o", "StrictHostKeyChecking=no", "-i", self.get_ssh_private_key_file()]
tests/none-backend.nix:      $coordinator->succeed("ssh -o StrictHostKeyChecking=no -v target1 ls / >&2");
tests/none-backend.nix:      $coordinator->succeed("ssh -o StrictHostKeyChecking=no -v target2 ls / >&2");
tests/none-backend.nix:                           "    -o StrictHostKeyChecking=no target1 : >&2");

It seems to me that for libvirtd, the flag is OK. AWS, GCE and Azure aren't present here, probably because they already have a secure host key fetching mechanism and thus don't need StrictHostKeyChecking=no.

So remaining are these 4 problematic cases:

  • ssh-tunnel
  • digital_ocean
  • hetzner
  • none

@elitak
Copy link
Author

elitak commented Jul 17, 2017

I reckon that nixops should use ssh-keyscan to obtain and store the host keys in the database (when the host is first deployed), thereafter generating in tmpfs a known_hosts file, whenever ssh needs to be run. Ssh commands that formerly used -o StrictHostKeyChecking=no instead use GlobalKnownHostsFile=/tmp/known_hosts.XXX -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=yes. Host keys are deleted from the db only when the machine is deleted or a new command like nixops clear-hostkey <machine> is run. Another command like nixops set-hostkey <machine> "<line from ssh-keyscan>" would be useful as well, for the paranoid.

All this is rather clunky, but I don't see a better way of doing it correctly, short of using some protocol-level ssh lib like paramiko to avoid the need to have the hostkeys on disk. I'm not too familiar with the nixops code, so I'm reluctant to attempt these changes without more input.

sorki added a commit to vpsfreecz/nixops that referenced this issue Aug 9, 2018
sorki added a commit to vpsfreecz/nixops that referenced this issue Aug 9, 2018
This backend is a stripped down version of "none" backend
without key pair generation / loading on the target machine.

It instead relies on system/user SSH config for both authentication
and host-key checking. This means that appropriate SSH keys needs to be added
to targets manually.

Related to NixOS#696 and many issues with SSH bootstrapping/recovery.
@sorki
Copy link
Member

sorki commented Aug 9, 2018

I've dropped this in vpsfreecz@c0affe0 completely to follow KISS principles and just use ~/.ssh/known_hosts as usual.

In vpsfreecz@510485d I've added a dumb backend which is like none backend but without keypair generation/storage in state - should respect ~/.ssh/config and allow for password and key pair authentication if public key is deployed by other means.

What do you think? Can revise these and create pull requests if it makes sense.

@elitak
Copy link
Author

elitak commented Aug 9, 2018

That suits my purposes. I'm happy to manage everything ssh-related in .ssh/known_hosts and .ssh/config, although perhaps there should be a way to append -o <ArbitraryOptions> to the ssh commandline? I can't think of any situation where it wouldn't be possible to handle all special behavior within a singular ssh_config, through the use of Match stanzas, but command-line tampering might still be more convenient, to some users.

Thanks for the effort in addressing this; I'd long forgotten about it.

@aszlig
Copy link
Member

aszlig commented Aug 31, 2018

Probably it would be a better idea to add functionality for managing ssh host keys in ssh_util, so that they're saved in the state db on first use. Back then when I wrote this comment I was deploying around 90 servers and it's really tedious to accept the host key for every single one.

However, I'd still set StrictHostKeyChecking to no for the Hetzner rescue system until we have that functionality, because on every reboot into rescue, you'll get a new host key.

Regarding the dumb backend: I'd just add an option for the none backend to not store the key in the state db.

@elitak
Copy link
Author

elitak commented Aug 31, 2018

I would use ssh-keyscan in cases like that, to deliberately reconstruct the known_hosts file. Keeping separate the intentions of performing a remote operation and changing host signatures is important, I think.

@coretemp
Copy link

coretemp commented Oct 8, 2018

The correct solution is to set the hostkey in Nix configuration. Preferably automatically with manual overrides depending on the backend. Any mechanism which depends on StrictHostKeyChecking not equal to yes is flawed.

I find it scary that anyone ever thought it was a good idea to implement it in the way it is currently working. It's basically a root kit on an infrastructure level for sufficiently motivated adversaries.

If this would be more popular software, it would receive a CVE number.

aszlig added a commit that referenced this issue Oct 20, 2018
From issue #696:

  The hardcoded -o StrictHostKeyChecking=no everywhere is a big SecOps
  no-no. It's quite feasible an attacker could wind up with an IP
  address you neglect to change after relinquishing, and have an entire
  host config hand-delivered to him to inspect for vulnerabilities. He
  wouldn't be able to MITM the the deployment, but obtaining what is
  essentially a dump of the host's whole filesystem is still pretty
  disastrous, from a defensive standpoint.

I by myself have been guilty of using this (added this to the Hetzner
backend), because I did actually misunderstand the meaning of setting
this option to no. My understanding was that it will refuse to connect
whenever an existing host key is different from that in known hosts.

However, this turns out to be only true for keyboard interactive or
password authentication and if we're using pubkey auth, OpenSSH will
happily connect.

Now the real fix for this (already deploying with a pre-generated host
key) is a bit more involved, but we can mitigate this for now, because
since OpenSSH 7.5 there is the "accept-new" option to
StrictHostKeyChecking, which does exactly what I thought "no" would do:

  If this flag is set to ``accept-new'' then ssh will automatically add
  new host keys to the user known hosts files, but will not permit
  connections to hosts with changed host keys.

Signed-off-by: aszlig <[email protected]>
@aszlig
Copy link
Member

aszlig commented Oct 20, 2018

Okay, I did actually misunderstand the implications of setting this to no (as written in the edit of https://discourse.nixos.org/t/green-carbon-neutral-nixos-hosting/1187/10), so while #1023 is not a full fix for this issue, setting it to accept-new would at least mitigate the most scary parts about it.

@aszlig
Copy link
Member

aszlig commented Oct 30, 2018

Okay, Hetzner does have an API enpoint to query the host key of the rescue system: https://robot.your-server.de/doc/webservice/en.html#get-boot-server-ip-rescue-last

I've added aszlig/hetzner#32 accordingly to get this into the library so we can make use of it via NixOps.

@coretemp
Copy link

Thanks @aszlig. The more backends we have, the better the cloud environment becomes.

Somewhat related to this is using nixops ssh to login to EC2 machines, which currently gives a choice, but it would be nicer if it would use EC2 APIs to also ensure automated security.

PsyanticY pushed a commit to PsyanticY/nixops that referenced this issue Nov 14, 2018
From issue NixOS#696:

  The hardcoded -o StrictHostKeyChecking=no everywhere is a big SecOps
  no-no. It's quite feasible an attacker could wind up with an IP
  address you neglect to change after relinquishing, and have an entire
  host config hand-delivered to him to inspect for vulnerabilities. He
  wouldn't be able to MITM the the deployment, but obtaining what is
  essentially a dump of the host's whole filesystem is still pretty
  disastrous, from a defensive standpoint.

I by myself have been guilty of using this (added this to the Hetzner
backend), because I did actually misunderstand the meaning of setting
this option to no. My understanding was that it will refuse to connect
whenever an existing host key is different from that in known hosts.

However, this turns out to be only true for keyboard interactive or
password authentication and if we're using pubkey auth, OpenSSH will
happily connect.

Now the real fix for this (already deploying with a pre-generated host
key) is a bit more involved, but we can mitigate this for now, because
since OpenSSH 7.5 there is the "accept-new" option to
StrictHostKeyChecking, which does exactly what I thought "no" would do:

  If this flag is set to ``accept-new'' then ssh will automatically add
  new host keys to the user known hosts files, but will not permit
  connections to hosts with changed host keys.

Signed-off-by: aszlig <[email protected]>
amemni pushed a commit to amemni/nixops that referenced this issue Jan 8, 2019
From issue NixOS#696:

  The hardcoded -o StrictHostKeyChecking=no everywhere is a big SecOps
  no-no. It's quite feasible an attacker could wind up with an IP
  address you neglect to change after relinquishing, and have an entire
  host config hand-delivered to him to inspect for vulnerabilities. He
  wouldn't be able to MITM the the deployment, but obtaining what is
  essentially a dump of the host's whole filesystem is still pretty
  disastrous, from a defensive standpoint.

I by myself have been guilty of using this (added this to the Hetzner
backend), because I did actually misunderstand the meaning of setting
this option to no. My understanding was that it will refuse to connect
whenever an existing host key is different from that in known hosts.

However, this turns out to be only true for keyboard interactive or
password authentication and if we're using pubkey auth, OpenSSH will
happily connect.

Now the real fix for this (already deploying with a pre-generated host
key) is a bit more involved, but we can mitigate this for now, because
since OpenSSH 7.5 there is the "accept-new" option to
StrictHostKeyChecking, which does exactly what I thought "no" would do:

  If this flag is set to ``accept-new'' then ssh will automatically add
  new host keys to the user known hosts files, but will not permit
  connections to hosts with changed host keys.

Signed-off-by: aszlig <[email protected]>
aither64 pushed a commit to aither64/nixops that referenced this issue Apr 15, 2019
This backend is a stripped down version of "none" backend
without key pair generation / loading on the target machine.

It instead relies on system/user SSH config for both authentication
and host-key checking. This means that appropriate SSH keys needs to be added
to targets manually.

Related to NixOS#696 and many issues with SSH bootstrapping/recovery.
aither64 pushed a commit to aither64/nixops that referenced this issue Apr 16, 2019
aither64 pushed a commit to aither64/nixops that referenced this issue Apr 16, 2019
This backend is a stripped down version of "none" backend
without key pair generation / loading on the target machine.

It instead relies on system/user SSH config for both authentication
and host-key checking. This means that appropriate SSH keys needs to be added
to targets manually.

Related to NixOS#696 and many issues with SSH bootstrapping/recovery.
@grahamc
Copy link
Member

grahamc commented Apr 20, 2020

We don't use =no anymore.

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

No branches or pull requests

7 participants