Skip to content
This repository was archived by the owner on Dec 26, 2020. It is now read-only.

Idempotency when changing sshd ports #299

Closed
nununo opened this issue Jun 29, 2020 · 9 comments · Fixed by #307
Closed

Idempotency when changing sshd ports #299

nununo opened this issue Jun 29, 2020 · 9 comments · Fixed by #307

Comments

@nununo
Copy link
Contributor

nununo commented Jun 29, 2020

Is your feature request related to a problem? Please describe.
If I want to change the default sshd port from 22 to 12345, the server's ansible_port should be set to 12345so that Ansible can keep connecting to the server after the change. But, since when I run ssh-hardening for the first time the port is still 22, ssh-hardening will not be able to connect. So I am forced to edit the inventory after the first run.

Describe the solution you'd like
The ssh-hardening rule could try to connect to both port 22 and the desired final port 12345. Just like this article suggests.

This way the inventory could be configured with the final port 12345 and ssh-hardening would be idempotent, work both in the first and subsequent runs.

Describe alternatives you've considered
Add a custom role that runs before ssh-hardening implementing the solution proposed in the article linked above.

Additional context
NA

@rndmh3ro
Copy link
Member

Hey @nununo,

thanks for raising this issue. While I personally don't change the ssh port, I can relate to this.

However looking at the code in the linked article, I'm not really open to add this much code for such a "minor" issue with idempotency.
What I'm open about is adding a section to the readme about this issue. This could link to another file in the repo where an example implementation is described. What do you think?

@nununo
Copy link
Contributor Author

nununo commented Jun 29, 2020

HI @rndmh3ro,

I can totally understand why you wouldn't want to add all that code. It's a bit of a mess. What you propose makes sense.

If you agree, the solution to be proposed in the readme could be comprised of 2 parts:

  1. Code to run BEFORE your role which would automatically overwrite ansible_port with the current port (22 or the desired one).
  2. Code to run AFTER your role which would reset ansible_port to the one in the inventory.

It shoud probably also explain that fact gathering must be explicitly disabled (and manually invoked after the port was changed and sshd restarted) since Ansible will fail to reach the server while sshd is not configured with the port defined in the inventory.

Thanks!

@rndmh3ro
Copy link
Member

Sounds great! Do you want to open a PR for this?

@nununo
Copy link
Contributor Author

nununo commented Jun 29, 2020

Sure. Give me some days to try to come up with something. I warn you though that I'm an Ansible noob so, while I'll do my best, please take that into consideration and be critical about whatever I propose :)

@nununo
Copy link
Contributor Author

nununo commented Jul 2, 2020

Hi @rndmh3ro, I forked the rep and tried to create something as seamless as possible. While I managed to greatly simplify the example mentioned above, it still has many moving parts. Have a look at it in the fallback branch of my fork if you want:
https://github.com/nununo/ansible-ssh-hardening/tree/fallback

  1. New hardening_with_fallback.yml built as a wrapper around your hardening.yml;
  2. Extra task at the top of hardening.yml to gather facts since playbook's gather_facts must be false in order for this approach to work;
  3. New default variable fallback_to_default_port set to false;
  4. hardening_with_fallback.yml overwites ssh_server_ports with the inventory port;
  5. main.yml decides which file to include: hardening.yml or hardening_with_fallback.yml based on fallback_to_default_port.

So, yeah, overall it does add a lot of complexity! Which is want you didn't want. Even I am not thrilled about it :-) But, alas, I wasn't able to make it simpler. So please be assured that I'm not trying to convince you to incorporate it :)

This being a very important feature to me (idempotency is not a "minor" issue to me) I'll probably just make a new role wrapped around yours. This will give me the best of both worlds :-)

Also, I don't think this can be explained in the README in a clear way. Both because this would make the README file overly complex and it would still be quite hard for someone to convert that information into something executable. But even if a solution is not proposed in the README, I still think this subject should be mentioned as it can be a source of confusion.

Let me know what your thoughts are.

Cheers,
Nuno

@rndmh3ro
Copy link
Member

rndmh3ro commented Jul 4, 2020

Hey @nununo, thanks for following up on this!

But even if a solution is not proposed in the README, I still think this subject should be mentioned as it can be a source of confusion.

How about you write that wrapper-role and we put a separate section in the readme that briefly describes the problem, links to this issue here and to your wrapper role?
This way we don't have to maintain more code but people looking for a solution to this problem can still find it?

@nununo
Copy link
Contributor Author

nununo commented Jul 5, 2020

Good idea.

I have created a first draft here:
https://github.com/nununo/ansible-ssh-hardening-fallback

It already works in my local environment. But it still needs some work. I want to make it more robust and user friendly.

Any tips/suggestions/criticism are welcome.

But what will definitely take more time is testing. I have never worked with Travis, Molecule or any of these CI testing pipelines before. So, while this is a good opportunity for me to learn how to use them, I'll first need to find the time. I'll keep you posted.

@rndmh3ro
Copy link
Member

rndmh3ro commented Jul 7, 2020

I tested your role briefly in Centos 7 in a Docker container and it worked fine and idempotent!

So I have no problem linking to it in the readme if you want to create a PR for this.

@nununo
Copy link
Contributor Author

nununo commented Jul 10, 2020

Those are good news!
It also seems to work in Debian 10.

I'll have to find the time to learn how to implement proper tests and then publish it in Galaxy. Once that is done I'll do the PR ;)

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants