romana / core

Romana core components - Micro services written in Go.
Apache License 2.0
47 stars 11 forks source link

Add sample security policy to repo. #68

Closed pritesh closed 8 years ago

flashvoid commented 8 years ago

Now my few cents here.

Why do we allow port ranges with type ICMP? Why do we allow stateful flag with egress policies ?

I do realize this could be optional, but my point is in semantics design.

Say you are reading a policy and you see it's an Egress policy, which is ok, keep reading. Then you see that it's a stateful policy as well, which is also ok unless you remember that in another block it was defined as Egress. When syntax makes you do such retrospection in your head it's not very nice.

Now I do realize that making a good syntax is a damn hard thing - it's one of the hardest tasks humanity has, but maybe keeping it simple could help.

jbrendel commented 8 years ago

Stas, I agree with the "keep it simple". I also agree with "why ports in ICMP" (or ICMP type in TCP). Don't think it should be there. Previous comments mentioned this, but they keep disappearing for some reason.

Not sure why stateful cannot be applied to egress filters, though? If I allow an outgoing SYN packet, I should allow the incoming SYN+ACK: Stateful. What am I missing?

The idea is that we have just four top level elements to a policy: "AppliedTo", "Peers", "Direction", "Rules". That's pretty straight forward and simple, I would think?

pritesh commented 8 years ago

@flashvoid

Why do we allow port ranges with type ICMP? Why do we allow stateful flag with egress policies ? I do realize this could be optional, but my point is in semantics design.

This is combined policy definition, what comes in as POST and what does out as GET or other verbs is matter of implementation and thus those features are marked as optional.

Say you are reading a policy and you see it's an Egress policy, which is ok, keep reading. Then you see that it's a stateful policy as well, which is also ok unless you remember that in another block it was defined as Egress. When syntax makes you do such retrospection in your head it's not very nice.

The assumption you are making here is that user is going to have "Egress" policy which is also marked "stateful", from implementation point of view, you can always throw an error, but those are still implementation details, technically it is still is part of policy definition.

jbrendel commented 8 years ago

Simple. For example:

{
    "AppliedTo" : [ {
        "Tenant" : "Tenant1",
        "Segment" : "FrontEnd"
    } ],
    "Peers" : [ { "CidrBlock" : "0.0.0.0/0" } ],
    "Direction" : "ingress",
    "Rules" : [
        {
            "Protocol" : "TCP",
            "DstPortRange" : [ 80, 80 ],
        }
    ]
}

That's pretty simple, is it not?

flashvoid commented 8 years ago

@pritesh no argument here, you can express a lot with such definition, the bad thing is that you can express a lot of impossible stuff. Then on the processing side, you will have to make a decision on which one of 2 components is more relevant and you will have to rely on the 3rd component to make a decision. This can be done but amount of exceptions will grow.

pritesh commented 8 years ago

@flashvoid agreed, then what is the alternative you suggest?

jbrendel commented 8 years ago

Hold on a moment. I think we are quite close.

"Direction" tells you on which FORWARD chain to put the rules. "AppliedTo" tells you on which hosts to put it. "Peers" tells you all about the location-related traffic selectors. "Rules" tells you all about the protocol specific ones.

For each "Peer" you can have a test+jump into a chain on which you implement the rules.

The rules are either TCP, UDP or ICMP at this point. It's easy from there.

pritesh commented 8 years ago

@jbrendel agreed, we are close. so if we finalize the port range in rules then we are golden.

flashvoid commented 8 years ago

@jbrendel ahh ok, that depends on how you look at it. So in your example it's valid but then stateful is used to create implicit ingress rules in response to egress traffic. What the use of stateful in ingress then? If it is to allow return traffic to the clients for whom we have explicit ingress rules then what should the default be like ? We certainly expect implicit stateful when we create ingress rules, but that might not be the case for egress.

jbrendel commented 8 years ago

I see "stateful" as creating matching "return traffic" rules. SYN and SYN+ACK. I don't see it as: Reach out to the Peer hosts and start to establish rules there.

Rule for "stateful" in ingress is the same.

The question is whether 'stateLESS' would ever make sense for TCP. We might choose to default to stateful for TCP? Don't know, yet.

flashvoid commented 8 years ago

@pritesh for starters each of your top level components has many to many relationships to other components. So there should be no conflicts between top level components in any configuration.

But that's only a first guess - as I said it's a hard thing and I might not have an answer - just saying we might want to spend more time on it.

jbrendel commented 8 years ago

@flashvoid: Please explain your concern about the top-level components having many to many relationships. Example?