Outlaw illegal metadata characters

10 files changed, 163 insertions, 7 deletions

diff --git a/Makefile b/Makefile
index 31628b4412..f7ace3186a 100644
--- a/Makefile
+++ b/Makefile
@@ -793,6 +793,7 @@ fling_server: $(BINDIR)/$(CONFIG)/fling_server
 fling_stream_test: $(BINDIR)/$(CONFIG)/fling_stream_test
 fling_test: $(BINDIR)/$(CONFIG)/fling_test
 gen_hpack_tables: $(BINDIR)/$(CONFIG)/gen_hpack_tables
+gen_legal_metadata_characters: $(BINDIR)/$(CONFIG)/gen_legal_metadata_characters
 gpr_cmdline_test: $(BINDIR)/$(CONFIG)/gpr_cmdline_test
 gpr_env_test: $(BINDIR)/$(CONFIG)/gpr_env_test
 gpr_file_test: $(BINDIR)/$(CONFIG)/gpr_file_test
@@ -3386,7 +3387,7 @@ test_python: static_c
 tools: tools_c tools_cxx
 
 
-tools_c: privatelibs_c $(BINDIR)/$(CONFIG)/gen_hpack_tables $(BINDIR)/$(CONFIG)/grpc_create_jwt $(BINDIR)/$(CONFIG)/grpc_fetch_oauth2 $(BINDIR)/$(CONFIG)/grpc_print_google_default_creds_token $(BINDIR)/$(CONFIG)/grpc_verify_jwt
+tools_c: privatelibs_c $(BINDIR)/$(CONFIG)/gen_hpack_tables $(BINDIR)/$(CONFIG)/gen_legal_metadata_characters $(BINDIR)/$(CONFIG)/grpc_create_jwt $(BINDIR)/$(CONFIG)/grpc_fetch_oauth2 $(BINDIR)/$(CONFIG)/grpc_print_google_default_creds_token $(BINDIR)/$(CONFIG)/grpc_verify_jwt
 
 tools_cxx: privatelibs_cxx
 
@@ -7122,6 +7123,35 @@ endif
 endif
 
 
+GEN_LEGAL_METADATA_CHARACTERS_SRC = \
+    tools/codegen/core/gen_legal_metadata_characters.c \
+
+GEN_LEGAL_METADATA_CHARACTERS_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(GEN_LEGAL_METADATA_CHARACTERS_SRC))))
+ifeq ($(NO_SECURE),true)
+
+# You can't build secure targets if you don't have OpenSSL.
+
+$(BINDIR)/$(CONFIG)/gen_legal_metadata_characters: openssl_dep_error
+
+else
+
+$(BINDIR)/$(CONFIG)/gen_legal_metadata_characters: $(GEN_LEGAL_METADATA_CHARACTERS_OBJS) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libgrpc.a
+	$(E) "[LD]      Linking $@"
+	$(Q) mkdir -p `dirname $@`
+	$(Q) $(LD) $(LDFLAGS) $(GEN_LEGAL_METADATA_CHARACTERS_OBJS) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/gen_legal_metadata_characters
+
+endif
+
+$(OBJDIR)/$(CONFIG)/tools/codegen/core/gen_legal_metadata_characters.o:  $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libgrpc.a
+deps_gen_legal_metadata_characters: $(GEN_LEGAL_METADATA_CHARACTERS_OBJS:.o=.dep)
+
+ifneq ($(NO_SECURE),true)
+ifneq ($(NO_DEPS),true)
+-include $(GEN_LEGAL_METADATA_CHARACTERS_OBJS:.o=.dep)
+endif
+endif
+
+
 GPR_CMDLINE_TEST_SRC = \
     test/core/support/cmdline_test.c \
 
