mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
127 stars 41 forks source link

[Task]: Don't restrict an entire /32 when automatically restricting an IP6 network #15123

Closed diox closed 1 week ago

diox commented 3 weeks ago

Description

For extreme cases, we have a scanner action that can be triggered automatically that not only prevents auto-approval of the add-on but also restricts all the associated IP addresses, preventing them from submitting (Delay auto-approval indefinitely and add restrictions to future approvals).

Because IP restrictions are made for an entire network, we have the following code:

IPNetworkUserRestriction.objects.get_or_create(
    network=f'{ip}/32',
    restriction_type=restriction_type,
    defaults=restriction_defaults,
)

This is an ugly shortcut, but it works for IPv4 as /32 will be a network made of that single IP. For IPv6 though, it's doubly wrong:

What netmask we should use is an interesting question - since this is an automated action, perhaps it should just be a /64.

Acceptance Criteria

  ### Milestones/checkpoints
  - [ ] IP restrictions added for IPv6 as part of a scanner action are valid network
  - [ ] IP restrictions added for IPv6 as part of a scanner action have a more appropriate "netmask" (/64)

Checks

┆Issue is synchronized with this Jira Task

diox commented 1 week ago

it should just be a /64 or /128 to be safe, but OTOH most providers will give their users a /56 or /48, so for a restriction to be effective, maybe it should be at least a /56.

Looking at other IP ban implementations out there the consensus seems to be /64 strikes the best balance between effectiveness and potential side effects. This is only for this particular scanner action anyway, and it can be changed by admins if necessary.

ioanarusiczki commented 13 hours ago