weizhouapache commented on code in PR #13821:
URL: https://github.com/apache/cloudstack/pull/13821#discussion_r3803270326


##########
server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java:
##########
@@ -162,14 +164,43 @@ private DnsProvider getProviderByType(DnsProviderType 
type) {
         throw new CloudRuntimeException("No plugin found for DNS provider 
type: " + type);
     }
 
+    /**
+     * Trims and rejects a DNS provider URL that resolves to an illegal 
address before any provider client
+     * is given the chance to connect to it. See {@link 
UriUtils#validateUrl(String)} for the exact rules
+     * enforced (including the requirement that the URL declares an {@code 
http}/{@code https} scheme).
+     * Private/site-local addresses (e.g. {@code 192.168.0.0/16}) are only 
permitted for root admin callers.
+     *
+     * @return the trimmed URL.
+     * @throws InvalidParameterValueException if the URL is blank, fails 
validation, or is a private address
+     * requested by a non-root-admin caller.
+     */
+    private String validateDnsServerUrl(String url, Account caller) {
+        String trimmedUrl = StringUtils.trim(url);
+        if (StringUtils.isBlank(trimmedUrl)) {
+            throw new InvalidParameterValueException("URL cannot be blank.");
+        }
+        Pair<String, Integer> hostAndPort;
+        try {
+            hostAndPort = UriUtils.validateUrl(trimmedUrl);
+        } catch (IllegalArgumentException e) {
+            throw new InvalidParameterValueException(e.getMessage());
+        }
+        if (!accountMgr.isRootAdmin(caller.getId()) && 
NetUtils.isSiteLocalAddress(hostAndPort.first())) {
+            throw new InvalidParameterValueException(
+                    "Only root admin accounts can configure a DNS server on a 
private/internal network address.");
+        }
+        return trimmedUrl;
+    }
+
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_DNS_SERVER_ADD, eventDescription 
= "Adding a DNS Server")
     public DnsServer addDnsServer(AddDnsServerCmd cmd) {
         Account caller = CallContext.current().getCallingAccount();
-        DnsServer existing = dnsServerDao.findByUrlAndAccount(cmd.getUrl(), 
caller.getId());
+        String url = validateDnsServerUrl(cmd.getUrl(), caller);

Review Comment:
   should we trim the url before validation so the trimmed url will be checked 
for duplication and saved in database ?
   
   @sudo87 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to