Hi! > -----Original Message----- > From: Gert Doering [mailto:g...@greenie.muc.de] > Sent: maandag 12 december 2022 13:03 > To: Maximilian Fillinger <maximilian.fillin...@foxcrypto.com> > Cc: openvpn-devel@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH 2/2] Fix message for too long tls- > crypt-v2 metadata > > Hi, > > On Sat, Nov 26, 2022 at 05:26:48PM +0100, Max Fillinger wrote: > > The current code only checks if the base64-encoded metadata is at most > > 980 characters. However, that can encode up to 735 bytes of data, > while > > only up to 733 bytes are allowed. When passing 734 or 735 bytes, > openvpn > > prints a misleading error message saying that the base64 cannot be > > decoded. > > > > This patch checks the decoded length to show an accurate error > message. > > This patch looks overly complex to me for "change 735 to 733" - could > you (or Arne) explain the change a bit better?
What's going on is that the tls-crypt-v2 metadata can be at most 733 bytes. But the metadata is supplied in base64. If you want to encode 733 bytes in base64, you need 980 characters. Right now, openvpn just checks that we have at most 980 base64 characters and then tries to decode them into a 733 byte buffer. But 980 characters of base64 can encode up to 735 bytes. In that case, openvpn gives a fatal error about being unable to decode the base64 which I find misleading. My patch always allocates a large enough buffer to decode the base64 and checks that the decoded length is <= 733. An alternative would be to check the base64 length, decode to a 735 bytes buffer, then check the decoded length. I thought it's cleaner to have one length check, but I don't have a strong opinion about this. _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
