| Message ID | 20180316164059.9120-1-xiaoningning@yahoo.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [Openvpn-devel] Increase TLS Control Channel Buffer Size | expand |
Hi, On Fri, Mar 16, 2018 at 09:40:59AM -0700, Ning Wei via Openvpn-devel wrote: > Increase TLS_CHANNEL_BUF_SIZE to 4096. When the build is enabled with > ENABLE_PKCS11, the password field can be 4096. The old size of > TLS_CHANNEL_BUF_SIZE was 2048, which is not enough. Please do not keep re-sending this patch just because you did not get a response right away. This messes up our patch tracking in patchwork, and annoys those that will eventually find time to review it. Such a change might cause incompatibilities or overflows in unexpected ways, so a review is complex and not just anyone can do it - and those few that understand the openvpn protocol <-> tls interaction well enough are quite busy with other work. So, patience please. gert
Thank you for reminding this. The reason that I have resent was there was a typo in the commit comment.
Best regards,Ning
On Friday, March 16, 2018, 9:46:07 AM PDT, Gert Doering <gert@greenie.muc.de> wrote:
Hi,
On Fri, Mar 16, 2018 at 09:40:59AM -0700, Ning Wei via Openvpn-devel wrote:
> Increase TLS_CHANNEL_BUF_SIZE to 4096. When the build is enabled with
> ENABLE_PKCS11, the password field can be 4096. The old size of
> TLS_CHANNEL_BUF_SIZE was 2048, which is not enough.
Please do not keep re-sending this patch just because you did not get
a response right away. This messes up our patch tracking in patchwork,
and annoys those that will eventually find time to review it.
Such a change might cause incompatibilities or overflows in unexpected
ways, so a review is complex and not just anyone can do it - and those
few that understand the openvpn protocol <-> tls interaction well
enough are quite busy with other work.
So, patience please.
gert
Hi, Apologies in advance if I'm misreading the intent of this patch. On Fri, Mar 16, 2018 at 12:40 PM, Ning Wei via Openvpn-devel <openvpn-devel@lists.sourceforge.net> wrote: > Increase TLS_CHANNEL_BUF_SIZE to 4096. When the build is enabled with > ENABLE_PKCS11, the password field can be 4096. The old size of > TLS_CHANNEL_BUF_SIZE was 2048, which is not enough. I do not follow the logic behind this. Sure, ENABLE_PKCS11 increases the password buffer size to 4096, but pkcs11 password is only consumed locally, not sent out to the remote. Now, even if a auth-user-pass password or challenge response gets as large as 4096 bytes, there are other places in the code that need "fixing" to permit such long "passwords" (or response) -- e.g., it cannot be submitted via the management as the parser expects a max line length of 255 and word length of 255. Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a password that long as there are other things like username, key-source, local-options that have to fit into the buffer (see key_method_2_write). So what's the logic behind 4096? Finally, are there any particular challenge-response framework that requires such long responses? Even for U2F, we wouldn't need to send a response that large. U2F registration response could get very long (still mostly under 2000 bytes), but doing U2F key registration via the authentication channel is not a good implementation. Authentication responses are much smaller. In short, it looks premature to increase this value before we start supporting long responses via the management, and a real practical need for long challenge response strings that won't fit in the current 2048 bytes (minus overhead) arise. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Sorry, I did not state the intent clearly in the request. We wanted to use password field to pass authentication string, such as Token string,which can be long, the user auth plug in on the sever side can validate the token asthe part authentication step. Acquiring a token, such as JWT, usually involves user interaction with the token brokerservice provider. So it is better to do this on the client side. I agreed on the following statement. To support a long password field in TLS channels,the Buf size of 4096 is not enough in key_method_2_write. Should we propose to increase the buffer size further? "Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a password that long as there are other things like username, key-source, local-options that have to fit into the buffer (see key_method_2_write). So what's the logic behind 4096?" Regards,Ning On Friday, March 16, 2018, 10:30:45 AM PDT, Selva Nair <selva.nair@gmail.com> wrote: Hi, Apologies in advance if I'm misreading the intent of this patch. On Fri, Mar 16, 2018 at 12:40 PM, Ning Wei via Openvpn-devel <openvpn-devel@lists.sourceforge.net> wrote: > Increase TLS_CHANNEL_BUF_SIZE to 4096. When the build is enabled with > ENABLE_PKCS11, the password field can be 4096. The old size of > TLS_CHANNEL_BUF_SIZE was 2048, which is not enough. I do not follow the logic behind this. Sure, ENABLE_PKCS11 increases the password buffer size to 4096, but pkcs11 password is only consumed locally, not sent out to the remote. Now, even if a auth-user-pass password or challenge response gets as large as 4096 bytes, there are other places in the code that need "fixing" to permit such long "passwords" (or response) -- e.g., it cannot be submitted via the management as the parser expects a max line length of 255 and word length of 255. Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a password that long as there are other things like username, key-source, local-options that have to fit into the buffer (see key_method_2_write). So what's the logic behind 4096? Finally, are there any particular challenge-response framework that requires such long responses? Even for U2F, we wouldn't need to send a response that large. U2F registration response could get very long (still mostly under 2000 bytes), but doing U2F key registration via the authentication channel is not a good implementation. Authentication responses are much smaller. In short, it looks premature to increase this value before we start supporting long responses via the management, and a real practical need for long challenge response strings that won't fit in the current 2048 bytes (minus overhead) arise. Selva <html><head></head><body><div style="font-family:Helvetica Neue, Helvetica, Arial, sans-serif;font-size:13px;"><div></div> <div>Sorry, I did not state the intent clearly in the request.</div><div><br></div><div>We wanted to use password field to pass authentication string, such as Token string,</div><div>which can be long, the user auth plug in on the sever side can validate the token as</div><div>the part authentication step.</div><div><br></div><div>Acquiring a token, such as JWT, usually involves user interaction with the token broker</div><div>service provider. So it is better to do this on the client side.</div><div><br></div><div>I agreed on the following statement. To support a long password field in TLS channels,</div><div>the Buf size of 4096 is not enough in key_method_2_write. Should we propose to<br></div><div>increase the buffer size further?</div><div><br></div><div><div dir="ltr" style="font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;">"Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a<br></div><div dir="ltr" style="font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;">password that long as there are other things like username, key-source,<br></div><div dir="ltr" style="font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;">local-options that have to fit into the buffer (see key_method_2_write). So<br></div><div dir="ltr" style="font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;">what's the logic behind 4096?"</div><br></div><div>Regards,</div><div>Ning</div> <div id="yahoo_quoted_1779238704" class="yahoo_quoted"> <div style="font-family:'Helvetica Neue', Helvetica, Arial, sans-serif;font-size:13px;color:#26282a;"> <div> On Friday, March 16, 2018, 10:30:45 AM PDT, Selva Nair <selva.nair@gmail.com> wrote: </div> <div><br></div> <div><br></div> <div><div dir="ltr">Hi,<br></div><div dir="ltr"><br></div><div dir="ltr">Apologies in advance if I'm misreading the intent of this patch.<br></div><div dir="ltr"><br></div><div dir="ltr">On Fri, Mar 16, 2018 at 12:40 PM, Ning Wei via Openvpn-devel<br></div><div dir="ltr"><<a ymailto="mailto:openvpn-devel@lists.sourceforge.net" href="mailto:openvpn-devel@lists.sourceforge.net">openvpn-devel@lists.sourceforge.net</a>> wrote:<br></div><div dir="ltr">> Increase TLS_CHANNEL_BUF_SIZE to 4096. When the build is enabled with<br></div><div dir="ltr">> ENABLE_PKCS11, the password field can be 4096. The old size of<br></div><div dir="ltr">> TLS_CHANNEL_BUF_SIZE was 2048, which is not enough.<br></div><div dir="ltr"><br></div><div dir="ltr">I do not follow the logic behind this. Sure, ENABLE_PKCS11 increases the<br></div><div dir="ltr">password buffer size to 4096, but pkcs11 password is only consumed<br></div><div dir="ltr">locally, not sent out to the remote.<br></div><div dir="ltr"><br></div><div dir="ltr">Now, even if a auth-user-pass password or challenge response gets as<br></div><div dir="ltr">large as 4096 bytes, there are other places in the code that<br></div><div dir="ltr">need "fixing" to permit such long "passwords" (or response) -- e.g., it cannot<br></div><div dir="ltr">be submitted via the management as the parser expects a max line<br></div><div dir="ltr">length of 255 and word length of 255.<br></div><div dir="ltr"><br></div><div dir="ltr">Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a<br></div><div dir="ltr">password that long as there are other things like username, key-source,<br></div><div dir="ltr">local-options that have to fit into the buffer (see key_method_2_write). So<br></div><div dir="ltr">what's the logic behind 4096?<br></div><div dir="ltr"><br></div><div dir="ltr">Finally, are there any particular challenge-response framework that requires<br></div><div dir="ltr">such long responses? Even for U2F, we wouldn't need to send a response<br></div><div dir="ltr">that large. U2F registration response could get very long (still mostly under<br></div><div dir="ltr">2000 bytes), but doing U2F key registration via the authentication channel is<br></div><div dir="ltr">not a good implementation. Authentication responses are much smaller.<br></div><div dir="ltr"><br></div><div dir="ltr">In short, it looks premature to increase this value before we start<br></div><div dir="ltr">supporting long responses via the management, and a real practical<br></div><div dir="ltr">need for long challenge response strings that won't fit in the current<br></div><div dir="ltr">2048 bytes (minus overhead) arise.<br></div><div dir="ltr"><br></div><div dir="ltr">Selva<br></div></div> </div> </div></div></body></html> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Mon, Mar 19, 2018 at 5:00 PM, Ning Wei <xiaoningning@yahoo.com> wrote: > Sorry, I did not state the intent clearly in the request. > > We wanted to use password field to pass authentication string, such as Token > string, > which can be long, the user auth plug in on the sever side can validate the > token as > the part authentication step. > > Acquiring a token, such as JWT, usually involves user interaction with the > token broker > service provider. So it is better to do this on the client side. Its hard for me to imagine what authentication would need to pass more than ~1600 bytes (that's about the current limit based on the 2048 byte buffer). Unless you are trying to fit in data unrelated to auth in the token. The U2F registration case I mentioned earlier would be such an example. How large is the data you need to transfer? If its not purely authentication data (i.e., a challenge string + signature at most), sending it as password is not the right approach. If there is extra text included for convenience, pre-compressing may help. How do you plan to pass the "password" to openvpn client -- from stdin? The management i/f will not let you pass such a long password or username. > > I agreed on the following statement. To support a long password field in > TLS channels, > the Buf size of 4096 is not enough in key_method_2_write. Should we propose > to > increase the buffer size further? Increasing to 4096 looks marginally useful to me and will fit a reasonably sized user name (say < 128 bytes) and a largish challenge-response (upto ~3400 bytes) leaving space for other things. Beyond that is unlikely to be necessary. I mentioned the overhead in key_method_2_write only because your logic behind this increase didn't make sense to me. > > "Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a > password that long as there are other things like username, key-source, > local-options that have to fit into the buffer (see key_method_2_write). So > what's the logic behind 4096?" > > Regards, > Ning > On Friday, March 16, 2018, 10:30:45 AM PDT, Selva Nair > <selva.nair@gmail.com> wrote: > > > Hi, > > Apologies in advance if I'm misreading the intent of this patch. > > On Fri, Mar 16, 2018 at 12:40 PM, Ning Wei via Openvpn-devel > <openvpn-devel@lists.sourceforge.net> wrote: >> Increase TLS_CHANNEL_BUF_SIZE to 4096. When the build is enabled with >> ENABLE_PKCS11, the password field can be 4096. The old size of >> TLS_CHANNEL_BUF_SIZE was 2048, which is not enough. > > I do not follow the logic behind this. Sure, ENABLE_PKCS11 increases the > password buffer size to 4096, but pkcs11 password is only consumed > locally, not sent out to the remote. > > Now, even if a auth-user-pass password or challenge response gets as > large as 4096 bytes, there are other places in the code that > need "fixing" to permit such long "passwords" (or response) -- e.g., it > cannot > be submitted via the management as the parser expects a max line > length of 255 and word length of 255. > > Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a > password that long as there are other things like username, key-source, > local-options that have to fit into the buffer (see key_method_2_write). So > what's the logic behind 4096? > > Finally, are there any particular challenge-response framework that requires > such long responses? Even for U2F, we wouldn't need to send a response > that large. U2F registration response could get very long (still mostly > under > 2000 bytes), but doing U2F key registration via the authentication channel > is > not a good implementation. Authentication responses are much smaller. > > In short, it looks premature to increase this value before we start > supporting long responses via the management, and a real practical > need for long challenge response strings that won't fit in the current > 2048 bytes (minus overhead) arise. > > Selva Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Both of key_method_2_write and key_method_2_read take TLS_Channel_Bug_Size as buffer size. The current size, 2048 is not enough to read/write a long password response. I have notice the management interface has a much smaller than 2048 buffer size to read/write. Currently, if the management interface is not used, increasing tls channel buffer size will serve the need.
As token provider, the size of token can be bigger or smaller. Sometime, it has more than 1600 as a token. To accommodate that, a bigger buffer size will be needed.
ThanksNing
On Monday, March 19, 2018, 10:11:25 PM PDT, Selva Nair <selva.nair@gmail.com> wrote:
Hi,
On Mon, Mar 19, 2018 at 5:00 PM, Ning Wei <xiaoningning@yahoo.com> wrote:
> Sorry, I did not state the intent clearly in the request.
>
> We wanted to use password field to pass authentication string, such as Token
> string,
> which can be long, the user auth plug in on the sever side can validate the
> token as
> the part authentication step.
>
> Acquiring a token, such as JWT, usually involves user interaction with the
> token broker
> service provider. So it is better to do this on the client side.
Its hard for me to imagine what authentication would need to pass more
than ~1600 bytes (that's about the current limit based on the 2048
byte buffer).
Unless you are trying to fit in data unrelated to auth in the token. The U2F
registration case I mentioned earlier would be such an example.
How large is the data you need to transfer? If its not purely
authentication data
(i.e., a challenge string + signature at most), sending it as password is not
the right approach. If there is extra text included for convenience,
pre-compressing
may help.
How do you plan to pass the "password" to openvpn client -- from stdin? The
management i/f will not let you pass such a long password or username.
>
> I agreed on the following statement. To support a long password field in
> TLS channels,
> the Buf size of 4096 is not enough in key_method_2_write. Should we propose
> to
> increase the buffer size further?
Increasing to 4096 looks marginally useful to me and will fit a reasonably sized
user name (say < 128 bytes) and a largish challenge-response (upto ~3400 bytes)
leaving space for other things. Beyond that is unlikely to be necessary.
I mentioned the overhead in key_method_2_write only because your logic
behind this increase didn't make sense to me.
>
> "Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a
> password that long as there are other things like username, key-source,
> local-options that have to fit into the buffer (see key_method_2_write). So
> what's the logic behind 4096?"
>
> Regards,
> Ning
> On Friday, March 16, 2018, 10:30:45 AM PDT, Selva Nair
> <selva.nair@gmail.com> wrote:
>
>
> Hi,
>
> Apologies in advance if I'm misreading the intent of this patch.
>
> On Fri, Mar 16, 2018 at 12:40 PM, Ning Wei via Openvpn-devel
> <openvpn-devel@lists.sourceforge.net> wrote:
>> Increase TLS_CHANNEL_BUF_SIZE to 4096. When the build is enabled with
>> ENABLE_PKCS11, the password field can be 4096. The old size of
>> TLS_CHANNEL_BUF_SIZE was 2048, which is not enough.
>
> I do not follow the logic behind this. Sure, ENABLE_PKCS11 increases the
> password buffer size to 4096, but pkcs11 password is only consumed
> locally, not sent out to the remote.
>
> Now, even if a auth-user-pass password or challenge response gets as
> large as 4096 bytes, there are other places in the code that
> need "fixing" to permit such long "passwords" (or response) -- e.g., it
> cannot
> be submitted via the management as the parser expects a max line
> length of 255 and word length of 255.
>
> Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a
> password that long as there are other things like username, key-source,
> local-options that have to fit into the buffer (see key_method_2_write). So
> what's the logic behind 4096?
>
> Finally, are there any particular challenge-response framework that requires
> such long responses? Even for U2F, we wouldn't need to send a response
> that large. U2F registration response could get very long (still mostly
> under
> 2000 bytes), but doing U2F key registration via the authentication channel
> is
> not a good implementation. Authentication responses are much smaller.
>
> In short, it looks premature to increase this value before we start
> supporting long responses via the management, and a real practical
> need for long challenge response strings that won't fit in the current
> 2048 bytes (minus overhead) arise.
>
> Selva
Selva
<html><head></head><body><div style="font-family:Helvetica Neue, Helvetica, Arial, sans-serif;font-size:13px;"><div></div>
<div>Both of key_method_2_write and key_method_2_read take TLS_Channel_Bug_Size as buffer size. The current size, 2048 is not enough to read/write a long password response. I have notice the management interface has a much smaller than 2048 buffer size to read/write. Currently, if the management interface is not used, increasing tls channel buffer size will serve the need.</div><div><br></div><div>As token provider, the size of token can be bigger or smaller. Sometime, it has more than 1600 as a token. To accommodate that, a bigger buffer size will be needed.</div><div><br></div><div>Thanks</div><div>Ning</div><div><br></div>
<div id="yahoo_quoted_2420554380" class="yahoo_quoted">
<div style="font-family:'Helvetica Neue', Helvetica, Arial, sans-serif;font-size:13px;color:#26282a;">
<div>
On Monday, March 19, 2018, 10:11:25 PM PDT, Selva Nair <selva.nair@gmail.com> wrote:
</div>
<div><br></div>
<div><br></div>
<div><div dir="ltr">Hi,<br clear="none"><br clear="none">On Mon, Mar 19, 2018 at 5:00 PM, Ning Wei <<a shape="rect" ymailto="mailto:xiaoningning@yahoo.com" href="mailto:xiaoningning@yahoo.com">xiaoningning@yahoo.com</a>> wrote:<br clear="none">> Sorry, I did not state the intent clearly in the request.<br clear="none">><br clear="none">> We wanted to use password field to pass authentication string, such as Token<br clear="none">> string,<br clear="none">> which can be long, the user auth plug in on the sever side can validate the<br clear="none">> token as<br clear="none">> the part authentication step.<br clear="none">><br clear="none">> Acquiring a token, such as JWT, usually involves user interaction with the<br clear="none">> token broker<br clear="none">> service provider. So it is better to do this on the client side.<br clear="none"><br clear="none">Its hard for me to imagine what authentication would need to pass more<br clear="none">than ~1600 bytes (that's about the current limit based on the 2048<br clear="none">byte buffer).<br clear="none">Unless you are trying to fit in data unrelated to auth in the token. The U2F<br clear="none">registration case I mentioned earlier would be such an example.<br clear="none"><br clear="none">How large is the data you need to transfer? If its not purely<br clear="none">authentication data<br clear="none">(i.e., a challenge string + signature at most), sending it as password is not<br clear="none">the right approach. If there is extra text included for convenience,<br clear="none">pre-compressing<br clear="none">may help.<br clear="none"><br clear="none">How do you plan to pass the "password" to openvpn client -- from stdin? The<br clear="none">management i/f will not let you pass such a long password or username.<br clear="none"><br clear="none">><br clear="none">> I agreed on the following statement. To support a long password field in<br clear="none">> TLS channels,<br clear="none">> the Buf size of 4096 is not enough in key_method_2_write. Should we propose<br clear="none">> to<br clear="none">> increase the buffer size further?<br clear="none"><br clear="none">Increasing to 4096 looks marginally useful to me and will fit a reasonably sized<br clear="none">user name (say < 128 bytes) and a largish challenge-response (upto ~3400 bytes)<br clear="none">leaving space for other things. Beyond that is unlikely to be necessary.<br clear="none"><br clear="none">I mentioned the overhead in key_method_2_write only because your logic<br clear="none">behind this increase didn't make sense to me.<div class="yqt2558482608" id="yqtfd33968"><br clear="none"><br clear="none">><br clear="none">> "Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a<br clear="none">> password that long as there are other things like username, key-source,<br clear="none">> local-options that have to fit into the buffer (see key_method_2_write). So<br clear="none">> what's the logic behind 4096?"<br clear="none">><br clear="none">> Regards,<br clear="none">> Ning<br clear="none">> On Friday, March 16, 2018, 10:30:45 AM PDT, Selva Nair<br clear="none">> <<a shape="rect" ymailto="mailto:selva.nair@gmail.com" href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>> wrote:<br clear="none">><br clear="none">><br clear="none">> Hi,<br clear="none">><br clear="none">> Apologies in advance if I'm misreading the intent of this patch.<br clear="none">><br clear="none">> On Fri, Mar 16, 2018 at 12:40 PM, Ning Wei via Openvpn-devel<br clear="none">> <<a shape="rect" ymailto="mailto:openvpn-devel@lists.sourceforge.net" href="mailto:openvpn-devel@lists.sourceforge.net">openvpn-devel@lists.sourceforge.net</a>> wrote:<br clear="none">>> Increase TLS_CHANNEL_BUF_SIZE to 4096. When the build is enabled with<br clear="none">>> ENABLE_PKCS11, the password field can be 4096. The old size of<br clear="none">>> TLS_CHANNEL_BUF_SIZE was 2048, which is not enough.<br clear="none">><br clear="none">> I do not follow the logic behind this. Sure, ENABLE_PKCS11 increases the<br clear="none">> password buffer size to 4096, but pkcs11 password is only consumed<br clear="none">> locally, not sent out to the remote.<br clear="none">><br clear="none">> Now, even if a auth-user-pass password or challenge response gets as<br clear="none">> large as 4096 bytes, there are other places in the code that<br clear="none">> need "fixing" to permit such long "passwords" (or response) -- e.g., it<br clear="none">> cannot<br clear="none">> be submitted via the management as the parser expects a max line<br clear="none">> length of 255 and word length of 255.<br clear="none">><br clear="none">> Further, even a TLS_CHANNEL_BUF_SIZE of 4096 does not support a<br clear="none">> password that long as there are other things like username, key-source,<br clear="none">> local-options that have to fit into the buffer (see key_method_2_write). So<br clear="none">> what's the logic behind 4096?<br clear="none">><br clear="none">> Finally, are there any particular challenge-response framework that requires<br clear="none">> such long responses? Even for U2F, we wouldn't need to send a response<br clear="none">> that large. U2F registration response could get very long (still mostly<br clear="none">> under<br clear="none">> 2000 bytes), but doing U2F key registration via the authentication channel<br clear="none">> is<br clear="none">> not a good implementation. Authentication responses are much smaller.<br clear="none">><br clear="none">> In short, it looks premature to increase this value before we start<br clear="none">> supporting long responses via the management, and a real practical<br clear="none">> need for long challenge response strings that won't fit in the current<br clear="none">> 2048 bytes (minus overhead) arise.<br clear="none">><br clear="none">> Selva<br clear="none"><br clear="none">Selva<br clear="none"></div></div></div>
</div>
</div></div></body></html>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On 26-03-18 18:36, Ning Wei via Openvpn-devel wrote: > Both of key_method_2_write and key_method_2_read take > TLS_Channel_Bug_Size as buffer size. The current size, 2048 is not > enough to read/write a long password response. I have notice the > management interface has a much smaller than 2048 buffer size to > read/write. Currently, if the management interface is not used, > increasing tls channel buffer size will serve the need. > > As token provider, the size of token can be bigger or smaller. > Sometime, it has more than 1600 as a token. To accommodate that, a > bigger buffer size will be needed. I don't think this is achieving what you want. The username/password size on the OpenVPN protocol are fixed at 128 bytes max, and can not be changes without introducing a new protocol version or interoperability problems. See the USER_PASS_LEN variable used in key_method_2_read. So, NAK to this patch. -Steffan ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Wed, May 2, 2018 at 2:30 PM, Steffan Karger <steffan@karger.me> wrote: > Hi, > > On 26-03-18 18:36, Ning Wei via Openvpn-devel wrote: >> Both of key_method_2_write and key_method_2_read take >> TLS_Channel_Bug_Size as buffer size. The current size, 2048 is not >> enough to read/write a long password response. I have notice the >> management interface has a much smaller than 2048 buffer size to >> read/write. Currently, if the management interface is not used, >> increasing tls channel buffer size will serve the need. >> >> As token provider, the size of token can be bigger or smaller. >> Sometime, it has more than 1600 as a token. To accommodate that, a >> bigger buffer size will be needed. > > I don't think this is achieving what you want. The username/password > size on the OpenVPN protocol are fixed at 128 bytes max, and can not be > changes without introducing a new protocol version or interoperability > problems. See the USER_PASS_LEN variable used in key_method_2_read. USER_PASS_LEN is 4096 (not 128) for builds with ENABLE_PKCS11 and that was the assumption behind this patch. I'm not endorsing this patch but we do need changes to management interface and option parsing to allow long user/pass strings to support newer challenge-response protocols. Those changes are not hard but this patch falls short as I had pointed out earlier. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, Hi, On 02-05-18 20:40, Selva Nair wrote: > On Wed, May 2, 2018 at 2:30 PM, Steffan Karger <steffan@karger.me> wrote: >> On 26-03-18 18:36, Ning Wei via Openvpn-devel wrote: >>> Both of key_method_2_write and key_method_2_read take >>> TLS_Channel_Bug_Size as buffer size. The current size, 2048 is not >>> enough to read/write a long password response. I have notice the >>> management interface has a much smaller than 2048 buffer size to >>> read/write. Currently, if the management interface is not used, >>> increasing tls channel buffer size will serve the need. >>> >>> As token provider, the size of token can be bigger or smaller. >>> Sometime, it has more than 1600 as a token. To accommodate that, a >>> bigger buffer size will be needed. >> >> I don't think this is achieving what you want. The username/password >> size on the OpenVPN protocol are fixed at 128 bytes max, and can not be >> changes without introducing a new protocol version or interoperability >> problems. See the USER_PASS_LEN variable used in key_method_2_read. > > USER_PASS_LEN is 4096 (not 128) for builds with ENABLE_PKCS11 and that was > the assumption behind this patch. > > I'm not endorsing this patch but we do need changes to management interface and > option parsing to allow long user/pass strings to support newer > challenge-response > protocols. Those changes are not hard but this patch falls short as I > had pointed > out earlier. You're of course absolutely right. USER_PASS_LEN is not actually an interop issue here, the user/pass strings are not fixed length. Too quick with the answer, apologies. But I think changing TLS_CHANNEL_BUF_SIZE will result in tricky behaviour; when reading from the TLS channel, current implementations read upto 2048 bytes, and pass that to key_method_2_read() as if it was the whole message. If the user/pass exceeds the old max, that will result in truncated username/passwords, and finally password verification errors. It's not fatal, but sounds like really annoying to debug... -Steffan ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff --git a/src/openvpn/common.h b/src/openvpn/common.h index 4e6f4809..7e9b5f3a 100644 --- a/src/openvpn/common.h +++ b/src/openvpn/common.h @@ -75,7 +75,7 @@ typedef unsigned long ptr_type; * maximum size of a single TLS message (cleartext). * This parameter must be >= PUSH_BUNDLE_SIZE */ -#define TLS_CHANNEL_BUF_SIZE 2048 +#define TLS_CHANNEL_BUF_SIZE 4096 /* * This parameter controls the maximum size of a bundle
Increase TLS_CHANNEL_BUF_SIZE to 4096. When the build is enabled with ENABLE_PKCS11, the password field can be 4096. The old size of TLS_CHANNEL_BUF_SIZE was 2048, which is not enough. Signed-off-by: Ning Wei <xiaoningning@yahoo.com> --- src/openvpn/common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)