Use a SQL database for bans

[lokka30]

I suggest SQLite as the default just so servers can run out of the box.

For those running multiple servers referencing the same bans list, MySQL could be made available as a choice of database.

Hopefully this addition would help or mostly solve issues such as #114 and #619.

[NotAFile]

A major problem with SQLite is handling network range bans.

[lokka30]

A major problem with SQLite is handling network range bans.

Absolutely - in this case, a server network should use a remote database such as MySQL. SQLite is just a 'works out of the box' solution for most servers.

Unrelated, but I also want to add that it's highly likely that SQLite and MySQL can use the same set of queries from pique.

[godwhoa]

@lokka30 You might be misinterpreting; by network range they mean CIDR range.
Eg. Banning 10.0.0.0/24 would mean banning 256 IPs 10.0.0.[0-255].

Not sure how MySQL would help in this regard; do you have any specific schema or built-in functions in mind?

[lokka30]

@lokka30 You might be misinterpreting; by network range they mean CIDR range. Eg. Banning 10.0.0.0/24 would mean banning 256 IPs 10.0.0.[0-255].

Not sure how MySQL would help in this regard; do you have any specific schema or built-in functions in mind?

Ah, thank you for the clarification. I'm unsure how these would be stored - perhaps IP ranges could be stored as strings whilst specific IPs would be stored in bits, both columns are nullable but one is guaranteed to be present.

address ?type?	addressRange ?type?	reason VARCHAR(48)	removalDate DATE

[utf-4096]

Having SQLite as a ban backend would be nice, it's very portable and fast, and Python has built-in support for it through the sqlite3 module.

To implement this while keeping backwards compatibility and avoiding spaghetti code, I think it would be best to add a new config option specifying a module+class string in the config (example: piqueserver.bans.SQLiteBackend, piqueserver.bans.TXTBackend, ... -> piqueserver/bans.py class <name>Backend). Note: this is what the aloha-pk/piqueserver fork currently has.

For compat, I think we could keep piqueserver.bans.TXTBackend as a default in piqueserver/server.py but putting piqueserver.bans.SQLiteBackend in our example config.toml. Or maybe we should deprecate the TXT backend entirely and have something to migrate it over to SQLite? I'm not sure.

[utf-4096]

@lokka30 for the database structure, I would suggest this:

INT id	TEXT address	INT address_range	TEXT reason	INT created_at	?INT ends_at

Only ends_at is nullable in this case. Single addresses will have a range of 32 with IPv4, and a range of 128 for IPv6 (which we don't support yet, but future-proofing is always good).

[NotAFile]

The problem with it is kind of that we have to be able to look up wherher an IP is in any of the ranges efficiently. That's kind of hard without native support in the db.

[lokka30]

Thanks for your input utf and nota

I believe MySQL and Postgres have native support for IP addresses including IPV4 and IPV6 in the same field. INET may have been the type.

The INT address_range is ok for efficiency, simply adding it to the where clause when specifically comparing an ipv4 or ipv6 address (which is every case). However the TEXT address field will be slow to compare against and having to hash them (even if the DBMS does this automatically) is less preferable than a direct integer comparison via an inbuilt type like INET

I don't believe SQLite has this kind of type, which may make it less/un-suitable for this application

For backwards compatibility, is there an issue in having all servers migrate to the DB if we end up choosing a stable flatfile solution? Of course if this ends up only being a remote DB feature then txt will have to remain as default.

[NotAFile]

The flat file is simple enough that writing migrations for it wouldn't be hard.
The question is just, how many people are willing to set up dedicated DBs for this and, if we can not drop the flat file code anyway, what the benefits are.

I remember this originally being motivated by how long it takes to save the DB to a json file, which we I think somewhat mitigated by moving it to another thread? So another option here would be to simply continue keeping an index in memory, and only use the db for actually saving things.

[lokka30]

Forcibly moving away from the flatfile is certainly an option. Potentially the added difficulty for inexperienced users can be alleviated through recommending a 'flavour' of the current docker compose config recommended to also run a MariaDB service. That way you have a running server out of the box.

[NotAFile]

I don't personally think it makes sense to run a database just for bans for 99% of people. Basically anyone except aloha.