Hi @154pinkchairs ,

Thank you for these improvements! Making the mutexes private and the FirewallError type would be really nice to have.

Could you split the PR into smaller ones? "Make mutexes private" has nothing to do with adding a new error type, it'll make things easier to analyze and debug.

On the other hand, could you add only the changes relevant to the PR?
For example returning here prevents monitoring the file system-fw.json for changes:

https://github.com/evilsocket/opensnitch/pull/890/files#diff-c01f644586c786f0bdee74c46336aeb7d402fa28faef803bb48cdad9aa7c9f62L166-R181

Testing it out:
execute the daemon -> see that the error is logged -> modify the file system-fw.json -> notice that it's not reloaded.
then comment out the return;
execute the daemon -> see that the error is logged -> modify the file system-fw.json -> notice that it's reloaded .

Also, the new json tags doesn't correspond with the existent system-fw.json configuration fields, which on the one hand would break users' configurations, and on the other hand prevents from adding system firewall rules.

For example renaming "Version" to "versions" prevents from adding rules:

image

Other examples:
TargetParameters string json:"target_parameters"
Enabled bool json:"enabled" -> making it not capitalize disables GUI's ability to enable/disable it.
SystemRules []*chainsList json:"chains_list" -> error:

image

And here another change not relevant to the PR:
https://github.com/evilsocket/opensnitch/pull/890/files#diff-0c26094505ea9f96de01bd8aeb1abee5455726a6269ddfcceb939434ae6977dfR132

I have updated my PR to adjust to the changes requested.
The error wrapping moved to:
#899
I've also checked out config/config.go to master for this PR and fixed the json tags in #889 as it affects the json schema more.

thank you @154pinkchairs , looks better now!

There's only one minor problem, that's probably why these mutexes were embedded instead of being members of the structs.
There's a leak re-loading the fw configuration (for example when changing the input policy drop <-> allow, go build -race -o opensnitchd .)

WARNING: DATA RACE
Write at 0x00c000cb8b30 by goroutine 80:
(...)
 github.com/evilsocket/opensnitch/daemon/firewall/config.(*Config).loadConfiguration()
 /home/devuan/pr889/daemon/firewall/config/config.go:201 +0x112
 github.com/evilsocket/opensnitch/daemon/firewall/config.(*Config).SaveConfiguration()
 /home/devuan/pr889/daemon/firewall/config/config.go:218 +0x1b3
 github.com/evilsocket/opensnitch/daemon/firewall/nftables.(*Nft).SaveConfiguration()
 <autogenerated>:1 +0x4f
Previous read at 0x00c000cb8b30 by goroutine 59:
 github.com/evilsocket/opensnitch/daemon/firewall/nftables.(*Nft).AddSystemRules()
 /home/devuan/pr889/daemon/firewall/nftables/system.go:92 +0x537
 github.com/evilsocket/opensnitch/daemon/firewall/nftables.(*Nft).reloadConfCallback()
 /home/devuan/pr889/daemon/firewall/nftables/monitor.go:56 +0xd5

I've only found 2 ways of getting rid of it: 1) by embedding the lock as it was before, 2) by creating a new SystemConfig struct:

s := SystemConfig{}
if err := json.Unmarshal(rawConfig, &s); err != nil {
...
}
c.SysConfig = s

But unfortunately, doing it in this way, it does not delete all the rules when changing the policies.
I'll investigate it a litle bit further