Skip to content

Store iptables when creating network attack #215

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

Merged
cwen0 merged 7 commits into from
Sep 14, 2022
Merged

Store iptables when creating network attack #215

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f4e1f42
store iptables
invalid-email-address Sep 13, 2022
8db22b3
make check
invalid-email-address Sep 13, 2022
8b3c320
Merge branch 'main' into fix-partition
FingerLeader Sep 13, 2022
15a185a
update network test
invalid-email-address Sep 13, 2022
83eccfa
Merge branch 'fix-partition' of https://github.com/FingerLeader/chaos…
invalid-email-address Sep 13, 2022
a0efa92
move NeedApplyTC() at the begin of applying to the middle
invalid-email-address Sep 14, 2022
2d423a5
remove NeedApplyIptables
invalid-email-address Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions pkg/core/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"github.com/chaos-mesh/chaos-mesh/api/v1alpha1"
"github.com/chaos-mesh/chaos-mesh/controllers/podnetworkchaos/netutils"
"github.com/chaos-mesh/chaos-mesh/pkg/chaosdaemon/pb"
"github.com/chaos-mesh/chaos-mesh/pkg/netem"
"github.com/pingcap/errors"
Expand Down Expand Up @@ -523,20 +524,20 @@ func (n *NetworkCommand) NeedApplyTC() bool {
}
}

