| Message ID | 20181214224007.54813-4-cpaasch@apple.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | tcp: Introduce a TFO key-pool for clean cookie-rotation | expand |
On 12/14/2018 02:40 PM, Christoph Paasch wrote: > Print the list of the TFO-keys with a comma separated. For setting the > keys, we still only allow a single one to be set. > I wonder if some applications expecting current format could break after a formatting change.
On Sun, Dec 16, 2018 at 10:32 PM Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > > On 12/14/2018 02:40 PM, Christoph Paasch wrote: > > Print the list of the TFO-keys with a comma separated. For setting the > > keys, we still only allow a single one to be set. > > > > I wonder if some applications expecting current format could break > after a formatting change. I have the same concern as well. print the extra keys in a different sysctl maybe? e.g. net.ipv4.tcp_fastopen_alt_keys >
On 17/12/18 - 08:52:22, Yuchung Cheng wrote: > On Sun, Dec 16, 2018 at 10:32 PM Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > > > > > > On 12/14/2018 02:40 PM, Christoph Paasch wrote: > > > Print the list of the TFO-keys with a comma separated. For setting the > > > keys, we still only allow a single one to be set. > > > > > > > I wonder if some applications expecting current format could break > > after a formatting change. > I have the same concern as well. print the extra keys in a different > sysctl maybe? e.g. net.ipv4.tcp_fastopen_alt_keys True, some apps might break on that. Having a single place where all the keys are shown is still useful as that way the key-rotation can simply check the current keys in one place. I'm fine with adding net.ipv4.tcp_fastopen_key_list or something like that, if we want to keep sysctl-API stable. Christoph
On Mon, Dec 17, 2018 at 3:35 PM Christoph Paasch <cpaasch@apple.com> wrote: > > On 17/12/18 - 08:52:22, Yuchung Cheng wrote: > > On Sun, Dec 16, 2018 at 10:32 PM Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > > > > > > > > > > On 12/14/2018 02:40 PM, Christoph Paasch wrote: > > > > Print the list of the TFO-keys with a comma separated. For setting the > > > > keys, we still only allow a single one to be set. > > > > > > > > > > I wonder if some applications expecting current format could break > > > after a formatting change. > > I have the same concern as well. print the extra keys in a different > > sysctl maybe? e.g. net.ipv4.tcp_fastopen_alt_keys > > True, some apps might break on that. > > > Having a single place where all the keys are shown is still useful as that > way the key-rotation can simply check the current keys in one place. That's a good point - I am neutral now to use your existing proposal. Eric? > > I'm fine with adding net.ipv4.tcp_fastopen_key_list or something like that, > if we want to keep sysctl-API stable. > > > Christoph >
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c index ba0fc4b18465..f0806bab5562 100644 --- a/net/ipv4/sysctl_net_ipv4.c +++ b/net/ipv4/sysctl_net_ipv4.c @@ -282,11 +282,15 @@ static int proc_tcp_fastopen_key(struct ctl_table *table, int write, { struct net *net = container_of(table->data, struct net, ipv4.sysctl_tcp_fastopen); - struct ctl_table tbl = { .maxlen = (TCP_FASTOPEN_KEY_LENGTH * 2 + 10) }; + /* maxlen to print the list of keys in hex (*2), with a comma + * in between (+ TCP_FASTOPEN_CTXT_LEN) + */ + struct ctl_table tbl = { .maxlen = (TCP_FASTOPEN_KEY_LENGTH * 2 * TCP_FASTOPEN_CTXT_LEN + + TCP_FASTOPEN_CTXT_LEN + 10) }; struct tcp_fastopen_context *ctxt; - u32 user_key[4]; /* 16 bytes, matching TCP_FASTOPEN_KEY_LENGTH */ - __le32 key[4]; - int ret, i; + u32 user_key[TCP_FASTOPEN_CTXT_LEN * 4]; + __le32 key[TCP_FASTOPEN_CTXT_LEN * 4]; + int ret, i = 0, off = 0; tbl.data = kmalloc(tbl.maxlen, GFP_KERNEL); if (!tbl.data) @@ -294,17 +298,28 @@ static int proc_tcp_fastopen_key(struct ctl_table *table, int write, rcu_read_lock(); ctxt = rcu_dereference(net->ipv4.tcp_fastopen_ctx); - if (ctxt) - memcpy(key, ctxt->key, TCP_FASTOPEN_KEY_LENGTH); - else - memset(key, 0, sizeof(key)); + while (ctxt) { + memcpy(&key[i], ctxt->key, TCP_FASTOPEN_KEY_LENGTH); + i += 4; + ctxt = rcu_dereference(ctxt->next); + } rcu_read_unlock(); + memset(&key[i], 0, sizeof(key) - i * sizeof(u32)); + for (i = 0; i < ARRAY_SIZE(key); i++) user_key[i] = le32_to_cpu(key[i]); - snprintf(tbl.data, tbl.maxlen, "%08x-%08x-%08x-%08x", - user_key[0], user_key[1], user_key[2], user_key[3]); + for (i = 0; i < TCP_FASTOPEN_CTXT_LEN; i++) { + off += snprintf(tbl.data + off, tbl.maxlen - off, + "%08x-%08x-%08x-%08x", + user_key[i * 4], + user_key[i * 4 + 1], + user_key[i * 4 + 2], + user_key[i * 4 + 3]); + if (i + 1 < TCP_FASTOPEN_CTXT_LEN) + off += snprintf(tbl.data + off, tbl.maxlen - off, ","); + } ret = proc_dostring(&tbl, write, buffer, lenp, ppos); if (write && ret == 0) { @@ -923,7 +938,11 @@ static struct ctl_table ipv4_net_table[] = { .procname = "tcp_fastopen_key", .mode = 0600, .data = &init_net.ipv4.sysctl_tcp_fastopen, - .maxlen = ((TCP_FASTOPEN_KEY_LENGTH * 2) + 10), + /* maxlen to print the list of keys in hex (*2), with a comma + * in between (+ TCP_FASTOPEN_CTXT_LEN) + */ + .maxlen = ((TCP_FASTOPEN_KEY_LENGTH * 2 * TCP_FASTOPEN_CTXT_LEN) + + TCP_FASTOPEN_CTXT_LEN + 10), .proc_handler = proc_tcp_fastopen_key, }, {
Print the list of the TFO-keys with a comma separated. For setting the keys, we still only allow a single one to be set. Signed-off-by: Christoph Paasch <cpaasch@apple.com> --- net/ipv4/sysctl_net_ipv4.c | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-)