Skip to content

Add remote candidate IP filtering to ICE Agent#898

Open
sirzooro wants to merge 1 commit intopion:masterfrom
sirzooro:remote_ip_filter
Open

Add remote candidate IP filtering to ICE Agent#898
sirzooro wants to merge 1 commit intopion:masterfrom
sirzooro:remote_ip_filter

Conversation

@sirzooro
Copy link
Contributor

Summary

This PR adds support for filtering remote candidate IP addresses before they are accepted by the agent.

A new RemoteIPFilter callback can now be configured via both AgentConfig and WithRemoteIPFilter(...). The filter is applied consistently when:

  • remote candidates are added through AddRemoteCandidate / internal addRemoteCandidate
  • peer-reflexive candidates are discovered from inbound STUN Binding requests

If a remote candidate is rejected by policy (or its address cannot be parsed), it is ignored and not added.

Motivation

IPFilter controls local candidate gathering, but there was no symmetric policy control for incoming remote candidates. This change enables allowlist/blocklist style enforcement for remote candidate addresses.

What changed

  • agent_config.go

    • Added RemoteIPFilter func(net.IP) (keep bool) to AgentConfig.
  • agent_options.go

    • Added WithRemoteIPFilter(filter func(net.IP) bool) AgentOption.
  • agent.go

    • Added remoteIPFilter field to Agent.
    • Wired AgentConfig.RemoteIPFilter into agent construction.
    • Added shouldAcceptRemoteCandidate(cand Candidate) bool.
    • Applied filtering before adding remote candidates in addRemoteCandidate.
    • Applied filtering to newly created peer-reflexive candidates in inbound request handling.
    • Added warning logs when candidate address parsing fails or candidate is filtered out.
  • Tests

    • agent_options_test.go
      • Added TestWithRemoteIPFilterOption.
    • agent_test.go
      • Added coverage for filtering in direct remote candidate addition.
      • Added coverage for filtering in AddRemoteCandidate asynchronous path.
      • Added inbound Binding request case verifying blocked prflx candidates are discarded.
      • Included WithRemoteIPFilter in non-updatable options test matrix.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a configurable remote-candidate IP filtering hook to the ICE Agent, allowing applications to accept/ignore remote candidates (including newly discovered peer-reflexive candidates) based on their IP address.

Changes:

  • Added RemoteIPFilter to AgentConfig and WithRemoteIPFilter(...) to AgentOption.
  • Applied remote IP filtering when adding remote candidates and when creating peer-reflexive candidates from inbound STUN Binding requests.
  • Added tests covering both the synchronous/internal and async AddRemoteCandidate paths, plus the inbound prflx discovery path.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
agent_config.go Introduces AgentConfig.RemoteIPFilter for remote candidate admission policy.
agent_options.go Adds WithRemoteIPFilter option to configure the filter via options API.
agent.go Wires config/option into Agent, and enforces filtering for remote + prflx candidates.
agent_options_test.go Adds unit test ensuring the option sets agent.remoteIPFilter.
agent_test.go Adds behavioral tests ensuring remote filtering is enforced across code paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sirzooro sirzooro requested a review from JoTurk March 14, 2026 07:11
@codecov
Copy link

codecov bot commented Mar 14, 2026

Codecov Report

❌ Patch coverage is 83.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.45%. Comparing base (3cabadd) to head (9ec45ae).

Files with missing lines Patch % Lines
agent.go 77.77% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #898      +/-   ##
==========================================
- Coverage   88.46%   88.45%   -0.02%     
==========================================
  Files          45       45              
  Lines        5633     5654      +21     
==========================================
+ Hits         4983     5001      +18     
- Misses        447      449       +2     
- Partials      203      204       +1     
Flag Coverage Δ
go 88.45% <83.33%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JoTurk
Copy link
Member

JoTurk commented Mar 15, 2026

@sirzooro why is this need? i guess to filter out specific prflx candidates (applications can already filter other candidates), but do you have a specific real world usage in mind?

@sirzooro
Copy link
Contributor Author

@sirzooro why is this need? i guess to filter out specific prflx candidates (applications can already filter other candidates), but do you have a specific real world usage in mind?

For security purposes. Remote peer may send any address as a candidate, including addresses from local peer's internal network. I am not sure if this may be useful for some kind of attacks or not, so I want to block local IPs just in case.

@JoTurk
Copy link
Member

JoTurk commented Mar 15, 2026

@sirzooro but applications can already filter remote candidates at their end, and ice users (including pion/webrtc) can filter remote candidates too. I thought this was about prflx candidates.

@sirzooro
Copy link
Contributor Author

@JoTurk right, although app would have to parse received candidates, and resolve mDNS ones. And of course prflx ones cannot be filtered there. New filter is also easier to use as it gets IP address as a parameter.

@JoTurk
Copy link
Member

JoTurk commented Mar 18, 2026

Makes sense, I'll review the api and the change tonight, thank you.

Copy link
Member

@JoTurk JoTurk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, just one comment.

agent.go Outdated
Comment on lines 1612 to 1615
if !a.shouldAcceptRemoteCandidate(prflxCandidate) {
return nil, false
}
remoteCandidate = prflxCandidate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make it so we call shouldAcceptRemoteCandidate once, even if we have to refactor this code? I'm worried that we might cause a weird behavior if we call the filter twice, e.g: if the user filter is not-idempotent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Fixed this and added test for it.

Add support for filtering remote candidate IP addresses before they are
accepted by the agent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants