PowerDNS / weakforced

Anti-Abuse for servers at authentication time
GNU General Public License v3.0
124 stars 33 forks source link

Custom functions appear to return HTTP status 404 #136

Closed sshipway closed 7 years ago

sshipway commented 7 years ago

Created a custom function getstats to let me query the current database content. When calling this via the 8084 API, it returns the content correctly, but has an HTTP status 404. The function returns a true status, and the calling IP is in the ACL subnet.

function getstats (args)
sdb = getStringStatsDB("OneHourDB")
if ( args.attrs.remote and args.attrs.remote ~= "" )
then
  ip = newCA(args.attrs.remote)
  fpbyip = sdb:twGet(ip, "failedPasswords")
end
fpbylogin = sdb:twGet(args.attrs.login, "failedPasswords")
return true, { login=args.attrs.login, ip=args.attrs.remote, failedpasswordsbylogin=fpbylogin, failedpasswordsbyip=fpbyip }
end
setCustomEndpoint("getstats",false,getstats)

This seems to be an error in the wforced handler?

neilcook commented 7 years ago

Ok two things:

1) There is already a getDBStats command that does exactly what you want to do with this custom function. Look at docs/swagger/wforce_api.7.yml. 2) I've tried your function, and I don't get a 404, I get a 200 OK. Can you send an example session with output and logs? Are you using the latest commit from master?

neilcook commented 7 years ago

FYI, this is what I see when running your function: Neil-Cook-MBP-2:~ ncook$ curl -v -X POST -H "Content-Type: application/json" --data '{ "attrs": { "login":"ahu", "remote": "127.0.0.1", "pwhash":"1234'$a'", "success":"false"}}' -u wforce:super http://127.0.0.1:8084/?command=getstats Note: Unnecessary use of -X or --request, POST is already inferred.

Looking at the code, 404 is only returned if a command is not found.

sshipway commented 7 years ago

We're using the latest stable tag, v1.2.1, packaged into an RPM by OX. The getDBStats command (and the delBLendtry and getBL etc commands) look very useful, thanks for the pointer. If they're present in the version we're using I can certainly make use of them. I hadn't seen this extra documentation page - I'd been relying on the man pages and other online resources, which only mention allow/report/reset.

In our version, it's definitely returning a 404, but also returning the expected function output, which doesn't make sense to me. I'll investigate more tomorrow and see if I can get any more info - try with a null function, etc. When the next stable version comes out as an RPM I'll upgrade to that; I'm looking forward to the GeoIPCity info and performance stats queryable via the API.

neilcook commented 7 years ago

I hadn't seen this extra documentation page - I'd been relying on the man pages and other online resources, which only mention allow/report/reset.

http://oxpedia.org/wiki/index.php?title=AppSuite:Dovecot_Antiabuse_Shield contains a bunch of useful links, including a link to the API documentation which we maintain online. When you got the RPM from OX, they should have given you a link to this page, but maybe this didn't happen.

We're using the latest stable tag, v1.2.1, packaged into an RPM by OX.

In which case you might want to think about using the standard support mechanism for OX (e.g. our bugzilla instance). Are you working for/with SMX?

Also, I will try your custom function with 1.2.1, as I was testing against the latest master.

In our version, it's definitely returning a 404, but also returning the expected function output, which doesn't make sense to me.

I rewrote the web server handling code recently, which is probably why I couldn't reproduce in master.

When the next stable version comes out as an RPM I'll upgrade to that; I'm looking forward to the GeoIPCity info and performance stats queryable via the API.

Yes there will probably be a 1.3 release sometime in the next few months, as we've added some useful features since 1.2.1, and 2.0 won't be out until the end of the year.

neilcook commented 7 years ago

Ok I can reproduce the custom endpoint 404 issue, and I've identified the problem. As I said this is fixed in the dev branch - is this something you need fixing in the short-term, or can you wait for a 1.3 release in a few months?

sshipway commented 7 years ago

Thanks for the update. We can wait for the 1.3 release when it comes out, as we need that for the other new features anyway.

If you're looking for new feature suggestions, then a way to query if a given key already exists in an HLL database would be nice :) (so that an allow function can find out in advance if an HLL count will be incremented by the subsequent report)

neilcook commented 7 years ago

If you're looking for new feature suggestions, then a way to query if a given key already exists in an HLL database would be nice

This isn't really possible, given that the way HLL works. It's essentially a bitmap, and so it doesn't preserve what was put in, only the cardinality. The only way to find out if a key already exists is indeed to add a new value, and see if the count gets incremented. If you want that behaviour explicitly, you could look at the countmin type, (which takes a lot more RAM BTW - around 3x that of HLL), but that gives you counts-per-value. I could also potentially add a bloom filter type in future, which simply tells you whether a given value exists in a set (i.e. was it already added), but doesn't give cardinality.

neilcook commented 7 years ago

Closing as fixed in master