From 4df11a4198254c30167493fee429bd533ee6fbe8 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Sat, 21 Feb 2026 09:25:28 +0530 Subject: [PATCH] normalize dns zone and record in svc layer, always use dotless data in svc and handle dot version in client --- .../command/user/dns/CreateDnsZoneCmd.java | 24 ++-- .../META-INF/db/schema-42210to42300.sql | 4 +- .../dns/powerdns/PowerDnsClient.java | 4 +- .../apache/cloudstack/dns/DnsUtilTest.java | 103 ++++++++++++++++++ .../dns/DnsProviderManagerImpl.java | 53 +++++---- .../org/apache/cloudstack/dns/DnsUtil.java | 101 +++++++++++++++++ .../apache/cloudstack/dns/vo/DnsServerVO.java | 2 +- 7 files changed, 256 insertions(+), 35 deletions(-) create mode 100644 plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsUtilTest.java create mode 100644 server/src/main/java/org/apache/cloudstack/dns/DnsUtil.java diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/CreateDnsZoneCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/CreateDnsZoneCmd.java index ab0ac9cf4c3..61c7f5b5f44 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/CreateDnsZoneCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/CreateDnsZoneCmd.java @@ -17,6 +17,8 @@ package org.apache.cloudstack.api.command.user.dns; +import java.util.Arrays; + import javax.inject.Inject; import org.apache.cloudstack.api.APICommand; @@ -27,13 +29,14 @@ import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.DnsServerResponse; import org.apache.cloudstack.api.response.DnsZoneResponse; -import org.apache.cloudstack.api.response.NetworkResponse; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.dns.DnsProviderManager; import org.apache.cloudstack.dns.DnsZone; +import org.apache.commons.lang3.StringUtils; import com.cloud.event.EventTypes; import com.cloud.exception.ResourceAllocationException; +import com.cloud.utils.EnumUtils; @APICommand(name = "createDnsZone", description = "Creates a new DNS Zone on a specific server", responseObject = DnsZoneResponse.class, requestHasSensitiveInfo = false, @@ -55,10 +58,6 @@ public class CreateDnsZoneCmd extends BaseAsyncCreateCmd { required = true, description = "The ID of the DNS server to host this zone") private Long dnsServerId; - @Parameter(name = ApiConstants.NETWORK_ID, type = CommandType.UUID, entityType = NetworkResponse.class, - description = "Optional: The Guest Network to associate with this zone for auto-registration") - private Long networkId; - @Parameter(name = ApiConstants.TYPE, type = CommandType.STRING, description = "The type of zone (Public, Private). Defaults to Public.") private String type; @@ -78,12 +77,15 @@ public class CreateDnsZoneCmd extends BaseAsyncCreateCmd { return dnsServerId; } - public Long getNetworkId() { - return networkId; - } - - public String getType() { - return type; + public DnsZone.ZoneType getType() { + if (StringUtils.isBlank(type)) { + return DnsZone.ZoneType.Public; + } + DnsZone.ZoneType zoneType = EnumUtils.getEnumIgnoreCase(DnsZone.ZoneType.class, type); + if (type == null) { + throw new IllegalArgumentException("Invalid type value, supported values are: " + Arrays.toString(DnsZone.ZoneType.values())); + } + return zoneType; } public String getDescription() { diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql index 244930676c1..fe76e949b7c 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql @@ -62,9 +62,9 @@ CREATE TABLE `cloud`.`dns_server` ( `name` varchar(255) NOT NULL COMMENT 'display name of the dns server', `provider_type` varchar(255) NOT NULL COMMENT 'Provider type such as PowerDns', `url` varchar(1024) NOT NULL COMMENT 'dns server url', - `dns_username` varchar(255) NOT NULL COMMENT 'username or email for dns server credentials', + `dns_username` varchar(255) COMMENT 'username or email for dns server credentials', `api_key` varchar(255) NOT NULL COMMENT 'dns server api_key', - `dns_server_name` varchar(255) COMMENT 'dns server name e.g. localhost for powerdns', + `external_server_id` varchar(255) COMMENT 'dns server id e.g. localhost for powerdns', `port` int(11) DEFAULT NULL COMMENT 'optional dns server port', `name_servers` varchar(1024) DEFAULT NULL COMMENT 'Comma separated list of name servers', `is_public` tinyint(1) NOT NULL DEFAULT '0', diff --git a/plugins/dns/powerdns/src/main/java/org/apache/cloudstack/dns/powerdns/PowerDnsClient.java b/plugins/dns/powerdns/src/main/java/org/apache/cloudstack/dns/powerdns/PowerDnsClient.java index 2a19c306a08..27a322f7e39 100644 --- a/plugins/dns/powerdns/src/main/java/org/apache/cloudstack/dns/powerdns/PowerDnsClient.java +++ b/plugins/dns/powerdns/src/main/java/org/apache/cloudstack/dns/powerdns/PowerDnsClient.java @@ -139,7 +139,7 @@ public class PowerDnsClient implements AutoCloseable { public String createZone(String baseUrl, Integer port, String apiKey, String externalServerId, String zoneName, String zoneKind, boolean dnsSecFlag, List nameServers) throws DnsProviderException { - validateServerId(baseUrl, port, externalServerId, apiKey); + validateServerId(baseUrl, port, apiKey, externalServerId); String normalizedZone = normalizeZone(zoneName); ObjectNode json = MAPPER.createObjectNode(); json.put(ApiConstants.NAME, normalizedZone); @@ -167,7 +167,7 @@ public class PowerDnsClient implements AutoCloseable { public void updateZone(String baseUrl, Integer port, String apiKey, String externalServerId, String zoneName, String zoneKind, Boolean dnsSecFlag, List nameServers) throws DnsProviderException { - validateServerId(baseUrl, port, externalServerId, apiKey); + validateServerId(baseUrl, port, apiKey, externalServerId); String normalizedZone = normalizeZone(zoneName); String encodedZone = URLEncoder.encode(normalizedZone, StandardCharsets.UTF_8); String url = buildUrl(baseUrl, port,"/servers/" + externalServerId + "/zones/" + encodedZone); diff --git a/plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsUtilTest.java b/plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsUtilTest.java new file mode 100644 index 00000000000..bc155e8d982 --- /dev/null +++ b/plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsUtilTest.java @@ -0,0 +1,103 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.dns; + +import static org.apache.cloudstack.dns.DnsUtil.appendPublicSuffixToZone; +import static org.apache.cloudstack.dns.DnsUtil.normalizeDomain; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.util.Arrays; +import java.util.Collection; + +import org.apache.logging.log4j.util.Strings; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class DnsUtilTest { + private final String userZoneName; + private final String publicSuffix; + private final String expectedResult; + private final boolean expectException; + + public DnsUtilTest(String userZoneName, + String publicSuffix, + String expectedResult, + boolean expectException) { + this.userZoneName = userZoneName; + this.publicSuffix = publicSuffix; + this.expectedResult = expectedResult; + this.expectException = expectException; + } + + @Parameterized.Parameters + public static Collection data() { + return Arrays.asList(new Object[][]{ + {"tenant1.com", "example.com", "tenant1.example.com", false}, + {"dev.tenant2.com", "example.com", "dev.tenant2.example.com", false}, + {"tenant3.example.com", "example.com", "tenant3.example.com", false}, + {"Tenant1.CoM", "ExAmple.CoM", "tenant1.example.com", false}, + {"tenant1.com.", "example.com.", "tenant1.example.com", false}, + {"tenant1.com", "", "tenant1.com", false}, + {"tenant1.com", null, "tenant1.com", false}, + {"test.abc.com", "abc.com", "test.abc.com", false}, + {"sub.test.abc.com", "abc.com", "sub.test.abc.com", false}, + {"test.ai.abc.com", "abc.com", "test.ai.abc.com", false}, + {"deep.sub.abc.com", "abc.com", "deep.sub.abc.com", false}, + {"abc.com", "xyz.com", "abc.xyz.com", false}, + {"test.xyz.com", "xyz.com", "test.xyz.com", false}, + {"test.com.xyz.com", "xyz.com", "test.com.xyz.com", false}, + {"tenant", "example.com", null, true}, // single label + {"test", "abc.com", null, true}, + {"example.com.", "example.com", null, true}, + {"example.com", "example.com", null, true}, // root level forbidden + {"abc.com", "abc.com", null, true}, // root level forbidden + {"tenant1.org", "example.com", null, true}, // TLD mismatch + {"test.ai", "abc.com", null, true}, // TLD mismatch + {null, "example.com", null, true}, + }); + } + + @Test + public void testAppendPublicSuffix() { + if (expectException) { + try { + executeAppendSuffixTest(userZoneName, publicSuffix); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException ignored) { + // noop + } + } else { + String result; + if (Strings.isNotBlank(publicSuffix)) { + result = executeAppendSuffixTest(userZoneName, publicSuffix); + } else { + result = appendPublicSuffixToZone(normalizeDomain(userZoneName), publicSuffix); + } + assertEquals(expectedResult, result); + } + } + + String executeAppendSuffixTest(String zoneName, String domainSuffix) { + return appendPublicSuffixToZone( + normalizeDomain(zoneName), + normalizeDomain(domainSuffix)); + } +} diff --git a/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java b/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java index 6739ccb0b11..b48eece57aa 100644 --- a/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java @@ -20,6 +20,7 @@ package org.apache.cloudstack.dns; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.stream.Collectors; import javax.inject.Inject; @@ -117,6 +118,11 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa isDnsPublic = false; publicDomainSuffix = null; } + + if (StringUtils.isNotBlank(publicDomainSuffix)) { + publicDomainSuffix = DnsUtil.normalizeDomain(publicDomainSuffix); + } + DnsProviderType type = DnsProviderType.fromString(cmd.getProvider()); DnsServerVO server = new DnsServerVO(cmd.getName(), cmd.getUrl(), cmd.getPort(), cmd.getExternalServerId(), type, cmd.getDnsUserName(), cmd.getCredentials(), isDnsPublic, publicDomainSuffix, cmd.getNameServers(), @@ -208,7 +214,7 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa } if (cmd.getPublicDomainSuffix() != null) { - dnsServer.setPublicDomainSuffix(cmd.getPublicDomainSuffix()); + dnsServer.setPublicDomainSuffix(DnsUtil.normalizeDomain(cmd.getPublicDomainSuffix())); } if (cmd.getNameServers() != null) { @@ -255,6 +261,7 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa response.setId(server.getUuid()); response.setName(server.getName()); response.setUrl(server.getUrl()); + response.setPort(server.getPort()); response.setProvider(server.getProviderType()); response.setPublic(server.isPublic()); response.setNameServers(server.getNameServers()); @@ -272,7 +279,7 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa public boolean deleteDnsZone(Long zoneId) { DnsZoneVO zone = dnsZoneDao.findById(zoneId); if (zone == null) { - throw new InvalidParameterValueException("DNS zone with ID " + zoneId + " not found."); + throw new InvalidParameterValueException("DNS zone not found for the given ID."); } Account caller = CallContext.current().getCallingAccount(); @@ -347,23 +354,29 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa @Override public DnsRecordResponse createDnsRecord(CreateDnsRecordCmd cmd) { + String recordName = StringUtils.trimToEmpty(cmd.getName()).toLowerCase(); + if (StringUtils.isBlank(recordName)) { + throw new InvalidParameterValueException("Empty DNS record name is not allowed"); + } DnsZoneVO zone = dnsZoneDao.findById(cmd.getDnsZoneId()); if (zone == null) { throw new InvalidParameterValueException("DNS zone not found."); } - Account caller = CallContext.current().getCallingAccount(); accountMgr.checkAccess(caller, null, true, zone); DnsServerVO server = dnsServerDao.findById(zone.getDnsServerId()); try { + DnsRecord.RecordType type = cmd.getType(); + List normalizedContents = cmd.getContents().stream() + .map(value -> DnsUtil.normalizeDnsRecordValue(value, type)).collect(Collectors.toList()); + DnsRecord record = new DnsRecord(recordName, type, normalizedContents, cmd.getTtl()); DnsProvider provider = getProvider(server.getProviderType()); - DnsRecord record = new DnsRecord(cmd.getName(), cmd.getType(), cmd.getContents(), cmd.getTtl()); String normalizedRecordName = provider.addRecord(server, zone, record); record.setName(normalizedRecordName); return createDnsRecordResponse(record); } catch (Exception ex) { logger.error("Failed to add DNS record via provider", ex); - throw new CloudRuntimeException(String.format("Failed to add DNS record: %s", cmd.getName())); + throw new CloudRuntimeException(String.format("Failed to add DNS record: %s", recordName)); } } @@ -397,7 +410,7 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa public ListResponse listDnsRecords(ListDnsRecordsCmd cmd) { DnsZoneVO zone = dnsZoneDao.findById(cmd.getDnsZoneId()); if (zone == null) { - throw new InvalidParameterValueException(String.format("DNS zone with ID %s not found.", cmd.getDnsZoneId())); + throw new InvalidParameterValueException("DNS zone not found for the given ID."); } Account caller = CallContext.current().getCallingAccount(); accountMgr.checkAccess(caller, null, true, zone); @@ -435,28 +448,30 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa @Override public DnsZone allocateDnsZone(CreateDnsZoneCmd cmd) { - Account caller = CallContext.current().getCallingAccount(); + if (StringUtils.isBlank(cmd.getName())) { + throw new InvalidParameterValueException("DNS zone name cannot be empty"); + } + + String dnsZoneName = DnsUtil.normalizeDomain(cmd.getName()); DnsServerVO server = dnsServerDao.findById(cmd.getDnsServerId()); if (server == null) { - throw new InvalidParameterValueException("DNS server not found"); + throw new InvalidParameterValueException(String.format("DNS server not found for the given ID: %s", cmd.getDnsServerId())); } + + Account caller = CallContext.current().getCallingAccount(); boolean isOwner = (server.getAccountId() == caller.getId()); - if (!server.isPublic() && !isOwner) { - throw new PermissionDeniedException("You do not have permission to use this DNS server."); - } - DnsZone.ZoneType type = DnsZone.ZoneType.Public; - if (cmd.getType() != null) { - try { - type = DnsZone.ZoneType.valueOf(cmd.getType()); - } catch (IllegalArgumentException e) { - throw new InvalidParameterValueException("Invalid DNS zone Type"); + if (!isOwner) { + if (!server.isPublic()) { + throw new PermissionDeniedException("You do not have permission to use this DNS server."); } + dnsZoneName = DnsUtil.appendPublicSuffixToZone(dnsZoneName, DnsUtil.normalizeDomain(server.getPublicDomainSuffix())); } - DnsZoneVO existing = dnsZoneDao.findByNameServerAndType(cmd.getName(), server.getId(), type); + DnsZone.ZoneType type = cmd.getType(); + DnsZoneVO existing = dnsZoneDao.findByNameServerAndType(dnsZoneName, server.getId(), type); if (existing != null) { throw new InvalidParameterValueException("DNS zone already exists on this server."); } - DnsZoneVO dnsZoneVO = new DnsZoneVO(cmd.getName(), type, server.getId(), caller.getId(), caller.getDomainId(), cmd.getDescription()); + DnsZoneVO dnsZoneVO = new DnsZoneVO(dnsZoneName, type, server.getId(), caller.getId(), caller.getDomainId(), cmd.getDescription()); return dnsZoneDao.persist(dnsZoneVO); } diff --git a/server/src/main/java/org/apache/cloudstack/dns/DnsUtil.java b/server/src/main/java/org/apache/cloudstack/dns/DnsUtil.java new file mode 100644 index 00000000000..dc312f55287 --- /dev/null +++ b/server/src/main/java/org/apache/cloudstack/dns/DnsUtil.java @@ -0,0 +1,101 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.dns; + +import org.apache.commons.validator.routines.DomainValidator; + +import com.cloud.utils.StringUtils; + +public class DnsUtil { + static DomainValidator validator = DomainValidator.getInstance(true); + + public static String appendPublicSuffixToZone(String zoneName, String suffixDomain) { + if (StringUtils.isBlank(suffixDomain)) { + return zoneName; + } + // Already suffixed → return as-is + if (zoneName.toLowerCase().endsWith("." + suffixDomain.toLowerCase())) { + return zoneName; + } + + if (zoneName.equals(suffixDomain)) { + throw new IllegalArgumentException("Cannot create DNS zone at root-level: " + suffixDomain); + } + // Check TLD matches + String tldUser = getTld(zoneName); + String tldSuffix = getTld(suffixDomain); + + if (!tldUser.equalsIgnoreCase(tldSuffix)) { + throw new IllegalArgumentException("TLD mismatch between user zone and domain suffix"); + } + // Remove TLD from userZone + int lastDot = zoneName.lastIndexOf('.'); + String zonePrefix = zoneName.substring(0, lastDot); + return zonePrefix + "." + suffixDomain; + } + + private static String getTld(String domain) { + String[] labels = domain.split("\\."); + return labels[labels.length - 1]; + } + + public static String normalizeDomain(String domain) { + if (StringUtils.isBlank(domain)) { + throw new IllegalArgumentException("Domain cannot be empty"); + } + + String normalized = domain.trim().toLowerCase(); + if (normalized.endsWith(".")) { + normalized = normalized.substring(0, normalized.length() - 1); + } + // Validate domain, allow local/private TLDs + if (!validator.isValid(normalized)) { + throw new IllegalArgumentException("Invalid domain name: " + domain); + } + return normalized; + } + + public static String normalizeDnsRecordValue(String value, DnsRecord.RecordType recordType) { + if (StringUtils.isBlank(value)) { + throw new IllegalArgumentException("DNS record value cannot be empty"); + } + switch (recordType) { + case A: + case AAAA: + // IP addresses: trim only + return value.trim(); + + case CNAME: + case NS: + case PTR: + case SRV: + // Domain names: normalize like zones + return normalizeDomain(value); + case MX: + // PowerDNS MX: contains priority + domain, only trim and lowercase + return value.trim().toLowerCase(); + + case TXT: + // Free text: preserve exactly + return value; + + default: + throw new IllegalArgumentException("Unsupported DNS record type: " + recordType); + } + } +} diff --git a/server/src/main/java/org/apache/cloudstack/dns/vo/DnsServerVO.java b/server/src/main/java/org/apache/cloudstack/dns/vo/DnsServerVO.java index 2b922f1063e..d27b3486612 100644 --- a/server/src/main/java/org/apache/cloudstack/dns/vo/DnsServerVO.java +++ b/server/src/main/java/org/apache/cloudstack/dns/vo/DnsServerVO.java @@ -66,7 +66,7 @@ public class DnsServerVO implements DnsServer { @Enumerated(EnumType.STRING) private DnsProviderType providerType; - @Column(name = "dns_user_name") + @Column(name = "dns_username") private String dnsUserName; @Encrypt