[ASM] IP blocking scenario #525
Conversation
|
@cbeauchesne: do you have any idea how could we try that the IP blocking changes are effective? We were discussing it with Nicolas and thinking of trying N times a request until it gets blocked, with a timeout to fail if it never happens. |
nizox
requested review from
Julio-Guerra,
robertpi,
ValentinZakharov and
cataphract
nizox
force-pushed
the
nicolas.vivet/appsec-ip-blocking
branch
from
f117629
to
1d7ae87
Compare
|
Java library 0.111.0 is released and works with this test, so we can enable it in CI. |
.github/workflows/ci.yml
Outdated
| @@ -529,6 +529,12 @@ jobs: | |||
| env: | |||
| DD_API_KEY: ${{ secrets.DD_API_KEY }} | |||
|
|
|||
| - name: Run APPSEC_IP_BLOCKING scenario | |||
| if: ${{ matrix.library != 'java' || matrix.weblog-variant == 'spring-boot' }} | |||
There was a problem hiding this comment.
why not running it on other weblogs, just skipping the failing test ?
There was a problem hiding this comment.
There is currently a manual flag required on the java cli to enable remote config. I only enabled it on spring-boot. As the next version of the tracer will remove this flag, we can wait the release and update this PR or open another PR later. WDYT?
There was a problem hiding this comment.
Given the current status in Java, I think it would be more convenient for us to:
- Change this PR to add CLI to enable remote config to all relevant weblog variants, and remove this condition here.
- We'll open a separate PR to remove the enable remote config CLI parameter from all Java weblog variants soon.
But I don't want to block this PR. If it's ready in its current form, then I'll go the other way around and remove this condition later.
There was a problem hiding this comment.
I would also prefer to add @missing_feature(context.library == "java" and context.weblog_variant != "spring-boot") on the corresponding test, it'll be easier for me to handle (I'll need to add this scenario in nightly runs)
Edit : see below he good line for this.
|
Ok, so as said in my comment, there is probably a race condition. Solution 1 : is there an explicit signal from the tracer that says the config is applied and ready to be used ? |
|
I've added a better "wait for", @nizox could you check that the check logic for this second wait for is coherent ? I've also added in the normal scenario something that test there is no error in RC response |
| def remote_config_is_applied(data): | ||
| if data["path"] == "/v0.7/config" and self.remote_config_is_sent: | ||
| if "config_states" in data.get("request", {}).get("content", {}).get("client", {}).get("state", {}): | ||
| config_states = data["request"]["content"]["client"]["state"]["config_states"] | ||
|
|
||
| for state in config_states: | ||
| if state["id"] == "ASM_DATA-base": | ||
| return True | ||
|
|
||
| return False |
There was a problem hiding this comment.
The apply status - it should be present in that payload
|
There are two comment above to resolve . Then it's ok for me |
LGTM |
.github/workflows/ci.yml
Outdated
| @@ -529,6 +529,12 @@ jobs: | |||
| env: | |||
| DD_API_KEY: ${{ secrets.DD_API_KEY }} | |||
|
|
|||
| - name: Run APPSEC_IP_BLOCKING scenario | |||
| if: ${{ matrix.library != 'java' || matrix.weblog-variant == 'spring-boot' }} | |||
There was a problem hiding this comment.
I would also prefer to add @missing_feature(context.library == "java" and context.weblog_variant != "spring-boot") on the corresponding test, it'll be easier for me to handle (I'll need to add this scenario in nightly runs)
Edit : see below he good line for this.
This PR adds a new scenario for ASM IP blocking. An ASM_DATA config is pushed to the library and requests are issued to the webapp to make sure IP addresses are either blocked or allowed.
Tested with .NET: