This is an automated email from the ASF dual-hosted git repository.
weizhouapache pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 4e4a26b49ef dns: prevent creation of cross-tenant conflicting DNS
zones in the same server (#13946)
4e4a26b49ef is described below
commit 4e4a26b49ef19e892a4b695b6291b21f2117c70b
Author: Fabricio Duarte <[email protected]>
AuthorDate: Mon Aug 24 10:29:25 2026 -0300
dns: prevent creation of cross-tenant conflicting DNS zones in the same
server (#13946)
---
.../cloudstack/dns/DnsProviderManagerImpl.java | 42 +++++++++++-
.../org/apache/cloudstack/dns/dao/DnsZoneDao.java | 2 +
.../apache/cloudstack/dns/dao/DnsZoneDaoImpl.java | 24 +++++--
.../cloudstack/dns/DnsProviderManagerImplTest.java | 76 ++++++++++++++++++++++
ui/src/views/network/dns/AddDnsServer.vue | 7 +-
ui/src/views/network/dns/UpdateDnsServer.vue | 7 +-
6 files changed, 145 insertions(+), 13 deletions(-)
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 b451da1baf7..e1ace5d7a3c 100644
--- a/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java
@@ -184,6 +184,11 @@ public class DnsProviderManagerImpl extends ManagerBase
implements DnsProviderMa
publicDomainSuffix =
DnsProviderUtil.normalizeDomainForDb(publicDomainSuffix);
}
+ if (isDnsPublic && StringUtils.isBlank(publicDomainSuffix)) {
+ throw new InvalidParameterValueException("A public DNS server
requires a public domain suffix so that " +
+ "DNS zones created by other accounts are contained under
it.");
+ }
+
DnsProviderType type = cmd.getProvider();
DnsServerVO server = new DnsServerVO(cmd.getName(), cmd.getUrl(),
cmd.getPort(), type,
cmd.getDnsUserName(), cmd.getDnsApiKey(), isDnsPublic,
publicDomainSuffix, cmd.getNameServers(),
@@ -273,12 +278,20 @@ public class DnsProviderManagerImpl extends ManagerBase
implements DnsProviderMa
if (accountMgr.isRootAdmin(caller.getId()) ||
accountMgr.isDomainAdmin(caller.getId())) {
if (cmd.isPublic() != null) {
boolean isPublic = BooleanUtils.isTrue(cmd.isPublic());
- dnsServer.setPublicServer(isPublic);
String publicDomainSuffix = null;
- if (isPublic &&
StringUtils.isNotBlank(cmd.getPublicDomainSuffix())) {
- publicDomainSuffix =
DnsProviderUtil.normalizeDomainForDb(cmd.getPublicDomainSuffix());
+ if (isPublic) {
+ if (StringUtils.isNotBlank(cmd.getPublicDomainSuffix())) {
+ publicDomainSuffix =
DnsProviderUtil.normalizeDomainForDb(cmd.getPublicDomainSuffix());
+ } else {
+ publicDomainSuffix = dnsServer.getPublicDomainSuffix();
+ }
+ if (StringUtils.isBlank(publicDomainSuffix)) {
+ throw new InvalidParameterValueException("A public DNS
server requires a public domain " +
+ "suffix so that DNS zones created by other
accounts are contained under it.");
+ }
}
+ dnsServer.setPublicServer(isPublic);
dnsServer.setPublicDomainSuffix(publicDomainSuffix);
}
}
@@ -590,6 +603,7 @@ public class DnsProviderManagerImpl extends ManagerBase
implements DnsProviderMa
throw new PermissionDeniedException("You do not have
permission to use this DNS server.");
}
dnsZoneName =
DnsProviderUtil.appendPublicSuffixToZone(dnsZoneName,
server.getPublicDomainSuffix());
+ checkDnsZoneNameConflictsAcrossAccounts(dnsZoneName,
server.getId(), caller.getId());
}
DnsZone.ZoneType type = cmd.getType();
DnsZoneVO existing = dnsZoneDao.findByNameServerAndType(dnsZoneName,
server.getId(), type);
@@ -600,6 +614,28 @@ public class DnsProviderManagerImpl extends ManagerBase
implements DnsProviderMa
return dnsZoneDao.persist(dnsZoneVO);
}
+ /**
+ * Rejects a DNS zone name that is equal to, a DNS child of, or a DNS
parent of an existing zone owned by a
+ * different account on the same DNS server. Without this, a co-tenant
could register e.g.
+ * {@code www.victimzone.<suffix>} on a shared public server and shadow
the victim's records in the
+ * authoritative name server, since the more specific zone wins resolution.
+ */
+ private void checkDnsZoneNameConflictsAcrossAccounts(String dnsZoneName,
long dnsServerId, long callerAccountId) {
+ String requestedName = dnsZoneName.toLowerCase();
+ List<DnsZoneVO> existingZones =
dnsZoneDao.listByDnsServerId(dnsServerId);
+ for (DnsZoneVO zone : existingZones) {
+ if (zone.getAccountId() == callerAccountId) {
+ continue;
+ }
+ String existingName = zone.getName().toLowerCase();
+ if (requestedName.equals(existingName) ||
requestedName.endsWith("." + existingName)
+ || existingName.endsWith("." + requestedName)) {
+ throw new PermissionDeniedException(String.format("DNS zone
name %s conflicts with an existing DNS " +
+ "zone owned by another account on this DNS server.",
dnsZoneName));
+ }
+ }
+ }
+
@Override
public DnsZone provisionDnsZone(long dnsZoneId, boolean isExistingZone) {
DnsZoneVO dnsZone = dnsZoneDao.findById(dnsZoneId);
diff --git a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDao.java
b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDao.java
index 43bf60818d5..adcf3542189 100644
--- a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDao.java
+++ b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDao.java
@@ -34,4 +34,6 @@ public interface DnsZoneDao extends GenericDao<DnsZoneVO,
Long> {
String keyword, Filter filter);
List<Long> findDnsZoneIdsByServerId(long dnsServerId);
+
+ List<DnsZoneVO> listByDnsServerId(long dnsServerId);
}
diff --git
a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDaoImpl.java
b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDaoImpl.java
index 2487f2fee20..c8fdf8b8452 100644
--- a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDaoImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDaoImpl.java
@@ -35,17 +35,22 @@ import com.cloud.utils.db.SearchCriteria;
@Component
public class DnsZoneDaoImpl extends GenericDaoBase<DnsZoneVO, Long> implements
DnsZoneDao {
- SearchBuilder<DnsZoneVO> DnsServerSearch;
+ SearchBuilder<DnsZoneVO> DnsServerZoneIdsSearch;
+ SearchBuilder<DnsZoneVO> DnsServerZonesSearch;
SearchBuilder<DnsZoneVO> AccountSearch;
SearchBuilder<DnsZoneVO> NameServerTypeSearch;
public DnsZoneDaoImpl() {
super();
- DnsServerSearch = createSearchBuilder();
- DnsServerSearch.selectFields(DnsServerSearch.entity().getId());
- DnsServerSearch.and(ApiConstants.DNS_SERVER_ID,
DnsServerSearch.entity().getDnsServerId(), SearchCriteria.Op.EQ);
- DnsServerSearch.done();
+ DnsServerZoneIdsSearch = createSearchBuilder();
+
DnsServerZoneIdsSearch.selectFields(DnsServerZoneIdsSearch.entity().getId());
+ DnsServerZoneIdsSearch.and(ApiConstants.DNS_SERVER_ID,
DnsServerZoneIdsSearch.entity().getDnsServerId(), SearchCriteria.Op.EQ);
+ DnsServerZoneIdsSearch.done();
+
+ DnsServerZonesSearch = createSearchBuilder();
+ DnsServerZonesSearch.and(ApiConstants.DNS_SERVER_ID,
DnsServerZonesSearch.entity().getDnsServerId(), SearchCriteria.Op.EQ);
+ DnsServerZonesSearch.done();
AccountSearch = createSearchBuilder();
AccountSearch.and(ApiConstants.ACCOUNT_ID,
AccountSearch.entity().getAccountId(), SearchCriteria.Op.EQ);
@@ -116,8 +121,15 @@ public class DnsZoneDaoImpl extends
GenericDaoBase<DnsZoneVO, Long> implements D
return searchAndCount(sc, filter);
}
+ @Override
+ public List<DnsZoneVO> listByDnsServerId(long dnsServerId) {
+ SearchCriteria<DnsZoneVO> sc = DnsServerZonesSearch.create();
+ sc.setParameters(ApiConstants.DNS_SERVER_ID, dnsServerId);
+ return listBy(sc);
+ }
+
public List<Long> findDnsZoneIdsByServerId(long dnsServerId) {
- SearchCriteria<DnsZoneVO> sc = DnsServerSearch.create();
+ SearchCriteria<DnsZoneVO> sc = DnsServerZoneIdsSearch.create();
sc.setParameters(ApiConstants.DNS_SERVER_ID, dnsServerId);
List<DnsZoneVO> dnsZones = listBy(sc);
if (CollectionUtils.isEmpty(dnsZones)) {
diff --git
a/server/src/test/java/org/apache/cloudstack/dns/DnsProviderManagerImplTest.java
b/server/src/test/java/org/apache/cloudstack/dns/DnsProviderManagerImplTest.java
index 309f5e5d9cf..8ce31d4e7c7 100644
---
a/server/src/test/java/org/apache/cloudstack/dns/DnsProviderManagerImplTest.java
+++
b/server/src/test/java/org/apache/cloudstack/dns/DnsProviderManagerImplTest.java
@@ -262,6 +262,60 @@ public class DnsProviderManagerImplTest {
manager.allocateDnsZone(cmd);
}
+ @Test(expected = PermissionDeniedException.class)
+ public void testAllocateDnsZoneNonOwnerShadowingOtherAccountZoneRejected()
{
+ CreateDnsZoneCmd cmd = mock(CreateDnsZoneCmd.class);
+ when(cmd.getName()).thenReturn("www.tenant1.cloud.example");
+ when(cmd.getDnsServerId()).thenReturn(SERVER_ID);
+ when(dnsServerDao.findById(SERVER_ID)).thenReturn(serverVO);
+ Mockito.doReturn(SERVER_ID).when(serverVO).getId();
+ Mockito.doReturn(ACCOUNT_ID + 99).when(serverVO).getAccountId(); //
different owner
+ Mockito.doReturn(true).when(serverVO).getPublicServer();
+
Mockito.doReturn("cloud.example").when(serverVO).getPublicDomainSuffix();
+ DnsZoneVO victimZone = new DnsZoneVO("tenant1.cloud.example",
DnsZone.ZoneType.Public, SERVER_ID,
+ ACCOUNT_ID + 50, DOMAIN_ID, "victim zone");
+
when(dnsZoneDao.listByDnsServerId(SERVER_ID)).thenReturn(Collections.singletonList(victimZone));
+
+ manager.allocateDnsZone(cmd);
+ }
+
+ @Test(expected = PermissionDeniedException.class)
+ public void testAllocateDnsZoneNonOwnerParentOfOtherAccountZoneRejected() {
+ CreateDnsZoneCmd cmd = mock(CreateDnsZoneCmd.class);
+ when(cmd.getName()).thenReturn("tenant1.cloud.example");
+ when(cmd.getDnsServerId()).thenReturn(SERVER_ID);
+ when(dnsServerDao.findById(SERVER_ID)).thenReturn(serverVO);
+ Mockito.doReturn(SERVER_ID).when(serverVO).getId();
+ Mockito.doReturn(ACCOUNT_ID + 99).when(serverVO).getAccountId(); //
different owner
+ Mockito.doReturn(true).when(serverVO).getPublicServer();
+
Mockito.doReturn("cloud.example").when(serverVO).getPublicDomainSuffix();
+ DnsZoneVO victimZone = new DnsZoneVO("www.tenant1.cloud.example",
DnsZone.ZoneType.Public, SERVER_ID,
+ ACCOUNT_ID + 50, DOMAIN_ID, "victim zone");
+
when(dnsZoneDao.listByDnsServerId(SERVER_ID)).thenReturn(Collections.singletonList(victimZone));
+
+ manager.allocateDnsZone(cmd);
+ }
+
+ @Test
+ public void testAllocateDnsZoneNonOwnerPublicServerSuccess() {
+ CreateDnsZoneCmd cmd = mock(CreateDnsZoneCmd.class);
+ when(cmd.getName()).thenReturn("tenant2.cloud.example");
+ when(cmd.getDnsServerId()).thenReturn(SERVER_ID);
+ when(cmd.getType()).thenReturn(DnsZone.ZoneType.Public);
+ when(dnsServerDao.findById(SERVER_ID)).thenReturn(serverVO);
+ Mockito.doReturn(SERVER_ID).when(serverVO).getId();
+ Mockito.doReturn(ACCOUNT_ID + 99).when(serverVO).getAccountId(); //
different owner
+ Mockito.doReturn(true).when(serverVO).getPublicServer();
+
Mockito.doReturn("cloud.example").when(serverVO).getPublicDomainSuffix();
+
when(dnsZoneDao.listByDnsServerId(SERVER_ID)).thenReturn(Collections.emptyList());
+ when(dnsZoneDao.findByNameServerAndType(anyString(), anyLong(),
any())).thenReturn(null);
+ when(dnsZoneDao.persist(any(DnsZoneVO.class))).thenReturn(zoneVO);
+
+ DnsZone result = manager.allocateDnsZone(cmd);
+ assertNotNull(result);
+ verify(dnsZoneDao).persist(Mockito.argThat(z ->
"tenant2.cloud.example".equals(((DnsZoneVO) z).getName())));
+ }
+
@Test(expected = CloudRuntimeException.class)
public void testProvisionDnsZoneNotFound() {
when(dnsZoneDao.findById(ZONE_ID)).thenReturn(null);
@@ -806,6 +860,28 @@ public class DnsProviderManagerImplTest {
s -> !((DnsServerVO) s).getPublicServer() && ((DnsServerVO)
s).getPublicDomainSuffix() == null));
}
+ @Test(expected = InvalidParameterValueException.class)
+ public void testAddDnsServerPublicWithoutSuffixRejected() {
+ org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd cmd = mock(
+
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd.class);
+ when(accountMgr.isRootAdmin(callerMock.getId())).thenReturn(true);
+ when(cmd.getUrl()).thenReturn("http://newpdns:8081";);
+ when(cmd.isPublic()).thenReturn(true);
+ when(dnsServerDao.findByUrlAndAccount(anyString(),
anyLong())).thenReturn(null);
+ manager.addDnsServer(cmd);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void testUpdateDnsServerPublicWithoutSuffixRejected() {
+ org.apache.cloudstack.api.command.user.dns.UpdateDnsServerCmd cmd =
mock(
+
org.apache.cloudstack.api.command.user.dns.UpdateDnsServerCmd.class);
+ when(cmd.getId()).thenReturn(SERVER_ID);
+ when(cmd.isPublic()).thenReturn(true);
+ when(accountMgr.isRootAdmin(callerMock.getId())).thenReturn(true);
+ when(dnsServerDao.findById(SERVER_ID)).thenReturn(serverVO);
+ manager.updateDnsServer(cmd);
+ }
+
@Test(expected = CloudRuntimeException.class)
public void testAddDnsServerValidationFailure() throws Exception {
org.apache.cloudstack.api.command.user.dns.AddDnsServerCmd cmd = mock(
diff --git a/ui/src/views/network/dns/AddDnsServer.vue
b/ui/src/views/network/dns/AddDnsServer.vue
index 3950c85248d..054b0e37863 100644
--- a/ui/src/views/network/dns/AddDnsServer.vue
+++ b/ui/src/views/network/dns/AddDnsServer.vue
@@ -205,7 +205,10 @@ export default {
]
}
if (this.isAdminOrDomainAdmin()) {
- this.rules.publicdomainsuffix = [{ validator:
this.validatePublicDomainSuffix }]
+ this.rules.publicdomainsuffix = [{
+ required: true,
+ validator: this.validatePublicDomainSuffix
+ }]
}
this.fetchProviders()
},
@@ -331,7 +334,7 @@ export default {
validatePublicDomainSuffix (rule, value) {
const normalized = value?.toLowerCase().trim()
if (!normalized) {
- return Promise.resolve()
+ return Promise.reject(new
Error(this.$t('message.error.required.input')))
}
if (!FQDN_REGEX.test(normalized)) {
return Promise.reject(new Error('Invalid domain suffix'))
diff --git a/ui/src/views/network/dns/UpdateDnsServer.vue
b/ui/src/views/network/dns/UpdateDnsServer.vue
index b8bd4f352e4..728151a2153 100644
--- a/ui/src/views/network/dns/UpdateDnsServer.vue
+++ b/ui/src/views/network/dns/UpdateDnsServer.vue
@@ -164,7 +164,10 @@ export default {
]
}
if (this.isAdminOrDomainAdmin()) {
- this.rules.publicdomainsuffix = [{ validator:
this.validatePublicDomainSuffix }]
+ this.rules.publicdomainsuffix = [{
+ required: true,
+ validator: this.validatePublicDomainSuffix
+ }]
}
this.form.name = this.resource.name
this.form.url = this.resource.url
@@ -272,7 +275,7 @@ export default {
validatePublicDomainSuffix (rule, value) {
const normalized = value?.toLowerCase().trim()
if (!normalized) {
- return Promise.resolve()
+ return Promise.reject(new
Error(this.$t('message.error.required.input')))
}
if (!FQDN_REGEX.test(normalized)) {
return Promise.reject(new Error('Invalid domain suffix'))
