pre_login_hook connection IP

[antoinedeschenes]

Hi, do you think it would it make sense to expose the client IP to the pre_login_hook?

I'd like to implement fail2ban and geoip lookup, but currently, the only way I can figure to deny an IP address in a docker container without extra privileges would be to feed the denied_ip list via dynamic auth. Two negatives, the denied_ip list could get quite large, and I can't deny the IP until at least one failed login happened.

[drakkan]

Hi,

do you want to check the client IP against a, possibile large, blacklist yourself or do you want to use fail2ban for this?

The IP filters feature is heavily inspired to Pure-FTPd, do you know a server software that works as you expect? This way I can better understand what do you mean and take inspiration to add a similar feature to SFTPGo :) thank you!

[antoinedeschenes]

Hi,

do you want to check the client IP against a, possibile large, blacklist yourself

Exactly, a little like /etc/hosts.allow and /etc/hosts.deny work for sshd. One can spawn a script to allow or deny connections. I believe it would make more sense to implement this as a separate hook on the connection handler than the pre_login_hook

[drakkan]

Hi,

I need some time to understand what is the best way to add this feature, we could:

1) add connection info (remote address and client version) to pre-login hook and to external auth hook too 2) add a separate hook 3) add a new table/structure inside the data provider and populate it via REST API

My first impression is that we could avoid to add a new hook and adding connection info to the existing ones. Solution 3 requires more effort and we should add inside SFTPGo itself very specific features such as geoip lookup.

I want to verify how other similar sw handle this requirement too.

Currently I'm working to some refactoring to prepare SFTPGo to expose other protocols, so you have to wait a few weeks for this feature, sorry

[sergey-dryabzhinsky]

Hi, @drakkan @antoinedeschenes

Pre-login hook is a good feature, but here is some issues ahead.

1. Calling for external script may cause security issue
2. sftpGo uses it internal DB, so fail2ban / denyhosts may not found system-level users

Anyway, hook callback must implement simple logick:

• sftpGo check it users database if user exist
• call hook script with params: {login} {ip} {exist}
• wait (some time) for return code: 0 (all good), 1 {all bad)
• pass login process (0), or kick (1)

May be for more tight work with protecting soft sftpGo need "pre-connect" hook to check {ip} before even try to login?

[drakkan]

Hi, @drakkan @antoinedeschenes

Pre-login hook is a good feature, but here is some issues ahead.

1. Calling for external script may cause security issue
2. sftpGo uses it internal DB, so fail2ban / denyhosts may not found system-level users

Anyway, hook callback must implement simple logick:

• sftpGo check it users database if user exist
• call hook script with params: {login} {ip} {exist}
• wait (some time) for return code: 0 (all good), 1 {all bad)
• pass login process (0), or kick (1)

Hi, this is basically what pre_login hook already do, from the doc:

""" The external program can read the following environment variables to get info about the user trying to login:

SFTPGO_LOGIND_USER, it contains the user trying to login serialized as JSON. A JSON serialized user id equal to zero means the user does not exists inside SFTPGo SFTPGO_LOGIND_METHOD, possible values are: password, publickey and keyboard-interactive """

I think the only missing thing here is to add the ip and maybe the client version to the pre-login hook. This way if your blacklist is not so big you can store it directly inside the sftpgo database and you don't need a pre-login hook at all, alternately you can use the pre-login hook if you have a huge blacklist or if you want to check geoip or similar things.

I don't know fail2ban/denyhost very well, if you need some modifications to the log files to make these external software work better please open separate issues, thank you!

May be for more tight work with protecting soft sftpGo need "pre-connect" hook to check {ip} before even try to login?

[sergey-dryabzhinsky]

@drakkan

I'm sure if you add new ENV vars like SFTPGO_LOGIND_IP, SFTPGO_LOGIND_CLIENT_VERSION that would be enough for hook scripts to work properly.

I'll look out about logs compatibility with fail2ban/denyhosts ASAP.

[antoinedeschenes]

An advantage of a "pre_connect" hook would be to deny any connection attempt even before the login prompt is exposed

[sergey-dryabzhinsky]

The main purpose of pre_login / pre_connect hook scripts is to temporary (by X minutes/hours) or permanent block connections from {ip} by firewall. So they ({ip}) not bother sftpGo anyhow.

Criteria are different:

• non-existing user
• connection-from-ip-per-second - sftpGo missing that param calculation, may be external if {ip} will be sended
• wrong password (i.e. user exists) - sftpGo missing that check I think, not passed to hook.

Maybe add new ENV SFTPGO_LOGIND_CHECK:

• 'logged' - if every thing ok, user exists and password ok
• 'badpass' - password not equal
• 'notfound' - user not found
• any other check result

Sounds like post_login description now...

[sergey-dryabzhinsky]

@antoinedeschenes, agree.

[drakkan]

An advantage of a "pre_connect" hook would be to deny any connection attempt even before the login prompt is exposed

ah ok, now I understand what you mean.

@antoinedeschenes can you please explain what is missing inside the connection failed logs to make this work using fail2ban? As said above I don't know fail2ban very well and the fail2ban filter was user contributed, @binou-31 do you have something to say about this?

[drakkan]

The main purpose of pre_login / pre_connect hook scripts is to temporary (by X minutes/hours) or permanent block connections from {ip} by firewall. So they ({ip}) not bother sftpGo anyhow.

Criteria are different:

• non-existing user
• connection-from-ip-per-second - sftpGo missing that param calculation, may be external if {ip} will be sended
• wrong password (i.e. user exists) - sftpGo missing that check I think, not passed to hook.

Maybe add new ENV SFTPGO_LOGIND_CHECK:

• 'logged' - if every thing ok, user exists and password ok
• 'badpass' - password not equal
• 'notfound' - user not found
• any other check result

Sounds like post_login description now...

Hi, I think these info can be parsed from the log files, take a look at the connection failed logs, the provided fail2ban filter should already be able to parse these logs

[drakkan]

To summarize:

• please let me know if I can improve the connection failed logs to help fail2ban to work better, the configurations here should work
• I could consider to add a pre-connect hook, as suggested above, to reject the client as soon as it connects if you still find it useful, but maybe using fail2ban or something similar is better (it should support geoip lookups too)
• if you have a limited number of ip/netmask to allow/deny you can use the SFTPGo built-in feature and, if needed, you can change the list of allowed/deny ip address before the login using the pre-login hook

do you have other suggestions? Thank you.

[drakkan]

I added a post-login hook too, combing it with the post-connect hook you should be able to implement all the features discussed here

[antoinedeschenes]

Hi @drakkan, excellent, I have a "filter" sidecar in the works, along with helm charts for sftpgo and sftpgo-filter on kubernetes. I'm already using the pre_login_hook to filter against a GeoIP2 database and a DNSBL blocklist, I'll now be able to handle login failures with this

[drakkan]

Great! If you want to contribute something back I will be glad to add your filter in the examples directory or a link to your repo somewhere in the doc, thank you!