func (n *NetworkCommand) AdditionalChain(ipset string) ([]*pb.Chain, error) {
func (n *NetworkCommand) AdditionalChain(ipset string, uid string) ([]*pb.Chain, error) {
chains := make([]*pb.Chain, 0, 2)
var toChains, fromChains []*pb.Chain
var err error

if n.Direction == "to" || n.Direction == "both" {
toChains, err = n.getAdditionalChain(ipset, "to")
toChains, err = n.getAdditionalChain(ipset, "to", uid)
if err != nil {
return nil, err
}
}

if n.Direction == "from" || n.Direction == "both" {
fromChains, err = n.getAdditionalChain(ipset, "from")
fromChains, err = n.getAdditionalChain(ipset, "from", uid)
if err != nil {
return nil, err
}
Expand All @@ -548,7 +549,7 @@ func (n *NetworkCommand) AdditionalChain(ipset string) ([]*pb.Chain, error) {
return chains, nil
}

func (n *NetworkCommand) getAdditionalChain(ipset, direction string) ([]*pb.Chain, error) {
func (n *NetworkCommand) getAdditionalChain(ipset, direction string, uid string) ([]*pb.Chain, error) {
var directionStr string
var directionChain pb.Chain_Direction
if direction == "to" {
Expand All @@ -562,9 +563,11 @@ func (n *NetworkCommand) getAdditionalChain(ipset, direction string) ([]*pb.Chai
}

chains := make([]*pb.Chain, 0, 2)
// The `targetLength`s in `netutils.CompressName()` are different because of
// the need to distinguish between the different chains.
if len(n.AcceptTCPFlags) > 0 {
chains = append(chains, &pb.Chain{
Name: fmt.Sprintf("%s/0", directionStr),
Name: fmt.Sprintf("%s/%s", directionStr, netutils.CompressName(uid, 19, "")),
Ipsets: []string{ipset},
Direction: directionChain,
Protocol: n.IPProtocol,
Expand All @@ -575,7 +578,7 @@ func (n *NetworkCommand) getAdditionalChain(ipset, direction string) ([]*pb.Chai

if n.Action == NetworkPartitionAction {
chains = append(chains, &pb.Chain{
Name: fmt.Sprintf("%s/1", directionStr),
Name: fmt.Sprintf("%s/%s", directionStr, netutils.CompressName(uid, 20, "")),
Ipsets: []string{ipset},
Direction: directionChain,
Protocol: n.IPProtocol,
Expand Down
18 changes: 9 additions & 9 deletions pkg/core/network_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestPatitionChain(t *testing.T) {
},
chains: []*pb.Chain{
{
Name: "OUTPUT/1",
Name: "OUTPUT/3c552_e0172bc4fd046_",
Ipsets: []string{"test"},
Direction: pb.Chain_OUTPUT,
Protocol: "tcp",
Expand All @@ -53,7 +53,7 @@ func TestPatitionChain(t *testing.T) {
},
chains: []*pb.Chain{
{
Name: "INPUT/1",
Name: "INPUT/3c552_e0172bc4fd046_",
Ipsets: []string{"test"},
Direction: pb.Chain_INPUT,
Protocol: "tcp",
Expand All @@ -71,14 +71,14 @@ func TestPatitionChain(t *testing.T) {
},
chains: []*pb.Chain{
{
Name: "OUTPUT/1",
Name: "OUTPUT/3c552_e0172bc4fd046_",
Ipsets: []string{"test"},
Direction: pb.Chain_OUTPUT,
Protocol: "tcp",
Target: "DROP",
},
{
Name: "INPUT/1",
Name: "INPUT/3c552_e0172bc4fd046_",
Ipsets: []string{"test"},
Direction: pb.Chain_INPUT,
Protocol: "tcp",
Expand All @@ -97,30 +97,30 @@ func TestPatitionChain(t *testing.T) {
},
chains: []*pb.Chain{
{
Name: "OUTPUT/0",
Name: "OUTPUT/3c552_e0172bc4fd04_",
Ipsets: []string{"test"},
Direction: pb.Chain_OUTPUT,
Protocol: "tcp",
TcpFlags: "SYN,ACK SYN,ACK",
Target: "ACCEPT",
},
{
Name: "OUTPUT/1",
Name: "OUTPUT/3c552_e0172bc4fd046_",
Ipsets: []string{"test"},
Direction: pb.Chain_OUTPUT,
Protocol: "tcp",
Target: "DROP",
},
{
Name: "INPUT/0",
Name: "INPUT/3c552_e0172bc4fd04_",
Ipsets: []string{"test"},
Direction: pb.Chain_INPUT,
Protocol: "tcp",
TcpFlags: "SYN,ACK SYN,ACK",
Target: "ACCEPT",
},
{
Name: "INPUT/1",
Name: "INPUT/3c552_e0172bc4fd046_",
Ipsets: []string{"test"},
Direction: pb.Chain_INPUT,
Protocol: "tcp",
Expand All @@ -130,7 +130,7 @@ func TestPatitionChain(t *testing.T) {
},
}
for _, tc := range testCases {
chains, err := tc.cmd.AdditionalChain("test")
chains, err := tc.cmd.AdditionalChain("test", "3c5528e1-4c32-4f80-983c-913ad7e860e2")
if err != nil {
t.Errorf("failed to partition chain: %v", err)
}
Expand Down
71 changes: 39 additions & 32 deletions pkg/server/chaosd/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,9 @@ func (networkAttack) Attack(options core.AttackConfig, env Environment) (err err
return perrors.WithStack(err)
Copy link
Member

@cwen0 cwen0 Sep 14, 2022

Choose a reason for hiding this comment

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

We can delete NeedApplyIptables() and NeedApplyIPSet directly.

Copy link
Member Author

@FingerLeader FingerLeader Sep 14, 2022

Choose a reason for hiding this comment

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

I think leaving them can make the code logic clearly, and they may be needed to add features later (by the way, NeedApplyIPSet() is working now)

Copy link
Member

@cwen0 cwen0 Sep 14, 2022

Choose a reason for hiding this comment

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

I think the logic will be clearer If we delete them. The logic: Chaos Mesh will reset all network action when you apply the network actions. This logic will be more simple.

Copy link
Member Author

@FingerLeader FingerLeader Sep 14, 2022

Choose a reason for hiding this comment

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

OK, I think it's OK to be the same as chaos mesh, NeedApplyIptables can be deleted, but I find chaosd attack network delay -l 1s -d eth0 will not work without NeedApplyIPSet()

}
}

if attack.NeedApplyTC() {
if err = env.Chaos.applyTC(attack, ipsetName, env.AttackUid); err != nil {
return perrors.WithStack(err)
}
// Because some tcs add filter iptables which will not be stored in the DB, we must re-apply these tcs to add the iptables.
if err = env.Chaos.applyTC(attack, ipsetName, env.AttackUid); err != nil {
return perrors.WithStack(err)
}

case core.NetworkNICDownAction:
Expand Down Expand Up @@ -140,9 +138,11 @@ func (s *Server) applyIptables(attack *core.NetworkCommand, ipset, uid string) e
return perrors.WithStack(err)
}
chains := core.IptablesRuleList(iptables).ToChains()

var newChains []*pb.Chain
// Presently, only partition and delay with `accept-tcp-flags` need to add additional chains
if attack.NeedAdditionalChains() {
newChains, err := attack.AdditionalChain(ipset)
newChains, err = attack.AdditionalChain(ipset, uid)
if err != nil {
return perrors.WithStack(err)
}
Expand All @@ -156,15 +156,17 @@ func (s *Server) applyIptables(attack *core.NetworkCommand, ipset, uid string) e
return perrors.WithStack(err)
}

// TODO: cwen0
//if err := s.iptablesRule.Set(context.Background(), &core.IptablesRule{
// Name: newChain.Name,
// IPSets: strings.Join(newChain.Ipsets, ","),
// Direction: pb.Chain_Direction_name[int32(newChain.Direction)],
// Experiment: uid,
//}); err != nil {
// return perrors.WithStack(err)
//}
for _, newChain := range newChains {
if err := s.iptablesRule.Set(context.Background(), &core.IptablesRule{
Name: newChain.Name,
IPSets: strings.Join(newChain.Ipsets, ","),
Direction: pb.Chain_Direction_name[int32(newChain.Direction)],
Protocol: newChain.Protocol,
Experiment: uid,
}); err != nil {
return perrors.WithStack(err)
}
}

return nil
}
Expand All @@ -180,17 +182,24 @@ func (s *Server) applyTC(attack *core.NetworkCommand, ipset string, uid string)
return perrors.WithStack(err)
}

newTC, err := attack.ToTC(ipset)
if err != nil {
return perrors.WithStack(err)
}
var newTC *pb.Tc
if attack.NeedApplyTC() {
newTC, err = attack.ToTC(ipset)
if err != nil {
return perrors.WithStack(err)
}

tcs = append(tcs, newTC)
tcs = append(tcs, newTC)
}

if _, err := s.svr.SetTcs(context.Background(), &pb.TcsRequest{Tcs: tcs, EnterNS: false}); err != nil {
return perrors.WithStack(err)
}

if !attack.NeedApplyTC() {
return nil
}

tc := &core.TcParameter{
Device: attack.Device,
}
Expand Down Expand Up @@ -380,22 +389,20 @@ func (networkAttack) Recover(exp core.Experiment, env Environment) error {
case core.NetworkPortOccupiedAction:
return env.Chaos.recoverPortOccupied(attack, env.AttackUid)
case core.NetworkDelayAction, core.NetworkLossAction, core.NetworkCorruptAction, core.NetworkDuplicateAction, core.NetworkPartitionAction, core.NetworkBandwidthAction:
if attack.NeedApplyIPSet() {
if err := env.Chaos.recoverIPSet(env.AttackUid); err != nil {
return perrors.WithStack(err)
}
// `chaosdaemon.DeamonServer.SetTcs()` may build new iptables which will not be recorded in DB,
Copy link
Member

@cwen0 cwen0 Sep 14, 2022

Choose a reason for hiding this comment

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

We should update this comment.

Copy link
Member Author

@FingerLeader FingerLeader Sep 14, 2022

Choose a reason for hiding this comment

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

I don't think this comment should be updated, the filter iptables still not be stored in the DB

Copy link
Member

@cwen0 cwen0 Sep 14, 2022

Choose a reason for hiding this comment

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

Yes, but here we recover all because chaos mesh will apply all network actions when we apply a new one.

Copy link
Member Author

@FingerLeader FingerLeader Sep 14, 2022

Choose a reason for hiding this comment

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

I agree

// and network partition is not suppose to build iptables directly, `recoverIptables()` will not
// be called when recovering a partition experiment. To avoid other cross-build situations, all these
// three functions will be called.
if err := env.Chaos.recoverIPSet(env.AttackUid); err != nil {
return perrors.WithStack(err)
}

if attack.NeedApplyIptables() {
if err := env.Chaos.recoverIptables(env.AttackUid); err != nil {
return perrors.WithStack(err)
}
if err := env.Chaos.recoverIptables(env.AttackUid); err != nil {
return perrors.WithStack(err)
}

if attack.NeedApplyTC() {
if err := env.Chaos.recoverTC(env.AttackUid, attack.Device); err != nil {
return perrors.WithStack(err)
}
if err := env.Chaos.recoverTC(env.AttackUid, attack.Device); err != nil {
return perrors.WithStack(err)
}
case core.NetworkNICDownAction:
return env.Chaos.recoverNICDown(attack)
Expand Down