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

Keep all ports that we connect to #348

Merged
merged 9 commits into from
Dec 6, 2024
Merged

Conversation

foca
Copy link
Contributor

@foca foca commented Aug 29, 2024

Currently, when connecting to multiple ports in the same hostname, the firewall only tracks the first port connected to, and discards the rest. This fixes the code so that we track all ports connected to and report them to the server.

@foca foca requested review from hansott and timokoessler August 29, 2024 09:56
When tracking outbound connections, this makes sure that connections to
multiple ports in the same hostname are correctly tracked, rather than
ignored.
@foca foca force-pushed the dont-shadow-ports-in-hostnames branch from 49ae329 to 2d107d5 Compare August 29, 2024 10:05
Copy link

codecov bot commented Aug 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@timokoessler
Copy link
Contributor

timokoessler commented Sep 2, 2024

PR looks good to me. 👍
Maybe this approach would be slightly more efficient:

export class Hostnames {
   private map = new Map<string, Ports>();
 
   constructor(private readonly maxEntries: number = 200) {}
 
   add(hostname: string, port: number | undefined) {
     if (!this.map.has(hostname)) {
-      this.map.set(hostname, new Set());
+      this.map.set(hostname, new Set([port]));
+    } else {
+      this.map.get(hostname)?.add(port);
     }
-    (this.map.get(hostname) as Ports).add(port);
 

…w-ports-in-hostnames

* 'main' of github.com:AikidoSec/node-RASP: (484 commits)
  Fix linting
  Add unit tests for hono/context-storage
  Fix addHonoMiddleware types
  Upgrade mongodb in sample app
  Add comment about mongodb v6.10.0
  Keep same order
  Use process.nextTick before wrapping Collection
  Fix lock files
  Add links to docs
  Update end2end/tests/node-red.test.js
  Update end2end/tests/node-red.test.js
  Add node-red e2e test
  Fix property definition
  fix: Do not use wrap helper for req handler
  Improve package.json
  Fix test
  Fix lint
  Fix lint
  Fix type import
  Test express v4 and v5
  ...
add(hostname: string, port: number | undefined) {
if (this.map.has(hostname)) {
return;
private portKey(port: number | undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused?

@hansott hansott merged commit 7c3c217 into main Dec 6, 2024
11 checks passed
@hansott hansott deleted the dont-shadow-ports-in-hostnames branch December 6, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants