Skip to content

Conversation

@mib1185
Copy link
Contributor

@mib1185 mib1185 commented Sep 19, 2024

This will allow to enable password-based login for sftp, while it is still disabled for ssh connections.
To prevent a breaking change, this new option sftp_password_login inherits from ssh_server_password_login

@mib1185 mib1185 force-pushed the allow-seperate-password-login-for-sftp branch 3 times, most recently from a895c5f to b3bdd13 Compare September 19, 2024 11:29
@schurzi
Copy link
Contributor

schurzi commented Oct 13, 2024

Your proposal seems reasonable, but we really want to avoid introducing more variables for our roles.

I have spent some time thinking about this and I think you can use existing variables to achieve the same result. We offer a way to add more custom group matches to sshd config via ssh_server_match_group. All these matches are placed after our main config and can be used to override previous config. so if you add these variables you should be able to get this working without changing the role.

Can you try adding these variables?

    ssh_server_match_group:
      - group: sftponly
        rules:
          - PasswordAuthentication yes

@mib1185 mib1185 marked this pull request as draft October 16, 2024 08:29
@mib1185
Copy link
Contributor Author

mib1185 commented Nov 15, 2024

Hi @schurzi
sorry for late reply. thanks for this suggestion, we will check if it fits for our needs. Since we're in progress of re-thinking of the concept, which is affected by this settings, i'm going to close this PR for now.

@mib1185 mib1185 closed this Nov 15, 2024
@mib1185 mib1185 reopened this Feb 21, 2025
@mib1185
Copy link
Contributor Author

mib1185 commented Feb 21, 2025

Hi @schurzi
sorry for the very long delay, but I just come back to this topic now. I've checked your suggesiton with an additional match group section, but unfortunately when a rule is matched multiple times, then only the first is applied, so it is not possible to override with a subsequent match block. I can also confirm this, since I was trying to add such override the last 5 hours with no success 🙈

source: https://man7.org/linux/man-pages/man5/sshd_config.5.html

   Match   Introduces a conditional block.  If all of the criteria on
           the Match line are satisfied, the keywords on the
           following lines override those set in the global section
           of the config file, until either another Match line or the
           end of the file.  If a keyword appears in multiple Match
           blocks that are satisfied, only the first instance of the
           keyword is applied.

@mib1185 mib1185 marked this pull request as ready for review February 21, 2025 15:58
@mib1185
Copy link
Contributor Author

mib1185 commented Feb 25, 2025

An alternative solution should be, to move the current defined Match Group sftponly to the very end of the config, so one could add an own Match Group sftponly block with just the rules which should be overwritten. But this might cause to shadow the intended Match Group sftponly block by earlier rule matches 🤔

@schurzi
Copy link
Contributor

schurzi commented Feb 25, 2025

I can also confirm this, since I was trying to add such override the last 5 hours with no success 🙈

Damn usually the config works as last matching entry wins. I had not considered that this is reverse for Match rules.

Your proposal to put the default at the end would work with my initial suggestion and also avoid additional variables. I like that. Do you want to update this MR?

@mib1185 mib1185 force-pushed the allow-seperate-password-login-for-sftp branch 2 times, most recently from 7bda9dc to 5904ab8 Compare February 25, 2025 15:27
@mib1185 mib1185 force-pushed the allow-seperate-password-login-for-sftp branch from 5904ab8 to 4ae5acf Compare February 25, 2025 15:29
@mib1185
Copy link
Contributor Author

mib1185 commented Feb 25, 2025

I've rewokrd the PR as discussed. It is also tested and works as expected 👍

@schurzi schurzi changed the title Allow separate password login for sftp Allow to override settings for sftponly users Feb 25, 2025
@schurzi schurzi merged commit cf655a3 into dev-sec:master Feb 25, 2025
34 checks passed
@schurzi
Copy link
Contributor

schurzi commented Feb 25, 2025

awesome, thank you @mib1185!

@mib1185 mib1185 deleted the allow-seperate-password-login-for-sftp branch February 25, 2025 20:48
@mib1185
Copy link
Contributor Author

mib1185 commented Feb 26, 2025

Is there already a schedule for the next release of the collection?

@schurzi
Copy link
Contributor

schurzi commented Feb 26, 2025

Is there already a schedule for the next release of the collection?

now :)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants