Skip to content

Commit a33e357

Browse files
QxBytesQxBytes
authored
test: validate inputs to acn components (#2845)
* add validation for cni args and cns request values * address feedback * simplify regex match
1 parent f7b6e8b commit a33e357

File tree

4 files changed

+160
-1
lines changed

4 files changed

+160
-1
lines changed

‎cni/network/network.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"net"
1111
"os"
12+
"regexp"
1213
"strconv"
1314
"time"
1415

@@ -35,6 +36,9 @@ import (
3536
"go.uber.org/zap"
3637
)
3738

39+
// matches if the string fully consists of zero or more alphanumeric, dots, dashes, parentheses, spaces, or underscores
40+
var allowedInput = regexp.MustCompile(`^[a-zA-Z0-9._\-\(\) ]*$`)
41+
3842
const (
3943
dockerNetworkOption = "com.docker.network.generic"
4044
OpModeTransparent = "transparent"
@@ -408,6 +412,11 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {
408412
return err
409413
}
410414

415+
if argErr := plugin.validateArgs(args, nwCfg); argErr != nil {
416+
err = argErr
417+
return err
418+
}
419+
411420
iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
412421
plugin.setCNIReportDetails(nwCfg, CNI_ADD, "")
413422

@@ -933,6 +942,11 @@ func (plugin *NetPlugin) Get(args *cniSkel.CmdArgs) error {
933942

934943
logger.Info("Read network configuration", zap.Any("config", nwCfg))
935944

945+
if argErr := plugin.validateArgs(args, nwCfg); argErr != nil {
946+
err = argErr
947+
return err
948+
}
949+
936950
iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
937951

938952
// Initialize values from network config.
@@ -1015,6 +1029,11 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
10151029
return err
10161030
}
10171031

1032+
if argErr := plugin.validateArgs(args, nwCfg); argErr != nil {
1033+
err = argErr
1034+
return err
1035+
}
1036+
10181037
// Parse Pod arguments.
10191038
if k8sPodName, k8sNamespace, err = plugin.getPodInfo(args.Args); err != nil {
10201039
logger.Error("Failed to get POD info", zap.Error(err))
@@ -1206,6 +1225,11 @@ func (plugin *NetPlugin) Update(args *cniSkel.CmdArgs) error {
12061225
return err
12071226
}
12081227

1228+
if argErr := plugin.validateArgs(args, nwCfg); argErr != nil {
1229+
err = argErr
1230+
return err
1231+
}
1232+
12091233
logger.Info("Read network configuration", zap.Any("config", nwCfg))
12101234

12111235
iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
@@ -1468,3 +1492,14 @@ func convertCniResultToInterfaceInfo(result *cniTypesCurr.Result) network.Interf
14681492

14691493
return interfaceInfo
14701494
}
1495+
1496+
func (plugin *NetPlugin) validateArgs(args *cniSkel.CmdArgs, nwCfg *cni.NetworkConfig) error {
1497+
if !allowedInput.MatchString(args.ContainerID) || !allowedInput.MatchString(args.IfName) {
1498+
return errors.New("invalid args value")
1499+
}
1500+
if !allowedInput.MatchString(nwCfg.Bridge) {
1501+
return errors.New("invalid network config value")
1502+
}
1503+
1504+
return nil
1505+
}

‎cni/network/network_test.go‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,3 +1484,73 @@ func TestPluginSwiftV2MultipleAddDelete(t *testing.T) {
14841484
})
14851485
}
14861486
}
1487+
1488+
func TestValidateArgs(t *testing.T) {
1489+
p, _ := cni.NewPlugin("name", "0.3.0")
1490+
plugin := &NetPlugin{
1491+
Plugin: p,
1492+
}
1493+
1494+
tests := []struct {
1495+
name string
1496+
args *cniSkel.CmdArgs
1497+
nwCfg *cni.NetworkConfig
1498+
wantErr bool
1499+
}{
1500+
{
1501+
name: "Args",
1502+
args: &cniSkel.CmdArgs{
1503+
ContainerID: "5419067fa51b3b942bdd1af1ae78ea5f9cabc67ae71c7b5ef57ba8ca1b2386ec",
1504+
IfName: "eth0",
1505+
},
1506+
nwCfg: &cni.NetworkConfig{
1507+
Bridge: "azure0",
1508+
},
1509+
wantErr: false,
1510+
},
1511+
{
1512+
name: "Args with spaces and special characters",
1513+
args: &cniSkel.CmdArgs{
1514+
ContainerID: "test2-container",
1515+
IfName: "vEthernet (Ethernet 2)",
1516+
},
1517+
nwCfg: &cni.NetworkConfig{
1518+
Bridge: ".-_",
1519+
},
1520+
wantErr: false,
1521+
},
1522+
{
1523+
name: "Empty args",
1524+
args: &cniSkel.CmdArgs{
1525+
ContainerID: "",
1526+
IfName: "",
1527+
},
1528+
nwCfg: &cni.NetworkConfig{
1529+
Bridge: "",
1530+
},
1531+
wantErr: false,
1532+
},
1533+
{
1534+
name: "Invalid args",
1535+
args: &cniSkel.CmdArgs{
1536+
ContainerID: "",
1537+
IfName: "",
1538+
},
1539+
nwCfg: &cni.NetworkConfig{
1540+
Bridge: "\\value/\"",
1541+
},
1542+
wantErr: true,
1543+
},
1544+
}
1545+
for _, tt := range tests {
1546+
tt := tt
1547+
t.Run(tt.name, func(t *testing.T) {
1548+
err := plugin.validateArgs(tt.args, tt.nwCfg)
1549+
if tt.wantErr {
1550+
require.Error(t, err, "Expected error but did not receive one")
1551+
} else {
1552+
require.NoError(t, err, "Expected no error but received one")
1553+
}
1554+
})
1555+
}
1556+
}

