Thank you all for the good feedback on this feature suggestion. In particular, thank you Leif for your ideas.
Just to restate the goal of this feature: it would be helpful to specify IP allow rules using IP category names that reference sets of IP ranges. This allows for a human readable and manageable IP allow file with rules for IP categories like ACME_INTERNAL, referencing IP addresses belonging within an organization's network. Orthogonal to this, updates to these ranges for any given IP category can happen without the rules specifications needing to be updated. My previous proposals specified the mapping from IP categories to ranges via a plugin API. A plugin could then be implemented which updates the mapping at any arbitrary time. I've changed the model to something much more modest and simple. Rather than adding an API, I've now updated the IP allow mechanism to simply read a yaml configuration that states the mapping of categories to IP ranges. This can be supplied either in the ip_allow.yaml file itself, or in a separate file configured via proxy.config.cache.ip_categories.filename. Categories are specified with nodes like the following: ip_categories: - name: ACME_INTERNAL ip_addrs: 1.2.3.4 - name: ACME_EXTERNAL ip_addrs: - 5.6.7.8 - 6.7.8.9 I've also added remap.config ACL support for IP categories via @src_ip_category=ACME_INTERNAL. I've posted a PR implementing these changes here: https://github.com/apache/trafficserver/pull/11004 Feedback welcome. Thanks, Brian On Sun, Dec 17, 2023 at 9:06 PM Leif Hedstrom <apachezw...@gmail.com> wrote: > How is this going to deal with changes to the data source? The plugin has > to reload the e tire data set, and we do an atomic swap or something? > > This seems particularly unmanageable when the data source is large, and/or > it changes frequently. > > This also implies that one IP range can have just one data source? Can it > still have multiple categories ? > > — Leif > > > On Dec 15, 2023, at 13:11, Brian Neradt <brian.ner...@gmail.com> wrote: > > > > After considering the feedback on this list and some offline feedback, > I've > > re-implemented this feature quite differently. From the user's > > configuration perspective of ip_allow.yaml and the category specification > > in remap.config, the proposal does not change. That is, the user can > still > > provide category specifications in ip_allow.yaml like this (same as the > > earlier proposal in this thread): > > > > - apply: in ip_category: ACME_INTERNAL action: allow methods: - > > GET - HEAD - POST - DELETE > > > > > > Further, categories are still supported for so-called remap.config ACLs > > like the following: > > > > map http://foo.example.com/ http://foo.example.com/ @action=allow > > *@src_ip_category=ACME_INTERNAL* @method=post @method=get @method=head > > > > > > However, Alan helpfully pointed out that it is reasonable and far more > > efficient for a plugin to specify the IP spaces for each category rather > > than simply provide one-off requests about whether any given address > > belongs to any given category. This allows the IpAllow component to > > complete the IPSpace for the records as it has before, making address > > lookup for the appropriate records very efficient. From the plugin's > > perspective, therefore, it is it simply needs to call the following API > to > > set the mapping of category to the analogous space: > > > > /** For each IP allow category, define the IP space associated with it. > > * > > * @param[in] category_map For each category name, the IP space > > associated with it. > > */ > > void TSHttpSetCategoryIPSpaces(std::unordered_map<std::string, > > swoc::IPSpace<bool>> const &category_spaces); > > > > Again, as before, the idea is that the plugin can populate the spaces > from > > parsing a configuration file, querying a database, or whatever other > > mechanism is appropriate for the organization. It is expected that > > typically a developer for an organization will implement the plugin that > > provides the category IP spaces while the organization's ops team will be > > able to use these categories to specify IP category filtering via > > ip_allow.yaml and remap.config ACLs. > > > > With these changes, the ip_allow.yaml and remap.config interfaces remain > > intuitive for the user, the API is simplified, and the IpAllow rule > > implementation for these categories is exactly as efficient as it is > today > > for the current IP ranges. > > > > I have created a separate draft PR demonstrating this here: > > https://github.com/apache/trafficserver/pull/10939 > > > > Please chime in with any further thoughts about the IP category feature. > > > > > > > >> On Thu, Nov 30, 2023 at 6:13 PM Shu Kit Chan <chanshu...@gmail.com> > wrote: > >> > >> So for each request, the plugin will be called into action X number of > >> times for X number of categories found? > >> > >> More specifically, > >> If there is a match in a category, it will not call the remaining > >> number of categories to try? (Since I assume the first match will take > >> precedence over the rest?) > >> If there is no match in all categories, it will still go through X > >> number of times. > >> > >> Is my understanding correct? > >> > >> Perhaps if we give the plugin handler an array of categories in the > >> listed order in the config yaml, And ask the handler to return either > >> one of the categories (that matches) or null if there is no match. > >> Will this be a bit more efficient or probably the same? > >> > >> Kit > >> > >> > >>> On Mon, Nov 27, 2023 at 9:50 PM Brian Neradt <brian.ner...@gmail.com> > >>> wrote: > >>> > >>> A quick update: > >>> > >>> This morning I realized that remap ACLs > >>> < > >> > https://docs.trafficserver.apache.org/en/latest/admin-guide/files/remap.config.en.html#acl-filters > >>> > >>> should to be updated for this too. I updated the draft PR to include > >> adding > >>> @src_ip_category that takes a category to apply to remap rules. That > >>> functions in every way like the ip_allow.yaml ip_category described in > >> the > >>> previous email. > >>> > >>> Thanks, > >>> Brian > >>> > >>> On Wed, Nov 22, 2023 at 4:29 PM Brian Neradt <brian.ner...@gmail.com> > >> wrote: > >>> > >>>> dev@trafficserver.apache.org, > >>>> > >>>> Bryan Call and I have been working on an idea for ip_allow.yaml. At > >> Yahoo, > >>>> we have an internal API which categorizes IPs under certain groups via > >> a > >>>> database lookup. Each of the IP categories have potentially different > >>>> requirements for which HTTP methods are accessible to them. For > >> instance, > >>>> IP addresses in something like ACME_INTERNAL may be able to initiate > >> HTTP > >>>> requests using modifying methods such as PUSH and DELETE as well as > >> GET and > >>>> HEAD requests, while IP addresses labeled ANY_IP may only be able to > >> use > >>>> requests with GET and HEAD methods. While the general filtering > >>>> functionality of ip_allow.yaml would be ideal for controlling these > >> HTTP > >>>> method restrictions, the current ip_addrs node of ip_allow.yaml, while > >> good > >>>> for explicit IP range specification, does not allow for programmatic > IP > >>>> categorization like that described above. > >>>> > >>>> Currently, Yahoo implements the required filtering by basically > >> stripping > >>>> ip_allow.yaml to nothing but the absolute minimum denominator of > method > >>>> restrictions and then uses an internal plugin to implement the method > >>>> filtering based upon the categories of the database. > >>>> > >>>> In discussing this method filtering by IP category, Bryan Call had the > >>>> idea that this feature could be implemented as a general solution for > >> the > >>>> community by a relatively simple extension of ip_allow.yaml itself. I > >> spent > >>>> some time working on this and have a draft PR demonstrating the > feature > >>>> here: > >>>> > >>>> https://github.com/apache/trafficserver/pull/10840 > >>>> > >>>> The feature includes the following new user interfaces: > >>>> > >>>> ip_category > >>>> The feature adds ip_category to ip_allow.yaml record as an alternative > >> to > >>>> the ip_addrs key. Here is an example record specification using an > >>>> ip_category of ACME_INTERNAL: > >>>> > >>>> - apply: in ip_category: ACME_INTERNAL action: allow methods: - > >> GET - HEAD - POST - DELETE > >>>> > >>>> TS_HTTP_IP_ALLOW_CATEGORY_HOOK hook > >>>> The user then supplies a plugin that implements the > >>>> TS_HTTP_IP_ALLOW_CATEGORY_HOOK callback which the core uses to query > >>>> whether a given incoming IP is applicable to a given category, such as > >>>> ACME_INTERNAL above. Knowing this informs the core whether a > particular > >>>> ip_allow.yaml rule applies to an incoming or outgoing connection. > >>>> > >>>> New Category Plugin API Functions > >>>> The plugin that implements the TS_HTTP_IP_ALLOW_CATEGORY_HOOK handler > >> then > >>>> uses the following API to communicate with the core about whether an > IP > >>>> belongs to a category: > >>>> > >>>> /** Obtain the category name being inspected. > >>>> * @param[in] infop The ip_allow info object. > >>>> * @param[out] category The category name being inspected whether it > >>>> contains > >>>> * the address obtained via @a TSHttpIpAllowInfoAddrGet. > >>>> * @return TS_SUCCESS if the category name was obtained, TS_ERROR > >>>> otherwise. > >>>> */ > >>>> TSReturnCode TSHttpIpAllowInfoCategoryGet(TSHttpIpAllowInfo infop, > >>>> std::string_view &category); > >>>> > >>>> /** Obtain the address being queried whether it belongs to a > >> category. > >>>> * @param[in] infop The ip_allow info object. > >>>> * @param[out] addr The address being queried against a category > >>>> obtained via > >>>> * @a TSHttpIpAllowInfoCategoryGet. > >>>> * @return TS_SUCCESS if the address was obtained, TS_ERROR > >> otherwise. > >>>> */ > >>>> TSReturnCode TSHttpIpAllowInfoAddrGet(TSHttpIpAllowInfo infop, > >> sockaddr > >>>> &addr); > >>>> > >>>> /** Set whether an address applies to a category. > >>>> * @param[in] infop The ip_allow info object. > >>>> * @param[in] contains Whether the address obtained via > >>>> * @a TSHttpIpAllowInfoAddrGet is contained in the category > >> obtained via > >>>> * @a TSHttpIpAllowInfoCategoryGet. > >>>> */ > >>>> void TSHttpIpAllowInfoContainsSet(TSHttpIpAllowInfo infop, bool > >>>> contains); > >>>> > >>>> The TSHttpIpAllowInfo is an opaque structure, but in the backend it > >> simply > >>>> contains the three pieces of data represented above: > >>>> > >>>> struct HttpIpAllowInfo { > >>>> > >>>> /// A user-specified name for IP categories. > >>>> std::string_view category; > >>>> > >>>> /// The IP address to query whether it is contained in @a category. > >>>> swoc::IPAddr const &addr; > >>>> > >>>> /// The result of the query: the hook sets this to true of @a addr > >> is in > >>>> @a > >>>> /// category, false otherwise. > >>>> bool contains = false; > >>>> }; > >>>> > >>>> An example plugin that makes use of this API can be found in the draft > >> PR > >>>> here: > >>>> > >>>> > >>>> > >> > https://github.com/bneradt/trafficserver/blob/ip_allow_categories/example/plugins/c-api/ip_category/ip_category.cc > >>>> > >>>> We hope that this feature can be generally helpful to other users in > >> the > >>>> community who have IP categories that cannot be statically specified > >> in the > >>>> ip_addrs node of ip_allow.yaml. We welcome any thoughts or suggestions > >> for > >>>> improvement concerning the above suggested API. > >>>> > >>>> Thanks, > >>>> Brian > >>>> > >>>> > >>>> -- > >>>> "Come to Me, all who are weary and heavy-laden, and I will > >>>> give you rest. Take My yoke upon you and learn from Me, for > >>>> I am gentle and humble in heart, and you will find rest for > >>>> your souls. For My yoke is easy and My burden is light." > >>>> > >>>> ~ Matthew 11:28-30 > >>>> > >>> > >>> > >>> -- > >>> "Come to Me, all who are weary and heavy-laden, and I will > >>> give you rest. Take My yoke upon you and learn from Me, for > >>> I am gentle and humble in heart, and you will find rest for > >>> your souls. For My yoke is easy and My burden is light." > >>> > >>> ~ Matthew 11:28-30 > >> > > > > > > -- > > "Come to Me, all who are weary and heavy-laden, and I will > > give you rest. Take My yoke upon you and learn from Me, for > > I am gentle and humble in heart, and you will find rest for > > your souls. For My yoke is easy and My burden is light." > > > > ~ Matthew 11:28-30 > -- "Come to Me, all who are weary and heavy-laden, and I will give you rest. Take My yoke upon you and learn from Me, for I am gentle and humble in heart, and you will find rest for your souls. For My yoke is easy and My burden is light." ~ Matthew 11:28-30