From ae866bc4b748b2624210628cb878920b77c26f58 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Tue, 7 Nov 2023 00:44:43 +0000 Subject: [PATCH] fix: validate that NCIDs are well-formed GUIDs Signed-off-by: Evan Baker --- cns/NetworkContainerContract.go | 22 ++++++++++++++++++++++ cns/networkcontainers/networkcontainers.go | 6 +----- cns/restserver/api.go | 11 ++++++++--- cns/restserver/util.go | 9 ++++----- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index b84f176f637..69aae41d2aa 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" + "github.com/google/uuid" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" ) @@ -90,6 +91,8 @@ const ( MultiTenantCRD = "MultiTenantCRD" ) +var ErrInvalidNCID = errors.New("invalid NetworkContainerID") + // CreateNetworkContainerRequest specifies request to create a network container or network isolation boundary. type CreateNetworkContainerRequest struct { HostPrimaryIP string @@ -112,6 +115,16 @@ type CreateNetworkContainerRequest struct { NetworkInterfaceInfo NetworkInterfaceInfo //nolint // introducing new field for backendnic, to be used later by cni code } +func (req *CreateNetworkContainerRequest) Validate() error { + if req.NetworkContainerid == "" { + return errors.Wrap(ErrInvalidNCID, "NetworkContainerID is empty") + } + if _, err := uuid.Parse(req.NetworkContainerid); err != nil { + return errors.Wrapf(ErrInvalidNCID, "NetworkContainerID %s is not a valid UUID: %s", req.NetworkContainerid, err.Error()) + } + return nil +} + // CreateNetworkContainerRequest implements fmt.Stringer for logging func (req *CreateNetworkContainerRequest) String() string { return fmt.Sprintf("CreateNetworkContainerRequest"+ @@ -404,6 +417,15 @@ type PostNetworkContainersRequest struct { CreateNetworkContainerRequests []CreateNetworkContainerRequest } +func (req *PostNetworkContainersRequest) Validate() error { + for i := range req.CreateNetworkContainerRequests { + if err := req.CreateNetworkContainerRequests[i].Validate(); err != nil { + return err + } + } + return nil +} + // PostNetworkContainersResponse specifies response of creating all NCs that are sent from DNC. type PostNetworkContainersResponse struct { Response Response diff --git a/cns/networkcontainers/networkcontainers.go b/cns/networkcontainers/networkcontainers.go index 6d78bef4608..19dc99f34c0 100644 --- a/cns/networkcontainers/networkcontainers.go +++ b/cns/networkcontainers/networkcontainers.go @@ -91,11 +91,7 @@ func (cn *NetworkContainers) Delete(networkContainerID string) error { } // CreateLoopbackAdapter creates a loopback adapter with the specified settings -func CreateLoopbackAdapter( - adapterName string, - ipConfig cns.IPConfiguration, - setWeakHostOnInterface bool, - primaryInterfaceIdentifier string) error { +func CreateLoopbackAdapter(adapterName string, ipConfig cns.IPConfiguration, setWeakHostOnInterface bool, primaryInterfaceIdentifier string) error { return createOrUpdateWithOperation( adapterName, ipConfig, diff --git a/cns/restserver/api.go b/cns/restserver/api.go index 9c84d0b9ea9..540edcddd79 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -786,14 +786,19 @@ func (service *HTTPRestService) createOrUpdateNetworkContainer(w http.ResponseWr logger.Printf("[Azure CNS] createOrUpdateNetworkContainer") var req cns.CreateNetworkContainerRequest - err := service.Listener.Decode(w, r, &req) - logger.Request(service.Name, req.String(), err) - if err != nil { + if err := service.Listener.Decode(w, r, &req); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + if err := req.Validate(); err != nil { + w.WriteHeader(http.StatusBadRequest) return } + logger.Request(service.Name, req.String(), nil) var returnCode types.ResponseCode var returnMessage string + var err error switch r.Method { case http.MethodPost: if req.NetworkContainerType == cns.WebApps { diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 14510ff7627..bef1e1e197a 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -684,9 +684,7 @@ func (service *HTTPRestService) getNetPluginDetails() *networkcontainers.NetPlug func (service *HTTPRestService) getNetworkContainerDetails(networkContainerID string) (containerstatus, bool) { service.RLock() defer service.RUnlock() - containerDetails, containerExists := service.state.ContainerStatus[networkContainerID] - return containerDetails, containerExists } @@ -702,9 +700,7 @@ func (service *HTTPRestService) areNCsPresent() bool { func (service *HTTPRestService) isNetworkJoined(networkID string) bool { namedLock.LockAcquire(stateJoinedNetworks) defer namedLock.LockRelease(stateJoinedNetworks) - _, exists := service.state.joinedNetworks[networkID] - return exists } @@ -712,7 +708,6 @@ func (service *HTTPRestService) isNetworkJoined(networkID string) bool { func (service *HTTPRestService) setNetworkStateJoined(networkID string) { namedLock.LockAcquire(stateJoinedNetworks) defer namedLock.LockRelease(stateJoinedNetworks) - service.state.joinedNetworks[networkID] = struct{}{} } @@ -955,6 +950,10 @@ func (service *HTTPRestService) handlePostNetworkContainers(w http.ResponseWrite logger.Response(service.Name, response, response.Response.ReturnCode, err) return } + if err := req.Validate(); err != nil { //nolint:govet // shadow okay + w.WriteHeader(http.StatusBadRequest) + return + } createNCsResp := service.createNetworkContainers(req.CreateNetworkContainerRequests) response := cns.PostNetworkContainersResponse{