‎cns/NetworkContainerContract.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ const (
102102
)
103103

104104
var ErrInvalidNCID = errors.New("invalid NetworkContainerID")
105+
var ErrInvalidIP = errors.New("invalid IP")
105106

106107
// CreateNetworkContainerRequest specifies request to create a network container or network isolation boundary.
107108
type CreateNetworkContainerRequest struct {
@@ -132,9 +133,24 @@ func (req *CreateNetworkContainerRequest) Validate() error {
132133
if _, err := uuid.Parse(strings.TrimPrefix(req.NetworkContainerid, SwiftPrefix)); err != nil {
133134
return errors.Wrapf(ErrInvalidNCID, "NetworkContainerID %s is not a valid UUID: %s", req.NetworkContainerid, err.Error())
134135
}
136+
if req.PrimaryInterfaceIdentifier != "" && !isValidIP(req.PrimaryInterfaceIdentifier) {
137+
return errors.Wrapf(ErrInvalidIP, "PrimaryInterfaceIdentifier %s is not a valid ip address", req.PrimaryInterfaceIdentifier)
138+
}
139+
if req.IPConfiguration.GatewayIPAddress != "" && !isValidIP(req.IPConfiguration.GatewayIPAddress) {
140+
return errors.Wrapf(ErrInvalidIP, "GatewayIPAddress %s is not a valid ip address", req.IPConfiguration.GatewayIPAddress)
141+
}
135142
return nil
136143
}
137144

145+
func isValidIP(ipStr string) bool {
146+
// if can parse (i.e. not nil), then valid ip
147+
if ip, _, err := net.ParseCIDR(ipStr); err == nil {
148+
return ip != nil
149+
}
150+
ip := net.ParseIP(ipStr)
151+
return ip != nil
152+
}
153+
138154
// CreateNetworkContainerRequest implements fmt.Stringer for logging
139155
func (req *CreateNetworkContainerRequest) String() string {
140156
return fmt.Sprintf("CreateNetworkContainerRequest"+

‎cns/NetworkContainerContract_test.go‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,15 @@ func TestPostNetworkContainersRequest_Validate(t *testing.T) {
157157
NetworkContainerid: "f47ac10b-58cc-0372-8567-0e02b2c3d479",
158158
},
159159
{
160-
NetworkContainerid: "f47ac10b-58cc-0372-8567-0e02b2c3d478",
160+
NetworkContainerid: "f47ac10b-58cc-0372-8567-0e02b2c3d478",
161+
PrimaryInterfaceIdentifier: "10.240.0.4",
162+
IPConfiguration: IPConfiguration{
163+
GatewayIPAddress: "10.0.0.1",
164+
},
165+
},
166+
{
167+
NetworkContainerid: "a47ac10b-58cc-0372-8567-0e02b2c3d478",
168+
PrimaryInterfaceIdentifier: "10.240.0.4/24",
161169
},
162170
},
163171
},
@@ -191,6 +199,36 @@ func TestPostNetworkContainersRequest_Validate(t *testing.T) {
191199
},
192200
wantErr: true,
193201
},
202+
{
203+
name: "invalid",
204+
req: PostNetworkContainersRequest{
205+
CreateNetworkContainerRequests: []CreateNetworkContainerRequest{
206+
{
207+
NetworkContainerid: "f47ac10b-58cc-0372-8567-0e02b2c3d478",
208+
PrimaryInterfaceIdentifier: "10.240.0.4",
209+
IPConfiguration: IPConfiguration{
210+
GatewayIPAddress: "10.0.0.1;",
211+
},
212+
},
213+
},
214+
},
215+
wantErr: true,
216+
},
217+
{
218+
name: "invalid",
219+
req: PostNetworkContainersRequest{
220+
CreateNetworkContainerRequests: []CreateNetworkContainerRequest{
221+
{
222+
NetworkContainerid: "f47ac10b-58cc-0372-8567-0e02b2c3d478",
223+
PrimaryInterfaceIdentifier: "-10.240.0.4",
224+
IPConfiguration: IPConfiguration{
225+
GatewayIPAddress: "10.0.0.1",
226+
},
227+
},
228+
},
229+
},
230+
wantErr: true,
231+
},
194232
}
195233
for _, tt := range tests {
196234
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)