From 59dfcfd0c352b53ee8d0e0d12b4cffe76e41b822 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 9 Jan 2025 13:19:59 +0100 Subject: [PATCH 1/5] consider a valid ipv4 address as a validish ipv4 /32 cidr --- utils/src/main/java/com/cloud/utils/net/NetUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/src/main/java/com/cloud/utils/net/NetUtils.java b/utils/src/main/java/com/cloud/utils/net/NetUtils.java index 2703deaad649..1e2ce286ecc8 100644 --- a/utils/src/main/java/com/cloud/utils/net/NetUtils.java +++ b/utils/src/main/java/com/cloud/utils/net/NetUtils.java @@ -574,7 +574,7 @@ public static boolean isValidIp4Cidr(final String cidr) { final String[] cidrPair = cidr.split("\\/"); if (cidrPair.length != 2) { - return false; + return isValidIp4(cidr); // we consider an ip4v address as a valid /32 cidr } final String cidrAddress = cidrPair[0]; final String cidrSize = cidrPair[1]; From 6a18b66e9c15ac844455ab5c45c1740b5b2df07a Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 9 Jan 2025 18:19:14 +0100 Subject: [PATCH 2/5] refactor cidr evaluation for internal nets --- .../SecondaryStorageManagerImpl.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index cd6f23923e14..276124a69a13 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -402,11 +402,17 @@ private List getAllowedInternalSiteCidrs() { } String[] cidrs = _allowedInternalSites.split(","); for (String cidr : cidrs) { - if (NetUtils.isValidIp4Cidr(cidr) || NetUtils.isValidIp4(cidr) || !cidr.startsWith("0.0.0.0")) { - if (NetUtils.getCleanIp4Cidr(cidr).equals(cidr)) { + if (NetUtils.isValidIp4Cidr(cidr)) { + if (! NetUtils.getCleanIp4Cidr(cidr).equals(cidr)) { s_logger.warn(String.format("Invalid CIDR %s in %s", cidr, SecStorageAllowedInternalDownloadSites.key())); } - allowedCidrs.add(cidr); + allowedCidrs.add(NetUtils.getCleanIp4Cidr(cidr)); + } else if (NetUtils.isValidIp4(cidr)) { + String newCidr = cidr + "/32"; + s_logger.warn(String.format("Ip address is not a valid CIDR; %s using %s/32", cidr, newCidr)); + allowedCidrs.add(newCidr); + } else if (!cidr.startsWith("0.0.0.0")) { + allowedCidrs.add(NetUtils.getCleanIp4Cidr(cidr)); } } return allowedCidrs; From 5874e8078ffef31daef9809c1267b1b899a348de Mon Sep 17 00:00:00 2001 From: dahn Date: Fri, 10 Jan 2025 10:56:26 +0100 Subject: [PATCH 3/5] Apply suggestions from code review --- .../secondarystorage/SecondaryStorageManagerImpl.java | 4 +--- utils/src/main/java/com/cloud/utils/net/NetUtils.java | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index 276124a69a13..4e541215ad1a 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -402,7 +402,7 @@ private List getAllowedInternalSiteCidrs() { } String[] cidrs = _allowedInternalSites.split(","); for (String cidr : cidrs) { - if (NetUtils.isValidIp4Cidr(cidr)) { + if (NetUtils.isValidIp4Cidr(cidr) && !cidr.startsWith("0.0.0.0")) { if (! NetUtils.getCleanIp4Cidr(cidr).equals(cidr)) { s_logger.warn(String.format("Invalid CIDR %s in %s", cidr, SecStorageAllowedInternalDownloadSites.key())); } @@ -411,8 +411,6 @@ private List getAllowedInternalSiteCidrs() { String newCidr = cidr + "/32"; s_logger.warn(String.format("Ip address is not a valid CIDR; %s using %s/32", cidr, newCidr)); allowedCidrs.add(newCidr); - } else if (!cidr.startsWith("0.0.0.0")) { - allowedCidrs.add(NetUtils.getCleanIp4Cidr(cidr)); } } return allowedCidrs; diff --git a/utils/src/main/java/com/cloud/utils/net/NetUtils.java b/utils/src/main/java/com/cloud/utils/net/NetUtils.java index 1e2ce286ecc8..2703deaad649 100644 --- a/utils/src/main/java/com/cloud/utils/net/NetUtils.java +++ b/utils/src/main/java/com/cloud/utils/net/NetUtils.java @@ -574,7 +574,7 @@ public static boolean isValidIp4Cidr(final String cidr) { final String[] cidrPair = cidr.split("\\/"); if (cidrPair.length != 2) { - return isValidIp4(cidr); // we consider an ip4v address as a valid /32 cidr + return false; } final String cidrAddress = cidrPair[0]; final String cidrSize = cidrPair[1]; From deb353ea3ead2a8c72fe498e5f5fed71b31cc9c8 Mon Sep 17 00:00:00 2001 From: dahn Date: Fri, 10 Jan 2025 13:18:10 +0100 Subject: [PATCH 4/5] Update services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java --- .../secondarystorage/SecondaryStorageManagerImpl.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index 4e541215ad1a..36c862acfec1 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -408,9 +408,8 @@ private List getAllowedInternalSiteCidrs() { } allowedCidrs.add(NetUtils.getCleanIp4Cidr(cidr)); } else if (NetUtils.isValidIp4(cidr)) { - String newCidr = cidr + "/32"; - s_logger.warn(String.format("Ip address is not a valid CIDR; %s using %s/32", cidr, newCidr)); - allowedCidrs.add(newCidr); + s_logger.warn(String.format("Ip address is not a valid CIDR; %s consider using %s/32", cidr, cidr)); + allowedCidrs.add(cidr); } } return allowedCidrs; From 03c259865a4f38a7a94bfa4ef04611b77690fe27 Mon Sep 17 00:00:00 2001 From: dahn Date: Tue, 14 Jan 2025 10:30:29 +0100 Subject: [PATCH 5/5] Update services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java --- .../secondarystorage/SecondaryStorageManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index 36c862acfec1..94a8f367d08c 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -407,7 +407,7 @@ private List getAllowedInternalSiteCidrs() { s_logger.warn(String.format("Invalid CIDR %s in %s", cidr, SecStorageAllowedInternalDownloadSites.key())); } allowedCidrs.add(NetUtils.getCleanIp4Cidr(cidr)); - } else if (NetUtils.isValidIp4(cidr)) { + } else if (NetUtils.isValidIp4(cidr) && !cidr.startsWith("0.0.0.0")) { s_logger.warn(String.format("Ip address is not a valid CIDR; %s consider using %s/32", cidr, cidr)); allowedCidrs.add(cidr); }