Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support CIDR notation in IP Check rules #349

Closed
ilikepi opened this issue Mar 1, 2024 · 3 comments
Closed

Support CIDR notation in IP Check rules #349

ilikepi opened this issue Mar 1, 2024 · 3 comments

Comments

@ilikepi
Copy link

ilikepi commented Mar 1, 2024

It would be useful if IP addresses could be specified with CIDR notation as an alternate to the format currently used with IP Check rules. This would make it easier to build rules from large sets of IP ranges exported out of other network management systems.

For example:

{
    "Version": "1.0.0",
    "Dependency": {
        "wordpress": ">=6.4.2",
        "advanced-access-manager": ">=6.9.20"
    },
    "Statement": [
        {
            "Effect": "allow",
            "Resource": [
                "URI:*"
            ]
        }
    ],
    "Param": [
        {
            "Key": "ipcheck:website:rules",
            "Value": [
                {
                    "Effect": "deny",
                    "Type": "ip",
                    "Criteria": "0.0.0.0/0"
                },
                {
                    "Effect": "allow",
                    "Type": "ip",
                    "Criteria": "10.128.100.0/21"
                },
                {
                    "Effect": "allow",
                    "Type": "ip",
                    "Criteria": "172.2.1.197/32"
                }
            ]
        }
    ]
}

Interestingly, we tested this out to see if it was supported, and after saving the rule, the forward slash was escaped with a backslash.

@aamplugin
Copy link
Owner

Hello @ilikepi,

Great idea. We will consider adding it in the next AAM release.

In regards of forward slashes - this is actually an intended behavior, those I can see how it can be confusing. We'll also consider normalizing policies before displaying to avoid escaping forward and backward slashes.

Thank you for your message.

@aamplugin aamplugin self-assigned this Mar 2, 2024
@aamplugin aamplugin added this to the AAM - 6.9.24 milestone Mar 2, 2024
@aamplugin
Copy link
Owner

@ilikepi,

Quick update. We found your idea very good and already implemented all the necessary changes to both AAM basic version and our premium add-on. We will document and unittest changes, however, essentially this is how it all works.

The (*ip) typecast how gets some special treatment if there is a CIDR specified in the IP address. For example, the following policy denies access to all posts if user is coming from IP address between 10.0.0.0 - 10.0.0.255:

{
    "Statement": [
        {
            "Effect": "deny",
            "Resource": "PostType:post:posts",
            "Action": [
                "Read"
            ],
            "Condition": {
                "In": {
                    "${USER.ip}": "(*ip)10.0.0.0/24"
                }
            }
        }
    ]
}

The policy that you've mentioned earlier, I would recommend to rewrite to something like this after we release the next AAM version in a week or so (but we can give you the candidate release much sooner if you'd like to run some tests):

{
    "Statement": [
        {
            "Effect": "deny",
            "Resource": "URI:*",
            "Condition": {
                "NotIn": {
                    "${USER.ip}": [
                        "(*ip)10.128.100.0/21",
                        "(*ip)172.2.1.197/32"
                    ]
                }
            }
        }
    ]
}

The above policy denies access to all URLs unless user is coming from two stated IP schemas.

@aamplugin aamplugin added this to Ready For Release in Advanced Access Manager Mar 29, 2024
@ilikepi
Copy link
Author

ilikepi commented Mar 29, 2024

Thank you so much! 💛

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Advanced Access Manager
  
Ready For Release
Development

No branches or pull requests

2 participants