| Message ID | 20171111160359.28078-1-steffan@karger.me |
|---|---|
| State | Superseded |
| Delegated to: | Antonio Quartulli |
| Headers | show |
| Series | [Openvpn-devel] Use P_DATA_V2 for server->client packets too | expand |
Hi, On 12/11/17 00:03, Steffan Karger wrote: > P_DATA_V2 introduced the peer-id. This allows clients to float, but as a > side-effect 32-bit aligns the encrypted data. That alignment improves > performance particularly on cheaper/older CPUs. So although servers don't > actually have a peer-id, still use the V2 packet format (with a zero-id) > for server->client traffic too. > > Signed-off-by: Steffan Karger <steffan@karger.me> > --- > src/openvpn/forward.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c > index 1b7455bb..20e89d34 100644 > --- a/src/openvpn/forward.c > +++ b/src/openvpn/forward.c > @@ -496,7 +496,7 @@ encrypt_sign(struct context *c, bool comp_frag) > /* If using P_DATA_V2, prepend the 1-byte opcode and 3-byte peer-id to the > * packet before openvpn_encrypt(), so we can authenticate the opcode too. > */ > - if (c->c2.buf.len > 0 && !c->c2.tls_multi->opt.server && c->c2.tls_multi->use_peer_id) > + if (c->c2.buf.len > 0 && c->c2.tls_multi->use_peer_id) > { > tls_prepend_opcode_v2(c->c2.tls_multi, &b->encrypt_buf); > } NAK. Although the patch looks good and easy, it is actually missing an important part: c->c2.tls_multi->use_peer_id is never set to true on the server. This should probably happen after parsing "IV_PROTO=" in prepare_push_reply(). Cheers,
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 1b7455bb..20e89d34 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -496,7 +496,7 @@ encrypt_sign(struct context *c, bool comp_frag) /* If using P_DATA_V2, prepend the 1-byte opcode and 3-byte peer-id to the * packet before openvpn_encrypt(), so we can authenticate the opcode too. */ - if (c->c2.buf.len > 0 && !c->c2.tls_multi->opt.server && c->c2.tls_multi->use_peer_id) + if (c->c2.buf.len > 0 && c->c2.tls_multi->use_peer_id) { tls_prepend_opcode_v2(c->c2.tls_multi, &b->encrypt_buf); }
P_DATA_V2 introduced the peer-id. This allows clients to float, but as a side-effect 32-bit aligns the encrypted data. That alignment improves performance particularly on cheaper/older CPUs. So although servers don't actually have a peer-id, still use the V2 packet format (with a zero-id) for server->client traffic too. Signed-off-by: Steffan Karger <steffan@karger.me> --- src/openvpn/forward.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)