Skip to content

Commit 6b92514

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

File tree

4 files changed

+159
-1
lines changed

4 files changed

+159
-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"
@@ -347,6 +351,11 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {
347351
return err
348352
}
349353

354+
if argErr := plugin.validateArgs(args, nwCfg); argErr != nil {
355+
err = argErr
356+
return err
357+
}
358+
350359
iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
351360
plugin.setCNIReportDetails(nwCfg, CNI_ADD, "")
352361

@@ -871,6 +880,11 @@ func (plugin *NetPlugin) Get(args *cniSkel.CmdArgs) error {
871880

872881
logger.Info("Read network configuration", zap.Any("config", nwCfg))
873882

883+
if argErr := plugin.validateArgs(args, nwCfg); argErr != nil {
884+
err = argErr
885+
return err
886+
}
887+
874888
iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
875889

876890
// Initialize values from network config.
@@ -954,6 +968,11 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
954968
return err
955969
}
956970

971+
if argErr := plugin.validateArgs(args, nwCfg); argErr != nil {
972+
err = argErr
973+
return err
974+
}
975+
957976
// Parse Pod arguments.
958977
if k8sPodName, k8sNamespace, err = plugin.getPodInfo(args.Args); err != nil {
959978
logger.Error("Failed to get POD info", zap.Error(err))
@@ -1135,6 +1154,11 @@ func (plugin *NetPlugin) Update(args *cniSkel.CmdArgs) error {
11351154
return err
11361155
}
11371156

1157+
if argErr := plugin.validateArgs(args, nwCfg); argErr != nil {
1158+
err = argErr
1159+
return err
1160+
}
1161+
11381162
logger.Info("Read network configuration", zap.Any("config", nwCfg))
11391163

11401164
iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
@@ -1396,3 +1420,14 @@ func convertCniResultToInterfaceInfo(result *cniTypesCurr.Result) network.Interf
13961420

13971421
return interfaceInfo
13981422
}
1423+
1424+
func (plugin *NetPlugin) validateArgs(args *cniSkel.CmdArgs, nwCfg *cni.NetworkConfig) error {
1425+
if !allowedInput.MatchString(args.ContainerID) || !allowedInput.MatchString(args.IfName) {
1426+
return errors.New("invalid args value")
1427+
}
1428+
if !allowedInput.MatchString(nwCfg.Bridge) {
1429+
return errors.New("invalid network config value")
1430+
}
1431+
1432+
return nil
1433+
}

‎cni/network/network_test.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,3 +1232,72 @@ func TestPluginSwiftV2Add(t *testing.T) {
12321232
})
12331233
}
12341234
}
1235+
func TestValidateArgs(t *testing.T) {
1236+
p, _ := cni.NewPlugin("name", "0.3.0")
1237+
plugin := &NetPlugin{
1238+
Plugin: p,
1239+
}
1240+
1241+
tests := []struct {
1242+
name string
1243+
args *cniSkel.CmdArgs
1244+
nwCfg *cni.NetworkConfig
1245+
wantErr bool
1246+
}{
1247+
{
1248+
name: "Args",
1249+
args: &cniSkel.CmdArgs{
1250+
ContainerID: "5419067fa51b3b942bdd1af1ae78ea5f9cabc67ae71c7b5ef57ba8ca1b2386ec",
1251+
IfName: "eth0",
1252+
},
1253+
nwCfg: &cni.NetworkConfig{
1254+
Bridge: "azure0",
1255+
},
1256+
wantErr: false,
1257+
},
1258+
{
1259+
name: "Args with spaces and special characters",
1260+
args: &cniSkel.CmdArgs{
1261+
ContainerID: "test2-container",
1262+
IfName: "vEthernet (Ethernet 2)",
1263+
},
1264+
nwCfg: &cni.NetworkConfig{
1265+
Bridge: ".-_",
1266+
},
1267+
wantErr: false,
1268+
},
1269+
{
1270+
name: "Empty args",
1271+
args: &cniSkel.CmdArgs{
1272+
ContainerID: "",
1273+
IfName: "",
1274+
},
1275+
nwCfg: &cni.NetworkConfig{
1276+
Bridge: "",
1277+
},
1278+
wantErr: false,
1279+
},
1280+
{
1281+
name: "Invalid args",
1282+
args: &cniSkel.CmdArgs{
1283+
ContainerID: "",
1284+
IfName: "",
1285+
},
1286+
nwCfg: &cni.NetworkConfig{
1287+
Bridge: "\\value/\"",
1288+
},
1289+
wantErr: true,
1290+
},
1291+
}
1292+
for _, tt := range tests {
1293+
tt := tt
1294+
t.Run(tt.name, func(t *testing.T) {
1295+
err := plugin.validateArgs(tt.args, tt.nwCfg)
1296+
if tt.wantErr {
1297+
require.Error(t, err, "Expected error but did not receive one")
1298+
} else {
1299+
require.NoError(t, err, "Expected no error but received one")
1300+
}
1301+
})
1302+
}
1303+
}

‎cns/NetworkContainerContract.go‎

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

103103
var ErrInvalidNCID = errors.New("invalid NetworkContainerID")
104+
var ErrInvalidIP = errors.New("invalid IP")
104105

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

144+
func isValidIP(ipStr string) bool {
145+
// if can parse (i.e. not nil), then valid ip
146+
if ip, _, err := net.ParseCIDR(ipStr); err == nil {
147+
return ip != nil
148+
}
149+
ip := net.ParseIP(ipStr)
150+
return ip != nil
151+
}
152+
137153
// CreateNetworkContainerRequest implements fmt.Stringer for logging
138154
func (req *CreateNetworkContainerRequest) String() string {
139155
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)