| Message ID | 20220422134038.3801239-5-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Stateless three-way handshake and control channel improvements | expand |
Hi, On Fri, Apr 22, 2022 at 03:40:33PM +0200, Arne Schwabe wrote: > +/** > + * Move a packet authentication HMAC + related fields to or from the front > + * of the buffer so it can be processed by encrypt/decrypt. > + * > + * Turning the on wire format that starts with the opcode to a format > + * that starts with the hmac > + * e.g. "onwire" [opcode + packet id] [hmac] [remainder of packed] > + * > + * > + * "internal" [hmac] [opcode + packet id] [remainer of packet] > + * I was about to merge this as "it is easy, and does not change code", but if we add documentation, it should be correct :-) - and this schematic does not match my understanding of the actual code - which seems to swap [opcode + session id] [hmac + packet id] could you double check that? The size of the "things it swaps" is /* hmac + packet_id (8 bytes) */ const int hmac_size = hmac_ctx_size(ctx->hmac) + packet_id_size(true); /* opcode + session_id */ const int osid_size = 1 + SID_SIZE; ... so that would make it "[hmac + packet id]" not "[hmac]" and "[opcode + packet id]"...?! gert
Hi, On Fri, Apr 22, 2022 at 09:05:34PM +0200, Gert Doering wrote: > I was about to merge this as "it is easy, and does not change code", but > if we add documentation, it should be correct :-) - and this schematic > does not match my understanding of the actual code - which seems to > swap > > [opcode + session id] [hmac + packet id] > > could you double check that? The size of the "things it swaps" is Ah, found it. This comment gets changed in 05/28, to the values I also figured out. So, ACK on this one, will merge together with 05/ ASAPish (maybe I'll move the "fix comment hunk" from 05 to 04). gert
Acked-by: Gert Doering <gert@greenie.muc.de> I have taken the hunks from 05/28 that relate to swap_hmac (change the comment, add byte sizes to the "osid_size" comment) into this one, so this one is now factually correct, and 05(bis)/28 does no longer contain "swap_hmac" related changes. As this brings no code change, I've just verified that the comment matches my understanding of the function (it now does), not tested anything. Your patch has been applied to the master branch. commit e061ec5800af746cb86ccbb9b571e1cb18f8ad99 Author: Arne Schwabe Date: Fri Apr 22 15:40:33 2022 +0200 Add documentation for swap_hmac function Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20220422134038.3801239-5-arne@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24177.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 4ebf5acc2..f58f3b727 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1366,10 +1366,7 @@ tls_multi_free(struct tls_multi *multi, bool clear) } -/* - * Move a packet authentication HMAC + related fields to or from the front - * of the buffer so it can be processed by encrypt/decrypt. - */ + /* * Dependent on hmac size, opcode size, and session_id size. @@ -1377,6 +1374,23 @@ tls_multi_free(struct tls_multi *multi, bool clear) */ #define SWAP_BUF_SIZE 256 +/** + * Move a packet authentication HMAC + related fields to or from the front + * of the buffer so it can be processed by encrypt/decrypt. + * + * Turning the on wire format that starts with the opcode to a format + * that starts with the hmac + * e.g. "onwire" [opcode + packet id] [hmac] [remainder of packed] + * + * + * "internal" [hmac] [opcode + packet id] [remainer of packet] + * + * @param buf the buffer the swap operation is executed on + * @param incoming determines the direction of the swap + * @param co crypto options, determines the hmac to use in the swap + * + * @return if the swap was successful (buf was large enough) + */ static bool swap_hmac(struct buffer *buf, const struct crypto_options *co, bool incoming) {
Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/ssl.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)