Closed nemesifier closed 4 years ago
I think we should have an option for "any" IP address as well, maybe if field is 0.0.0.0
, then any IP address should be allowed, what do you think?
@atb00ker can I ask you how did you come up to this conclusion?
The freeradius servers allowed to communicate to the API endpoints that are dedicated to freeradius (authorize, accounting, postauth) are known beforehand. Allowing any ip to use these endpoints is a serious security breach because it would allow anyone to interfere with the authentication, authorization and accounting mechanism of all the organizations using OpenWISP (imagine what this means for an ISP). Somebody can try to impersonate freeradius and send accounting stop requests, try to brute force authentication attempts using authorize, and so on, I cannot even think of all the possible attacks that are made possible by this.
During development, the list could be set as to contain only localhost/127.0.0.1.
Yes, I realize it's not safe, I didn't mean it for production systems but for testing / development / staging to bypass this security, say in kubernetes while testing 127.0.0.1
will not do the trick. But I understand that this feature can be implemented later should the need be felt later, I'll do basic implementation of the feature in the top comment. :smile:
To be done after #88.
There must be a way to restrict the IP addresses which can consume the API endpoints
authorize
,accounting
andpostauth
.I would add this at organization level using OrganizationRadiusSettings, using something like a string field: a comma separated string with the ip-addresses (the string will have to be validated).
I would add a setting which defines the default allowed IP addresses to consume the freeradius API, defaulting to empty list.
If the new setting is not empty, the model field should be allowed to be left blank, and the IP addresses from the setting should be used.
If the new setting is empty, the model field shall not be allowed to be blank.
If the model field is filled, it will override what's in the settings.
We need to document this.