Merge pull request #30 from Netflix/https-cors

Restrict "CORS" to HTTPS and a full hostname match, as well as to other schemes with a full origin match. Wildcard matching of HTTPS subdomains is supported, and there is limited support for wildcard matching in other schemes.

client/makefile

@@ -6,12 +6,12 @@ CC=$(TARGET)g++
 includes = $(wildcard *.h)
 OBJS := main.cpp DialServer.cpp DialDiscovery.cpp DialConformance.cpp DialClientInput.cpp
 
+debug: CFLAGS += -DDEBUG
+debug: dialclient
+
 # You may not need all these libraries.  This example uses a build of curl that needs crypto, ssl, cares, and zlib
 dialclient: $(OBJS) ${includes}
 	$(CC) -Wall -Werror -g $(OBJS) $(INCLUDES) $(LDFLAGS) -ldl -lpthread -lcurl -lz -lcrypto -lssl -o dialclient
 
-dialclient_debug: $(OBJS) ${includes}
-	$(CC) -DDEBUG -Wall -Werror -g $(OBJS) $(INCLUDES) $(LDFLAGS) -ldl -lpthread -lcurl -lz -lcrypto -lssl -lcares -o dialclient_debug
-
 clean:
 	rm -f *.o dialclient

makefile

@@ -3,6 +3,8 @@ DIRS += server
 
 all:
 	for dir in $(DIRS); do (make -C $$dir || exit 1) || exit 1; done
+debug:
+	for dir in $(DIRS); do (make debug -C $$dir || exit 1) || exit 1; done
 clean:
 	for dir in $(DIRS); do (make clean -C $$dir || exit 1) || exit 1; done
 

server/dial_server.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2019 Netflix, Inc.
+ * Copyright (c) 2014-2020 Netflix, Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -42,7 +42,8 @@
 #define DIAL_PORT (56789)
 #define DIAL_DATA_SIZE (8*1024)
 
-static const char *gLocalhost = "127.0.0.1";
+static const char * const gLocalhost = "127.0.0.1";
+static const char * const gHttpsProto = "https://";
 
 struct DIALApp_ {
     struct DIALApp_ *next;
@@ -491,70 +492,202 @@ static void handle_dial_data(struct mg_connection *conn,
     ds_unlock(ds);
 }
 
-static int ends_with(const char *str, const char *suffix) {
+/**
-    if (!str || !suffix)
+ * Returns true if the origin is acceptable based on the candidate value.
-        return 0;
+ * The candidate may accept any subdomain if its domain begins with *. and
-    size_t lenstr = strlen(str);
+ * may require an exact port number match if it includes a colon to
-    size_t lensuffix = strlen(suffix);
+ * indicate a port number.
-    if (lensuffix > lenstr)
+ *
-        return 0;
+ * This function assumes that the candidate value is well-formed, meaning
-    return strncmp(str + lenstr - lensuffix, suffix, lensuffix) == 0;
+ * it will not include invalid characters or a non-numeric port number.
-}
+ * 
-
+ * @param origin the origin header value, which must begin with https://
-
+ * @param candidate the authorized origin value, which must begin with
-// str contains a white space separated list of strings (only supports SPACE characters for now)
+ *        https://
-static int ends_with_in_list (const char *str, const char *list) {
+ * @return true if accepted and false if not.
-    if (!str || !list)
+ */
+static int host_matches(const char *origin, const char *candidate) {
+    // Make sure there is something to compare.
+    if (!origin || !candidate)
         return 0;
     
-    const char * scanPointer=list;
+    // Make sure the origin and candidate both begin with HTTPS.
-    const char * spacePointer;
+    const size_t https_len= strlen(gHttpsProto);
-    unsigned int substringSize = 257;
+    if (strncmp(origin, gHttpsProto, https_len) != 0 ||
-    char *substring = (char *)malloc(substringSize);
+        strncmp(candidate, gHttpsProto, https_len) != 0)
-    if (!substring){
+    {
         return 0;
     }
-    while ( (spacePointer =strchr(scanPointer, ' ')) != NULL) {
+    
-    	int copyLength = spacePointer - scanPointer;      
+    // For the rest of the check, we only care about the hostname and optional
-      
+    // port number.
-      // protect against buffer overflow
+    const char * origin_host = origin + https_len;
-      if (copyLength>=substringSize){
+    const char * host = candidate + https_len;
-          substringSize=copyLength+1;
-          free(substring);
-          substring=(char *)malloc(substringSize);
-          if (!substring){
-              return 0;
-          }
-      }
 
-    	memcpy(substring, scanPointer, copyLength);
+    // Set the initial lengths for comparison.
-    	substring[copyLength] = '\0';
+    size_t origin_len = strlen(origin_host);
-    	//printf("found %s \n", substring);
+    size_t host_len = strlen(host);
-    	if (ends_with(str, substring)) {
+
-          free(substring); substring = NULL;
+    // Look for port numbers.
-          return 1;
+    const char * origin_colon = strrchr(origin_host, ':');
-    	}
+    const char * host_colon = strrchr(host, ':');
-    	scanPointer = scanPointer + copyLength + 1; // assumption: only 1 character
+
+    // If the host contains a port number (indicated by a colon)...
+    if (host_colon != NULL) {
+        // If the host port number is 443, then accept the origin host if it
+        // does not have any port number under the assumption we already
+        // verified https://
+        if (strlen(host_colon) == 4 &&
+            strncmp(host_colon, ":443", 4) == 0 &&
+            origin_colon == NULL)
+        {
+            // We will ignore the host port number of 443.
+            host_len = host_colon - host;
+        }
+
+        // Other port numbers must match exactly. So leave the host length
+        // untouched.
     }
-    free(substring); substring = NULL;
+
-    return ends_with(str, scanPointer);
+    // Otherwise ignore any port number in the origin.
+    else if (origin_colon != NULL) {
+       origin_len = origin_colon - origin_host;
+    }
+
+    // At this point, the origin length excludes any port number if the host
+    // does not specify one, and the host length excludes its port number if
+    // it was 443 and there is no origin port number.
+    //
+    // If either length is zero then fail.
+    if (origin_len == 0 || host_len == 0)
+        return 0;
+
+    // Check to see if the host permits subdomains.
+    const char * wildcard = "*.";
+    const int acceptSubdomain = (host_len > strlen(wildcard))
+        ? strncmp(host, wildcard, strlen(wildcard)) == 0
+        : 0;
+
+    // If the host accepts subdomains, verify that the origin ends with the
+    // portion of the host that occurs after the subdomain wildcard.
+    if (acceptSubdomain) {
+        // The origin must be at least as long as the host.
+        if (origin_len < host_len)
+            return 0;
+
+        // Skip the subdomain of the origin, which should equate to the
+        // wildcard of the host. Likewise skip the wildcard of the host.
+        const char * origin_domain = strchr(origin_host, '.');
+        const char * host_domain = host + 1;
+        if (!origin_domain || !host_domain)
+            return 0;
+
+        // Remove from comparison the characters we skipped.
+        origin_len -= (origin_domain - origin_host);
+        host_len -= 1;
+
+        // The remainder must be an exact match.
+        return (origin_len == host_len &&
+                strncmp(origin_domain, host_domain, origin_len) == 0);
+    }
+
+    // Otherwise the host and origin must be an exact match.
+    return (origin_len == host_len &&
+            strncmp(origin_host, host, origin_len) == 0);
 }
 
-static int should_check_for_origin( char * origin ) {
+/**
-    const char * const CHECK_PROTOS[] = { "http:", "https:", "file:" };
+ * Returns true if the origin is acceptable based on the candidate value.
-    for (int i = 0; i < 3; ++i) {
+ * The origin must be an exact match to the candidate, unless the
-        if (!strncmp(origin, CHECK_PROTOS[i], strlen(CHECK_PROTOS[i]) - 1)) {
+ * candidate is of the form 'scheme://*' or 'scheme:*' in which case
+ * everything before the wildcard '*' character must be an exact match but
+ * anything is accepted in place of the wildcard.
+ *
+ * This function assumes that the candidate value is well-formed, meaning
+ * it will not include invalid chracters and it will be a valid URI.
+ *
+ * @param origin the origin header value.
+ * @param candidate the authorized origin value.
+ * @return true if accepted and false if not.
+ */
+static int origin_matches(const char *origin, const char *candidate) {
+    // Make sure there is something to compare.
+    if (!origin || !candidate)
+        return 0;
+
+    // If the candidate consists of a scheme followed by wildcard,
+    // require an exact match of the scheme specifier.
+    size_t origin_len = strlen(origin);
+    size_t candidate_len = strlen(candidate);
+    if (candidate_len > 1 && candidate[candidate_len - 1] == '*') {
+        // The origin must be at least as long as the candidate for a
+        // wildcard match to succeed.
+        if (origin_len < candidate_len)
+            return 0;
+
+        fprintf(stderr, "comparing %s to %s len %lld\n", origin, candidate, candidate_len);
+        return strncmp(origin, candidate, candidate_len - 1) == 0;
+    }
+
+    // Require an exact match.
+    return (origin_len == strlen(candidate) &&
+        strncmp(origin, candidate, origin_len) == 0);
+}
+
+// str contains a white space separated list of strings (only supports SPACE characters for now)
+static int is_uri_in_list(const char *origin, const char *list) {
+    // Make sure there is something to compare.
+    if (!origin || !list)
+        return 0;
+
+    int isHttps = (strncmp(origin, gHttpsProto, strlen(gHttpsProto)) == 0);
+    
+    const char * scanPointer = list;
+    const char * spacePointer;
+    unsigned int substringSize = 257;
+    char *candidate = (char *)malloc(substringSize);
+    if (!candidate) {
+        return 0;
+    }
+    while ((spacePointer = strchr(scanPointer, ' ')) != NULL) {
+        int copyLength = spacePointer - scanPointer;      
+      
+        // protect against buffer overflow
+        if (copyLength >= substringSize) {
+            substringSize = copyLength + 1;
+            free(candidate);
+            candidate = (char *)malloc(substringSize);
+            if (!candidate) {
+                return 0;
+            }
+        }
+
+        memcpy(candidate, scanPointer, copyLength);
+        candidate[copyLength] = '\0';
+        //printf("found %s \n", candidate);
+        // If the URI begins with https://, perform a host comparison because
+        // any port numbers must be handled specially. Otherwise perform a
+        // regular match.
+        if ((isHttps && host_matches(origin, candidate)) ||
+            (!isHttps && origin_matches(origin, candidate)))
+        {
+            free(candidate); candidate = NULL;
             return 1;
         }
+        scanPointer = scanPointer + copyLength + 1; // assumption: only 1 character
     }
-    return 0;
+    free(candidate); candidate = NULL;
+    return ((isHttps && host_matches(origin, scanPointer)) ||
+            (!isHttps && origin_matches(origin, scanPointer)));
 }
 
 static int is_allowed_origin(DIALServer* ds, char * origin, const char * app_name) {
-    if (!origin || strlen(origin)==0 || !should_check_for_origin(origin)) {
+    fprintf(stderr, "checking %s for %s\n", origin, app_name);
+
+    if (!origin || strlen(origin)==0) {
         return 1;
     }
-
+    
     if (!ds_lock(ds)) {
         // If we can't check, fail in favor of safety.
         return 0;
@@ -562,9 +695,9 @@ static int is_allowed_origin(DIALServer* ds, char * origin, const char * app_nam
     DIALApp *app;
     int result = 0;
     for (app = ds->apps; app != NULL; app = app->next) {
-    	if (!strcmp(app->name, app_name)) {
+        if (!strcmp(app->name, app_name)) {
             if (!app->corsAllowedOrigin[0] ||
-                ends_with_in_list(origin, app->corsAllowedOrigin)) {
+                is_uri_in_list(origin, app->corsAllowedOrigin)) {
                 result = 1;
                 break;
             }
@@ -798,6 +931,8 @@ int DIAL_register_app(DIALServer *ds, const char *app_name,
         app->corsAllowedOrigin[0] = '\0';
         if (corsAllowedOrigin && strlen(corsAllowedOrigin) < sizeof(app->corsAllowedOrigin)) {
             strcpy(app->corsAllowedOrigin, corsAllowedOrigin);
+        } else {
+            return -1;
         }
         *ptr = app;
         ds_unlock(ds);

server/main.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014 Netflix, Inc.
+ * Copyright (c) 2014-2020 Netflix, Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -288,8 +288,13 @@ void runDial(void)
     struct DIALAppCallbacks cb_yt = {youtube_start, youtube_hide, youtube_stop, youtube_status};
     struct DIALAppCallbacks cb_system = {system_start, system_hide, NULL, system_status};
 
-    if (DIAL_register_app(ds, "Netflix", &cb_nf, NULL, 1, ".netflix.com") == -1 ||
+#if defined DEBUG
-        DIAL_register_app(ds, "YouTube", &cb_yt, NULL, 1, ".youtube.com") == -1 ||
+    if (DIAL_register_app(ds, "Netflix", &cb_nf, NULL, 1, "https://netflix.com https://www.netflix.com https://port.netflix.com:123 proto://*") == -1 ||
+        DIAL_register_app(ds, "YouTube", &cb_yt, NULL, 1, "https://youtube.com https://www.youtube.com https://*.youtube.com:443 https://port.youtube.com:123 package:com.google.android.youtube package:com.google.ios.youtube proto:*") == -1 ||
+#else
+    if (DIAL_register_app(ds, "Netflix", &cb_nf, NULL, 1, "https://netflix.com https://www.netflix.com") == -1 ||
+        DIAL_register_app(ds, "YouTube", &cb_yt, NULL, 1, "https://youtube.com https://*.youtube.com package:*") == -1 ||
+#endif
         DIAL_register_app(ds, "system", &cb_system, NULL, 1, "") == -1)
     {
         printf("Unable to register DIAL applications.\n");

server/makefile

@@ -12,17 +12,17 @@ HEADERS := $(wildcard *.h)
 #	$(CC) -Wall -Werror -g -std=gnu99 $(CFLAGS) -c $*.c -o $*.o
 	$(CC) -Wall -g -fPIC -std=gnu99 $(CFLAGS) -c $*.c -o $*.o
 
-all: dialserver test
+all: dialserver
+debug: CFLAGS += -DDEBUG
+debug: dialserver test
 
 nf_callbacks_lib: nf_callbacks.o
 #	$(CC) -Wall -Werror -g nf_callbacks.o -o libnfCallbacks.so --shared
 	$(CC) -Wall -Werror -Wl,-undefined -Wl,dynamic_lookup -g nf_callbacks.o -o libnfCallbacks.so --shared
 
-
 dialserver: nf_callbacks_lib $(OBJS)
 	$(CC) -Wall -Werror -Wl,-rpath,. -g $(OBJS) -ldl -lpthread -lrt -L. -lnfCallbacks -o dialserver
 
-
 dialserver_with_ASAN: nf_callbacks_lib $(OBJS)
 	$(CC) -Wall -Werror  -fsanitize=address -Wl,-rpath,. -g $(OBJS) -ldl -lpthread -lrt -L. -lnfCallbacks -o dialserver_with_ASAN
 

server/tests/test_cors.html

@@ -17,7 +17,7 @@ and open the template in the editor.
               console.log("make dial post...");
               var ip = $("#ipAddress").val();
               var port = $("#dialPort").val();
-              var urlStr = "http://"+ip+":"+port+"/apps/"+app;
+              var urlStr = "https://"+ip+":"+port+"/apps/"+app;
               console.log(urlStr);
               $("#status").text("posted to "+urlStr);
               $.ajax({

server/tests/test_cors.sh

@@ -9,7 +9,7 @@ ip_address=$1
 port=$2
 
 #Testing all the positive cases
-origins="http://www4.netflix.com http://1.netflix.com https://www.netflix.com https://www4.netflix.com ftp://this.is.fine"
+origins="https://www.netflix.com https://netflix.com https://port.netflix.com:123 https://www.netflix.com:80 https://www.netflix.com:123 proto://netflix.com proto://netflix proto://netflix.com:123"
 for origin in $origins; do
 curl --fail --silent --header "Origin:$origin" --data "v=QH2-TGUlwu4"  http://$ip_address:$port/apps/Netflix || echo "failed: $origin should be accepted"
 curl --fail --silent --header "Origin:$origin" -X OPTIONS  http://$ip_address:$port/apps/Netflix || echo "failed: $origin should be accepted"
@@ -21,7 +21,7 @@ then
 fi
 done
 
-origins="http://www4.youtube.com http://1.youtube.com https://www.youtube.com https://www4.youtube.com ftp://this.is.fine"
+origins="https://www.youtube.com https://music.youtube.com https://youtube.com https://port.youtube.com:123 https://www.youtube.com:80 https://www.youtube.com:123 package:com.google.android.youtube package:com.google.ios.youtube proto:g proto:com.google"
 for origin in $origins; do
 curl --fail --silent --header "Origin:$origin" --data "v=QH2-TGUlwu4"  http://$ip_address:$port/apps/YouTube || echo "failed: $origin should be accepted"
 curl --fail --silent --header "Origin:$origin" -X OPTIONS  http://$ip_address:$port/apps/YouTube || echo "failed: $origin should be accepted"
@@ -34,7 +34,7 @@ fi
 done
 
 #Testing all the negative cases
-origins="http://www.netflix-a.com http://www.netflix.com4 http://a-netflix.com https://ww.netflix-a.com https://www.netflix.com4 https://a-netflix.com http://netflix.com http://www.attack.com https://www.attack.com file://www.attack.com"
+origins="http://www.netflix-a.com http://www.netflix.com4 http://a-netflix.com http://www4.netflix.com https://port.netflix.com:1234 http://1.netflix.com https://www4.netflix.com https://ww.netflix-a.com https://www.netflix.com4 https://a-netflix.com http://netflix.com http://www.attack.com https://www.attack.com file://www.attack.com ftp://this.is.not.fine package: package:com.netflix.null proto:// proto:n proto:/n proto"
 for origin in $origins; do
 curl --fail --silent --header "Origin:$origin" --data "v=QH2-TGUlwu4"  http://$ip_address:$port/apps/Netflix && echo "failed: $origin should be rejected"
 curl --fail --silent --header "Origin:$origin" -X OPTIONS  http://$ip_address:$port/apps/Netflix && echo "failed: $origin should be rejected"
@@ -46,7 +46,7 @@ then
 fi
 done
 
-origins="http://www.youtube-a.com http://www.youtube.com4 http://a-youtube.com https://ww.youtube-a.com https://www.youtube.com4 https://a-youtube.com http://youtube.com https://youtube.com http://www.attack.com https://www.attack.com file://www.attack.com"
+origins="http://www.youtube-a.com http://www.youtube.com4 https://.youtube.com http://a-youtube.com https://ww.youtube-a.com http://www4.youtube.com https://port.youtube.com:1234 http://1.youtube.com https://www.youtube.com4 https://a-youtube.com http://youtube.com http://www.attack.com https://www.attack.com file://www.attack.com ftp://this.is.not.fine packagecom.google.android.youtube package:com.google.android.utube packagea package: pack:com.google.android protoa proto:"
 for origin in $origins; do
 curl --fail --silent --header "Origin:$origin" --data "v=QH2-TGUlwu4"  http://$ip_address:$port/apps/YouTube && echo "failed: $origin should be rejected"
 curl --fail --silent --header "Origin:$origin" -X OPTIONS  http://$ip_address:$port/apps/YouTube && echo "failed: $origin should be rejected"