turn ProtocolInspectPolicy into AclDstHostRuleSet

[GlenDC]

Closes #341

tested it with web sockets on a local g3 proxy, and it seems to work as expected:

websocket_inspect_policy:
      exact:
        default: intercept
        block:
          - "websocketstest.com"

Should work just as well for h2_inspect_policy, smtp_inspect_policy and imap_inspect_policy. But these I haven't tested. Code is same however.

[zh-jq]

I will do a full review after back to work on 10.10.

[GlenDC]

I will do a full review after back to work on 10.10.

Meaning in a month or what timeline can I expect? This is not to demand anything, but more so I can manage my own expectations and reminders. Either answer is fine, it is just to know :)

[zh-jq]

I will do a full review after back to work on 10.10.

Meaning in a month or what timeline can I expect? This is not to demand anything, but more so I can manage my own expectations and reminders. Either answer is fine, it is just to know :)

the day after tomorrow, maybe I should write 10/10 in English? (⊙o⊙)

[GlenDC]

All good. Thank you for clarifying. Now I understand. Thanks!

[zh-jq-b]

The *Bulder struct is used in config/* module to allow Clone/PartialEq. You can avoid Clone here by using the corresponding Builder in g3proxy/src/config/audit/auditor.rs.

[GlenDC]

I don't think I understand your feedback here. Could you try in more detail?

[zh-jq]

Use AclDstHostRuleSetBuilder/ProtocolInspectPolicyBuilder in AuditorConfig, and build AclDstHostRuleSet/ProtocolInspectPolicy as new fields in Auditor.

[GlenDC]

I am correct to assume that you take this one over as well after merge. Or did I misunderstand that?

[zh-jq]

The concern here is that, wr can not say intercept is more restrict than detour. A different build approach may be needed for AclAction and ProtocolInspectAction. You can squash the commits into smaller ones, and I will update the build logic here after merged.

[GlenDC]

I agree. I'll get to the squashing now so you can take over.

Note that besides your feedback regarding the restrict thing and also the builder clone that you can probably easily fix yourself, for which thank you, there is also the more severe issue, which is the bug I was talking to you about in the issue. Regarding also blocking non-ws traffic once the ws policy is defined. So that's something to look out for as well.

[GlenDC]

Ok @zh-jq @zh-jq-b I cleaned up the history.
It has now just two commits. My initial code and then also one to apply all feedback from you

[GlenDC]

Double checked the code in this PR, looks still fine even after all the rebasing. I also do not see immediately where there could be a bug. Gonna double check running this code in a demo env and see if I still have the same issue, because as far as I can see the code maps 1 to 1 with the old logic, with the only difference being that we support acl-like sets now. Assuming the check methods are correct.

[GlenDC]

Great news @zh-jq I pulled this branch into a demo env and all seems to work. E.g.

websocket_inspect_policy:
    child:
    default: intercept
    block:
        - websocketstest.com

Blocks that host, but all others are fine. Seems that I had a mistake in my demo env due to a wrong merge at some point.
With a clean slate and this branch merged into there all is good.

So seems no buggy code after all. So I think you can merge this code with just those two small remarks that are probably easier for you to get right as I have a feeling you have a specific solution in mind that is probably faster to just do yourself than communicate to me.

But if you think it is useful for me to do it do tell me with more explanation and detail and I will do my best to contribute it as well. All good for me.