diff --git a/build.json b/build.json
index bd707d2e34..c6a670970b 100644
--- a/build.json
+++ b/build.json
@@ -1144,6 +1144,16 @@
       ]
     },
     {
+      "name": "gen_legal_metadata_characters",
+      "build": "tool",
+      "language": "c",
+      "src": [
+        "tools/codegen/core/gen_legal_metadata_characters.c"
+      ],
+      "deps": [
+      ]
+    },
+    {
       "name": "gpr_cmdline_test",
       "build": "test",
       "language": "c",
diff --git a/src/core/channel/compress_filter.h b/src/core/channel/compress_filter.h
index 0917e81ca4..415459bca6 100644
--- a/src/core/channel/compress_filter.h
+++ b/src/core/channel/compress_filter.h
@@ -36,7 +36,7 @@
 
 #include "src/core/channel/channel_stack.h"
 
-#define GRPC_COMPRESS_REQUEST_ALGORITHM_KEY "internal:grpc-encoding-request"
+#define GRPC_COMPRESS_REQUEST_ALGORITHM_KEY "grpc-internal-encoding-request"
 
 /** Compression filter for outgoing data.
  *
diff --git a/src/core/surface/call.c b/src/core/surface/call.c
index 33f277da46..4426bbbce9 100644
--- a/src/core/surface/call.c
+++ b/src/core/surface/call.c
@@ -1046,10 +1046,11 @@ static int prepare_application_metadata(grpc_call *call, size_t count,
                                                (const gpr_uint8 *)md->value,
                                                md->value_length, 1);
     if (!grpc_mdstr_is_legal_header(l->md->key)) {
-      gpr_log(GPR_ERROR, "attempt to send invalid metadata key");
+      gpr_log(GPR_ERROR, "attempt to send invalid metadata key: %s",
+              grpc_mdstr_as_c_string(l->md->key));
       return 0;
     } else if (!grpc_mdstr_is_bin_suffixed(l->md->key) &&
-               !grpc_mdstr_is_legal_header(l->md->value)) {
+               !grpc_mdstr_is_legal_nonbin_header(l->md->value)) {
       gpr_log(GPR_ERROR, "attempt to send invalid metadata value");
       return 0;
     }
diff --git a/src/core/transport/metadata.c b/src/core/transport/metadata.c
index f92e87e9dd..d758351feb 100644
--- a/src/core/transport/metadata.c
+++ b/src/core/transport/metadata.c
@@ -681,16 +681,36 @@ void grpc_mdctx_locked_mdelem_unref(grpc_mdctx *ctx,
 
 void grpc_mdctx_unlock(grpc_mdctx *ctx) { unlock(ctx); }
 
-int grpc_mdstr_is_legal_header(grpc_mdstr *s) {
-  /* TODO(ctiller): consider caching this, or computing it on construction */
+static int conforms_to(grpc_mdstr *s, const gpr_uint8 *legal_bits) {
   const gpr_uint8 *p = GPR_SLICE_START_PTR(s->slice);
   const gpr_uint8 *e = GPR_SLICE_END_PTR(s->slice);
   for (; p != e; p++) {
-    if (*p < 32 || *p > 126) return 0;
+    int idx = *p;
+    int byte = idx / 8;
+    int bit = idx % 8;
+    if ((legal_bits[byte] & (1 << bit)) == 0) return 0;
   }
   return 1;
 }
 
+int grpc_mdstr_is_legal_header(grpc_mdstr *s) {
+  static const gpr_uint8 legal_header_bits[256 / 8] = {
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0xff, 0x03, 0xfe, 0xff, 0xff,
+      0x07, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+  /* TODO(ctiller): consider caching this, or computing it on construction */
+  return conforms_to(s, legal_header_bits);
+}
+
+int grpc_mdstr_is_legal_nonbin_header(grpc_mdstr *s) {
+  static const gpr_uint8 legal_header_bits[256 / 8] = {
+      0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+      0xff, 0xff, 0xff, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+  return conforms_to(s, legal_header_bits);
+}
+
 int grpc_mdstr_is_bin_suffixed(grpc_mdstr *s) {
   /* TODO(ctiller): consider caching this */
   return grpc_is_binary_header((const char *)GPR_SLICE_START_PTR(s->slice),
diff --git a/src/core/transport/metadata.h b/src/core/transport/metadata.h
index a7af49ba55..eb17747be7 100644
--- a/src/core/transport/metadata.h
+++ b/src/core/transport/metadata.h
@@ -154,6 +154,7 @@ void grpc_mdelem_unref(grpc_mdelem *md);
 const char *grpc_mdstr_as_c_string(grpc_mdstr *s);
 
 int grpc_mdstr_is_legal_header(grpc_mdstr *s);
+int grpc_mdstr_is_legal_nonbin_header(grpc_mdstr *s);
 int grpc_mdstr_is_bin_suffixed(grpc_mdstr *s);
 
 /* Batch mode metadata functions.
diff --git a/tools/codegen/core/gen_legal_metadata_characters.c b/tools/codegen/core/gen_legal_metadata_characters.c
new file mode 100644
index 0000000000..5e32d91d50
--- /dev/null
+++ b/tools/codegen/core/gen_legal_metadata_characters.c
@@ -0,0 +1,73 @@
+/*
+ *
+ * Copyright 2015, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+/* generates constant table for metadata.c */
+
+#include <stdio.h>
+#include <string.h>
+
+static unsigned char legal_bits[256 / 8];
+
+static void legal(int x) {
+  int byte = x / 8;
+  int bit = x % 8;
+  legal_bits[byte] |= 1 << bit;
+}
+
+static void dump(void) {
+  int i;
+
+  printf("static const gpr_uint8 legal_header_bits[256/8] = ");
+  for (i = 0; i < 256 / 8; i++)
+    printf("%c 0x%02x", i ? ',' : '{', legal_bits[i]);
+  printf(" };\n");
+}
+
+static void clear(void) { memset(legal_bits, 0, sizeof(legal_bits)); }
+
+int main(void) {
+  int i;
+
+  clear();
+  for (i = 'a'; i <= 'z'; i++) legal(i);
+  for (i = 'A'; i <= 'Z'; i++) legal(i);
+  for (i = '0'; i <= '9'; i++) legal(i);
+  legal('-');
+  dump();
+
+  clear();
+  for (i = 32; i <= 126; i++) legal(i);
+  dump();
+
+  return 0;
+}
diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json
index 50f078586d..142c84901d 100644
--- a/tools/run_tests/sources_and_headers.json
+++ b/tools/run_tests/sources_and_headers.json
@@ -240,6 +240,18 @@
   {
     "deps": [
       "gpr", 
+      "grpc"
+    ], 
+    "headers": [], 
+    "language": "c", 
+    "name": "gen_legal_metadata_characters", 
+    "src": [
+      "tools/codegen/core/gen_legal_metadata_characters.c"
+    ]
+  }, 
+  {
+    "deps": [
+      "gpr", 
       "gpr_test_util"
     ], 
     "headers": [], 
diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json
index 6c06d74834..127b1dfc40 100644
--- a/tools/run_tests/tests.json
+++ b/tools/run_tests/tests.json
@@ -1538,6 +1538,7 @@
       "posix", 
       "windows"
     ], 
+    "exclude_configs": [], 
     "flaky": false, 
     "language": "c++", 
     "name": "status_test", 
diff --git a/vsprojects/Grpc.mak b/vsprojects/Grpc.mak
index 662de784f7..3d853d9c76 100644
--- a/vsprojects/Grpc.mak
+++ b/vsprojects/Grpc.mak
@@ -183,6 +183,14 @@ gen_hpack_tables: gen_hpack_tables.exe
 	echo Running gen_hpack_tables
 	$(OUT_DIR)\gen_hpack_tables.exe
 
+gen_legal_metadata_characters.exe: build_gpr build_grpc $(OUT_DIR)
+	echo Building gen_legal_metadata_characters
+	$(CC) $(CFLAGS) /Fo:$(OUT_DIR)\ $(REPO_ROOT)\tools\codegen\core\gen_legal_metadata_characters.c 
+	$(LINK) $(LFLAGS) /OUT:"$(OUT_DIR)\gen_legal_metadata_characters.exe" Debug\gpr.lib Debug\grpc.lib $(LIBS) $(OUT_DIR)\gen_legal_metadata_characters.obj 
+gen_legal_metadata_characters: gen_legal_metadata_characters.exe
+	echo Running gen_legal_metadata_characters
+	$(OUT_DIR)\gen_legal_metadata_characters.exe
+
 gpr_cmdline_test.exe: build_gpr_test_util build_gpr $(OUT_DIR)
 	echo Building gpr_cmdline_test
 	$(CC) $(CFLAGS) /Fo:$(OUT_DIR)\ $(REPO_ROOT)\test\core\support\cmdline_test.c