* Tweaks to new H2Padding, now simply is of the form:

```H2Padding numbits``` and applies a *random* number of padding bytes to each payload frame. The range is from [0, 2^N[. The default is 0, so no padding.
icing · Mar 5, 2019 · e716f4b · e716f4b
1 parent a9f5d5c
commit e716f4b
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 97 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,10 @@
+v1.14.1
+--------------------------------------------------------------------------------
+ * Tweaks to new H2Padding, now simply is of the form:
+    ```H2Padding numbits```
+   and applies a *random* number of padding bytes to each payload frame. The
+   range is from [0, 2^N[. The default is 0, so no padding.
+
 v1.14.0
 --------------------------------------------------------------------------------
  * new configuration directive:

diff --git a/configure.ac b/configure.ac
@@ -14,7 +14,7 @@
 #
 
 AC_PREREQ([2.69])
-AC_INIT([mod_http2], [1.14.0], [[email protected]])
+AC_INIT([mod_http2], [1.14.1], [[email protected]])
 
 LT_PREREQ([2.2.6])
 LT_INIT()

diff --git a/mod_http2/h2_config.c b/mod_http2/h2_config.c
@@ -113,8 +113,8 @@ static h2_config defconf = {
     0,                      /* copy files across threads */
     NULL,                   /* push list */
     0,                      /* early hints, http status 103 */
-    4,                      /* padding bits */
-    0,                      /* padding always */
+    0,                      /* padding bits */
+    1,                      /* padding always */
 };
 
 static h2_dir_config defdconf = {
@@ -891,24 +891,11 @@ static const char *h2_conf_set_early_hints(cmd_parms *cmd,
     return NULL;
 }
 
-static const char *h2_conf_set_padding(cmd_parms *cmd, void *dirconf, 
-                                       const char *v1, const char *v2)
+static const char *h2_conf_set_padding(cmd_parms *cmd, void *dirconf, const char *value)
 {
     int val;
 
-    if (v2) {
-        if (v2 && !strcasecmp(v1, "prefer")) {
-            CONFIG_CMD_SET(cmd, dirconf, H2_CONF_PADDING_ALWAYS, 0);
-        }
-        else if (v2 && !strcasecmp(v1, "enforce")) {
-            CONFIG_CMD_SET(cmd, dirconf, H2_CONF_PADDING_ALWAYS, 1);
-        }
-        else {
-            return "[ 'enforce' | 'prefer' ] numbits";
-        }
-        v1 = v2;
-    }
-    val = (int)apr_atoi64(v1);
+    val = (int)apr_atoi64(value);
     if (val < 0) {
         return "number of bits must be >= 0";
     }
@@ -988,7 +975,7 @@ const command_rec h2_cmds[] = {
                    OR_FILEINFO|OR_AUTHCFG, "add a resource to be pushed in this location/on this server."),
     AP_INIT_TAKE1("H2EarlyHints", h2_conf_set_early_hints, NULL,
                   RSRC_CONF, "on to enable interim status 103 responses"),
-    AP_INIT_TAKE12("H2Padding", h2_conf_set_padding, NULL,
+    AP_INIT_TAKE1("H2Padding", h2_conf_set_padding, NULL,
                   RSRC_CONF, "set payload padding"),
     AP_END_CMD
 };

diff --git a/mod_http2/h2_session.c b/mod_http2/h2_session.c
@@ -629,23 +629,12 @@ static ssize_t select_padding_cb(nghttp2_session *ngh2,
     ssize_t frame_len = frame->hd.length + H2_FRAME_HDR_LEN; /* the total length without padding */
     ssize_t padded_len = frame_len;
 
-    /* Determine # of padding bytes to append to frame. We use different
-     * paddings for payload and meta frames, as the latter ones are usually shorter
-     * and may not contain as sensitive data.
-     * Up to 255 bytes of padding data are possible in HTTP/2.
-     * As a security feature, the usefulness of padding h2 frames depends on the overall
-     * protocol stack. See chapter 10.7 of RFC 7540.
-     * The implementation here is a fixed size approach: 
-     * - specify the number of bits the frame length shall be rounded to. E.g. 2 bits
-     *   makes the effective frame lengths multiples of 4.
-     * - unless padding is configured to happen always:
-     * - When a "write size" limit is in place, cap paddings to it. E.g. when a cap
-     *   of 1300 bytes is in place, do not exceed this by padding.
+    /* Determine # of padding bytes to append to frame. Unless session->padding_always
+     * the number my be capped by the ui.write_size that currently applies. 
      */
-    if (session->padding_mask) {
-        padded_len = H2MIN(max_payloadlen + H2_FRAME_HDR_LEN, 
-                           ((frame_len + session->padding_mask) 
-                            & ~session->padding_mask)); 
+    if (session->padding_max) {
+        int n = ap_random_pick(0, session->padding_max);
+        padded_len = H2MIN(max_payloadlen + H2_FRAME_HDR_LEN, frame_len + n); 
     }
 
     if (padded_len != frame_len) {
@@ -906,9 +895,9 @@ apr_status_t h2_session_create(h2_session **psession, conn_rec *c, request_rec *
     ap_add_input_filter("H2_IN", session->cin, r, c);
 
     h2_conn_io_init(&session->io, c, s);
-    session->padding_mask = h2_config_sgeti(s, H2_CONF_PADDING_BITS);
-    if (session->padding_mask) {
-        session->padding_mask = (0x01 << session->padding_mask) - 1; 
+    session->padding_max = h2_config_sgeti(s, H2_CONF_PADDING_BITS);
+    if (session->padding_max) {
+        session->padding_max = (0x01 << session->padding_max) - 1; 
     }
     session->padding_always = h2_config_sgeti(s, H2_CONF_PADDING_ALWAYS);
     session->bbtmp = apr_brigade_create(session->pool, c->bucket_alloc);

diff --git a/mod_http2/h2_session.h b/mod_http2/h2_session.h
@@ -85,7 +85,7 @@ typedef struct h2_session {
     struct h2_workers *workers;     /* for executing stream tasks */
     struct h2_filter_cin *cin;      /* connection input filter context */
     h2_conn_io io;                  /* io on httpd conn filters */
-    int padding_mask;               /* padding bit mask for frame length */
+    int padding_max;                /* max number of padding bytes */
     int padding_always;             /* padding has precedence over I/O optimizations */
     struct nghttp2_session *ngh2;   /* the nghttp2 session (internal use) */
 

diff --git a/mod_http2/h2_version.h b/mod_http2/h2_version.h
@@ -27,15 +27,15 @@
  * @macro
  * Version number of the http2 module as c string
  */
-#define MOD_HTTP2_VERSION "1.14.0-git"
+#define MOD_HTTP2_VERSION "1.14.1-git"
 
 /**
  * @macro
  * Numerical representation of the version number of the http2 module
  * release. This is a 24 bit number with 8 bits for major number, 8 bits
  * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
  */
-#define MOD_HTTP2_VERSION_NUM 0x010e00
+#define MOD_HTTP2_VERSION_NUM 0x010e01
 
 
 #endif /* mod_h2_h2_version_h */
diff --git a/test/e2e/test_104_padding.py b/test/e2e/test_104_padding.py
@@ -43,16 +43,6 @@ def setup_module(module):
     conf.add_line("H2Padding 8")
     conf.add_line("AddHandler cgi-script .py")
     conf.end_vhost()
-    conf.start_vhost( TestEnv.HTTPS_PORT, "pad8-pref", docRoot="htdocs/cgi", withSSL=True)
-    conf.add_line("Protocols h2 http/1.1")
-    conf.add_line("H2Padding prefer 8")
-    conf.add_line("AddHandler cgi-script .py")
-    conf.end_vhost()
-    conf.start_vhost( TestEnv.HTTPS_PORT, "pad8-force", docRoot="htdocs/cgi", withSSL=True)
-    conf.add_line("Protocols h2 http/1.1")
-    conf.add_line("H2Padding enforce 8")
-    conf.add_line("AddHandler cgi-script .py")
-    conf.end_vhost()
 
     conf.install()
     assert TestEnv.apache_restart() == 0
@@ -73,7 +63,7 @@ def setup_method(self, method):
     def teardown_method(self, method):
         print("teardown_method: %s" % method.__name__)
 
-    # default paddings settings: 4 bits
+    # default paddings settings: 0 bits
     def test_104_01(self):
         url = TestEnv.mkurl("https", "cgi", "/echo.py")
         # we get 2 frames back: one with data and an empty one with EOF
@@ -82,8 +72,8 @@ def test_104_01(self):
             r = r = TestEnv.nghttp().post_data(url, data, 5)
             assert 200 == r["response"]["status"]
             assert r["paddings"] == [ 
-                frame_padding(len(data)+1, 4), 
-                frame_padding(0, 4)
+                frame_padding(len(data)+1, 0), 
+                frame_padding(0, 0)
             ]
 
     # 0 bits of padding
@@ -100,68 +90,33 @@ def test_104_03(self):
         for data in [ "x", "xx", "xxx", "xxxx", "xxxxx", "xxxxxx", "xxxxxxx", "xxxxxxxx" ]:
             r = r = TestEnv.nghttp().post_data(url, data, 5)
             assert 200 == r["response"]["status"]
-            assert r["paddings"] == [ 
-                frame_padding(len(data)+1, 1), 
-                frame_padding(0, 1)
-            ]
+            for i in r["paddings"]:
+                assert i in range(0, 2)
 
     # 2 bits of padding
     def test_104_04(self):
         url = TestEnv.mkurl("https", "pad2", "/echo.py")
         for data in [ "x", "xx", "xxx", "xxxx", "xxxxx", "xxxxxx", "xxxxxxx", "xxxxxxxx" ]:
             r = r = TestEnv.nghttp().post_data(url, data, 5)
             assert 200 == r["response"]["status"]
-            assert r["paddings"] == [ 
-                frame_padding(len(data)+1, 2), 
-                frame_padding(0, 2)
-            ]
+            for i in r["paddings"]:
+                assert i in range(0, 4)
 
     # 3 bits of padding
     def test_104_05(self):
         url = TestEnv.mkurl("https", "pad3", "/echo.py")
         for data in [ "x", "xx", "xxx", "xxxx", "xxxxx", "xxxxxx", "xxxxxxx", "xxxxxxxx" ]:
             r = r = TestEnv.nghttp().post_data(url, data, 5)
             assert 200 == r["response"]["status"]
-            assert r["paddings"] == [ 
-                frame_padding(len(data)+1, 3), 
-                frame_padding(0, 3)
-            ]
+            for i in r["paddings"]:
+                assert i in range(0, 8)
 
     # 8 bits of padding
     def test_104_06(self):
         url = TestEnv.mkurl("https", "pad8", "/echo.py")
         for data in [ "x", "xx", "xxx", "xxxx", "xxxxx", "xxxxxx", "xxxxxxx", "xxxxxxxx" ]:
             r = r = TestEnv.nghttp().post_data(url, data, 5)
             assert 200 == r["response"]["status"]
-            assert r["paddings"] == [ 
-                frame_padding(len(data)+1, 8), 
-                frame_padding(0, 8)
-            ]
-
-    # 8 bits of padding, prefer
-    def test_104_10(self):
-        url = TestEnv.mkurl("https", "pad8-pref", "/echo.py")
-        # h2 starts with frams of ~1300 bytes length in early connections
-        # padding adapts to that restriction, if we get a response body of 1281
-        # bytes, and have 9 bytes frame header, so a 1290 frame, we would normally
-        # add 246 bytes of padding. BUT, restricted to 1300, we just add 10
-        data = "0123456789" * 128
-        r = r = TestEnv.nghttp().post_data(url, data, 5)
-        assert 200 == r["response"]["status"]
-        assert r["paddings"] == [ 
-            1300 - (len(data)+1+9), 
-            frame_padding(0, 8)
-        ]
-
-    # 8 bits of padding, enforce
-    def test_104_11(self):
-        url = TestEnv.mkurl("https", "pad8-force", "/echo.py")
-        # 'enforce' overrides any restrictions on IO size and pads as expected
-        data = "0123456789" * 128
-        r = r = TestEnv.nghttp().post_data(url, data, 5)
-        assert 200 == r["response"]["status"]
-        assert r["paddings"] == [ 
-            frame_padding(len(data)+1, 8), 
-            frame_padding(0, 8)
-        ]
+            for i in r["paddings"]:
+                assert i in range(0, 256)