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.

Sign up for GitHub

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

Jump to bottom

deny-network rule: panic: interface conversion: interface {} is net.IP, not string #333

Closed
phil294 opened this issue Jan 16, 2021 · 6 comments
Closed

deny-network rule: panic: interface conversion: interface {} is net.IP, not string #333

phil294 opened this issue Jan 16, 2021 · 6 comments

Comments

@phil294
Copy link

phil294 commented Jan 16, 2021

Hello,

Describe the bug
I was just getting to use opensnitch (it's awesome) and tried various rules, when "Status" in the UI somehow started saying "not running" even though the service was active. opensnitchd crashes and is not recoverable via UI.

To Reproduce
All via UI:

  • Stop interception (top right corner)
  • Create a new rule: 
    Action: Deny
[x] Enable
Duration: always
To this IP / Network: 127.0.0.0/8
    (127.0.0.0/8 can be found in the dropdown defaults)
  • Apply, Close
  • /etc/opensnitchd/rules/deny-network-127-0-0-0-8.json now exists as expected
  • In the UI, edit the newly created rule
  • Change the IP of the rule: 
    To this IP / Network: ^127.0.0.0
  • Apply, Close
  • /etc/opensnitchd/rules/deny-network-127-0-0-0-8.json is modified on disk
  • Start interception via UI (top right corner)
  • Fire some network request
  • Service crashes with below error message in log (interface conversion: interface {} is net.IP, not string)
  • Deleting the rule in UI does not help, need to manually remove the /etc config file

Post error logs:
journalctl -u opensnitchd:

Jan 16 19:09:35 pc opensnitchd[10903]: [2021-01-16 18:09:35]  INF  Loading rules from /etc/opensnitchd/rules ...
Jan 16 19:09:40 pc opensnitchd[10903]: panic: interface conversion: interface {} is net.IP, not string
Jan 16 19:09:40 pc opensnitchd[10903]: goroutine 27 [running]:
Jan 16 19:09:40 pc opensnitchd[10903]: github.com/evilsocket/opensnitch/daemon/rule.(*Operator).reCmp(0xc00055b318, 0x55d85bec55a0, 0xc0005e3c20, 0xc0005e3c20)
Jan 16 19:09:40 pc opensnitchd[10903]:         github.com/evilsocket/opensnitch/daemon/rule/operator.go:126 +0x16f
Jan 16 19:09:40 pc opensnitchd[10903]: github.com/evilsocket/opensnitch/daemon/rule.(*Operator).Match(0xc00055b318, 0xc0005fa400, 0xc000558300)
Jan 16 19:09:40 pc opensnitchd[10903]:         github.com/evilsocket/opensnitch/daemon/rule/operator.go:178 +0x31a
Jan 16 19:09:40 pc opensnitchd[10903]: github.com/evilsocket/opensnitch/daemon/rule.(*Rule).Match(...)
Jan 16 19:09:40 pc opensnitchd[10903]:         github.com/evilsocket/opensnitch/daemon/rule/rule.go:65
Jan 16 19:09:40 pc opensnitchd[10903]: github.com/evilsocket/opensnitch/daemon/rule.(*Loader).FindFirstMatch(0xc000566000, 0xc0005fa400, 0x0)
Jan 16 19:09:40 pc opensnitchd[10903]:         github.com/evilsocket/opensnitch/daemon/rule/loader.go:311 +0x125
Jan 16 19:09:40 pc opensnitchd[10903]: main.acceptOrDeny(0xc00159ff18, 0xc0005fa400, 0x0)
Jan 16 19:09:40 pc opensnitchd[10903]:         github.com/evilsocket/opensnitch/daemon/main.go:215 +0xa5
Jan 16 19:09:40 pc opensnitchd[10903]: main.onPacket(0x55d85bf0b200, 0xc000768000, 0x0, 0xc00073c5a0, 0x4000003d3)
Jan 16 19:09:40 pc opensnitchd[10903]:         github.com/evilsocket/opensnitch/daemon/main.go:193 +0x14e
Jan 16 19:09:40 pc opensnitchd[10903]: main.worker(0x0)
Jan 16 19:09:40 pc opensnitchd[10903]:         github.com/evilsocket/opensnitch/daemon/main.go:131 +0xc9
Jan 16 19:09:40 pc opensnitchd[10903]: created by main.setupWorkers
Jan 16 19:09:40 pc opensnitchd[10903]:         github.com/evilsocket/opensnitch/daemon/main.go:143 +0xe5
Jan 16 19:09:40 pc systemd[1]: opensnitchd.service: Main process exited, code=exited, status=2/INVALIDARGUMENT
Jan 16 19:09:40 pc systemd[1]: opensnitchd.service: Failed with result 'exit-code'.

If the daemon doesn't start:

  • Post last 15 lines of the log file /var/log/opensnitchd.log
[2021-01-16 18:30:52]  IMP  Got signal: terminated
[2021-01-16 18:30:57]  WAR  queue stuck, closing by timeout
[2021-01-16 18:30:57]  WAR  Queue.destroy(), nfq_close() not closed: -1
[2021-01-16 18:30:57]  IMP  Start writing logs to %!(EXTRA string=/var/log/opensnitchd.log)
[2021-01-16 18:31:41]  IMP  Ruleset changed due to deny-network-127-0-0-0-8.json, reloading ...
[2021-01-16 18:31:57]  IMP  Ruleset changed due to deny-network-127-0-0-0-8.json, reloading ...
[2021-01-16 18:31:57]  IMP  Ruleset changed due to deny-network-127-0-0-0-8.json, reloading ...
[2021-01-16 18:32:16]  IMP  Added new rule: allow if process.path is '/usr/bin/curl'
[2021-01-16 18:32:20]  IMP  Added new rule: allow if process.path is '/usr/bin/curl'
[2021-01-16 18:32:27]  IMP  Ruleset changed due to deny-network-127-0-0-0-8.json, reloading ...
[2021-01-16 18:32:59]  IMP  Ruleset changed due to deny-network-127-0-0-0-8.json, reloading ...
[2021-01-16 18:33:19]  IMP  Ruleset changed due to deny-network-127-0-0-0-8.json, reloading ...
[2021-01-16 18:33:42]  IMP  Start writing logs to %!(EXTRA string=/var/log/opensnitchd.log)
[2021-01-16 18:35:06]  IMP  Added new rule: allow if process.path is '/usr/bin/nmbd'
[2021-01-16 18:35:09]  IMP  Added new rule: allow if process.path is '/usr/bin/nmbd'

Expected behavior (optional)
The reason for all this is probably (?) that the updated regex IP needs to be written as operator.operand dest.ip but is instead saved as dest.network.

OS (please complete the following information):
@gustavo-iniguez-goya
Copy link
Collaborator

gustavo-iniguez-goya commented Jan 16, 2021 •
edited
Loading

thank you @phil294 , reproduced. I'll fix it soon.

In any case, what were you trying to achieve with that rule? I mean, 127.0.0.0/8 is the same than ^127.0.0.0, no? any IP in the network range will start with 127, so there's no need to use a regexp.

@phil294
Copy link
Author

phil294 commented Jan 16, 2021

thank you @phil294 , reproduced. I'll fix it soon.

In any case, what was you trying to achieve with that rule? I mean, 127.0.0.0/8 is the same than ^127.0.0.0, no? any IP in the network range will start with 127, so there's no need to use a regexp.

Nice, thank you!
The above IP addresses did not really mean anything. What I was trying to achieve was to allow all traffic from a process except for local IPs where I want to be asked. That's what I need regexp for. Dont yet know how to do this:

Allow
IP: *NOT* ^(127\.0\..*|192\.168\..*|::1)$

But I havent looked into it any further. Except for the docs:

However you can use negated chars classes. For example, block all outgoing connections, except those to localhost:

[x] Action: deny
[x] To this destination IP: [^:127.0.0.1:]

Are you sure about that? This regex only matches one single character that is not in 1270:., I think, and the rule would also allow for an IP like 123.456.789.1.

@gustavo-iniguez-goya
Copy link
Collaborator

ah, I see. mm, I think that we don't support this scenario at the moment. There's no way to negate a field like !127.0.0.1, or that you be asked when a certain rule matches a connection.

Maybe you can filter by port, for example if the connections you want to allow usually goes through the ports 80 and 443:

000-allow-my-program

Action: Allow
to this port: ^(80|443)$

then, you'll be prompted to allow/deny any other connection.

Are you sure about that? This regex only matches one single character that is not in 1270:., I think, and the rule would also allow for an IP like 123.456.789.1.

I tested it long time ago and as far as I can remember it worked, but can't tell you for sure now if it works.

@gustavo-iniguez-goya
Copy link
Collaborator

oops, I've realized that there's an error when a regexp is used on the dst ip/network field. I'll fixed that as well.

gustavo-iniguez-goya added a commit that referenced this issue Jan 17, 2021
@gustavo-iniguez-goya
fixed regexp rules exceptions
b467283 
- ui, ruleseditor: added missing operator when using a regular
  expression on the DstIP/Net field.
- daemon, rules: ensure that regular expressions are of type string
  before evaluating them.

reported here: #333
@gustavo-iniguez-goya
Copy link
Collaborator

@phil294 I think both issues are fixed. I don't know if the manjaro package maintainer pulls latest changes from this repo, or if it only packages our releases. So if you can't or don't want to compile it, you'll have to wait to test it.

@gustavo-iniguez-goya
Copy link
Collaborator

I consider this issue fixed. Thank you for reporting it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gustavo-iniguez-goya @phil294