What if the inventory file already exists? Seems like overwriting it blindly is a wrong idea. This is (could be) a problem when running multiple tasks at once - might be OK for your simple testing setup, but in production, this can easily lead to race conditions, when multiple processes write to the same file at once.
Overall, this would solve the issue you described in the comment, but at the same time, this creates more problems than actual value.
I'm even not sure that disabling host key checking is a good idea on its own, disregarding the implementation.

the os.path.join() is really unnecessary, as it only returns 'inventory' string here.

I tested this and the prompt appears every time we start a task with --libvirt. Obviously we want to avoid that.

After looking a bit into this, I think I'm in favor of using UserKnownHostsFile=/dev/null and StrictHostKeyChecking=no. But let's also add PasswordAuthentication=no. That way we will mandate that ssh logins must be non-interactive (which makes sense to me for Taskotron use case), and by using pubkey authentication, there should hopefully be no issue with disabling host key checking (your private key won't leak).

However, @jskladan is very correct about possible race conditions here, when we run runtask simultaneously for many tasks. Can you achieve the same thing with either ansible.cfg (I guess I'd prefer this) or ansible-playbook command line options?

I'm still not sure this is a proper way of doing it.
If we decide to go the "we don't care what we connect to" way, I'd much rather see this be configurable, instead of this being the default behaviour.
I'd much rather see a solution where we add (the correct one) ssh identity key to a file/location/variable, and make use of it, instead of just blindly disabling the key check.
@tflink - what do you think about this?

The basic question is "what is host key checking good for"? As far as I understand this, it's to be notified when the server ssh key changes, e.g. when potentially being replaced with an evil machine (DNS spoofing, etc). That is to prevent:

• leaking your password when connecting with a password-based authentication (I'm not actually sure this is true)
• preventing you from running a sensitive commands on such a machine

I think the first point doesn't apply for pubkey-based authentication. Also, with pubkey auth you can only log in when the attacker also has your public key (which they can, but it's yet another obstacle). And the second point is not quite valid for us, because we run publicly available tasks, and don't even expose the shell to the user.

Specifically for --libvirt, there's another problem. After a while, you get the same IP for a different VM, so if you used host key checking and written this into hosts file, then you get the usual "Somebody is doing something nasty!" error and a connection refusal. How do you intend to solve that? And is there really anything to solve? We create that machine. Why would we not trust it?

I could see some reasoning to be made for --ssh, but I don't see any for --libvirt.

Also, please note that with paramiko we used AutoAddPolicy, which is IIUIC basically the same thing as proposed here for ansible. So this patch is not a security approach change.

Not necessarily a change of the current state, but a reasonable time to discuss it - especially given the implementation, IMO. I can see the usecase for --libvirt, not so much for --ssh. But if @tflink agrees with you, and reasonable implementation is found, I'll yield.

It should be trivial to redo this patch to use ansible.cfg - ansible even has a special option for this, host_key_checking = False or via the command line args: --ssh-common-args=. If we want to distinguish --libvirt and --ssh, we will probably need to use the command line args.

So how exactly would you like this to work, Josef? If the host key is not present, would you auto-add it, require interactive confirmation, or fail with an error? If the is present and differs, there's only one option outside of ignoring it - fail with an error.

As I said - depends on Tim's PoW now. I don't necessarily need to see this changed, I just want to make sure this is what we really want to do.
Also - let's use the cmdline argument - makes more sense to use something explicit, than implicit (aka 'hidden' config option in ansible.cfg) IMO.

Here is the thing:I think it's a little annoying that I'm asking to input the yes every time,when I run some --libvirt tasks,which dosen't happen with develop branch code.Then I come up with the solution for ansible --libvirt tasks,and without thinking much about the potential attack,I think it's better to deploy the solution to --ssh tasks passingly.

Now there are three choices then:
first is putting "ssh_args = -C -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o PasswordAuthentication=no" into .ansible.cfg;
second is putting "if self.arg_data['libvirt']:
cmd.extend(['--ssh-extra-args', '-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o PasswordAuthentication=no'])" into executor.py
Or , just leave this alone.

The basic question is "what is host key checking good for"? As far as I understand this, it's to >be notified when the server ssh key changes, e.g. when potentially being replaced with an evil >machine (DNS spoofing, etc). That is to prevent:
leaking your password when connecting with a password-based authentication (I'm not >actually sure this is true)
preventing you from running a sensitive commands on such a machine
I think the first point doesn't apply for pubkey-based authentication. Also, with pubkey auth >you can only log in when the attacker also has your public key (which they can, but it's yet >another obstacle). And the second point is not quite valid for us, because we run publicly >available tasks, and don't even expose the shell to the user.
Specifically for --libvirt, there's another problem. After a while, you get the same IP for a >different VM, so if you used host key checking and written this into hosts file, then you get the >usual "Somebody is doing something nasty!" error and a connection refusal. How do you >intend to solve that? And is there really anything to solve? We create that machine. Why would >we not trust it?
I could see some reasoning to be made for --ssh, but I don't see any for --libvirt.
Also, please note that with paramiko we used AutoAddPolicy, which is IIUIC basically the same >thing as proposed here for ansible. So this patch is not a security approach change.

I'm totally agree with you handsome.

no longer applies