Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feature/systemd query events #1728
Feature/systemd query events #1728
Changes from all commits
9d26cd2
811a3d9
194eac2
b3b1615
8345c17
a873a01
bf6d8b5
1a6237b
f189adc
6137358
5019a12
7f2b8fc
1b4f5f1
dff2f34
eda62f4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Missing Resolver() implementation in spn/instance.go
The verification reveals that
spn/instance.go
implementsNetEnv()
but lacks the newly addedResolver()
method. This implementation needs to be updated to maintain interface compatibility.spn/instance.go
: Add the missingResolver()
method to match the interface requirementsservice/instance.go
and test files are correctly updated🔗 Analysis chain
Verify interface implementation across the codebase.
The addition of
Resolver()
method to the interface looks good, but we should verify that all types implementing this interface have been updated.The previous attempt might have failed due to complex AST pattern. Let's try using ripgrep to:
The previous output shows we have potentially found an issue - the
spn/instance.go
implementation hasNetEnv()
but might be missingResolver()
. Let's verify the complete interface definition and check the full implementation in spn/instance.go to confirm this.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 95
Script:
Length of output: 695
Script:
Length of output: 649
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can allow any normal DNS, as we can see the packets. We only need to prevent encrypted DNS connections from apps.
This is getting complicated. Maybe we can simplify and regroup the logic here? Let's talk about options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
conn.Inspecting is currently unused. Are you using it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes Im using it here:
https://github.com/safing/portmaster/pull/1728/files#diff-e0558e79bafb1ddeb52febdec98e2d2ddfd0dedb243d8533e77ff7b365b6a45cR485
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security: Enhance DNS inspection mechanism
The current implementation allows packets before inspection completes, which could create a small window of opportunity for attacks.
Consider these improvements:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent method calls after session is destroyed
The methods
StartTrace
(lines 57-59~) andStopTrace
(lines 81-82~) do not check if the session has been destroyed before proceeding. Calling these methods afterDestroySession
could result in undefined behavior or errors due to the invalidl.state
.Add checks to ensure the session is still running before executing these methods.
Apply the following changes to add safety checks:
Also applies to: 81-82