naviserver-devel

Re: [naviserver-devel] modules/nsssl nsssl.c,1.6,1.7

[Stephen D. <sd...@gm...>]

On Wed, Oct 22, 2008 at 4:23 AM, Vlad Seryakov
<ser...@us...> wrote:
> Update of /cvsroot/naviserver/modules/nsssl
> In directory fdv4jf1.ch3.sourceforge.com:/tmp/cvs-serv22913/modules/nsssl
>
> Modified Files:
>        nsssl.c
> Log Message:
> new accept status, SSL driver works now
>
>
> Index: nsssl.c
> ===================================================================
> *** 286,298 ****
>
>          if (sslPtr == NULL) {
> !             sslPtr = SSL_new(drvPtr->ctx);
>              if (sslPtr == NULL) {
>                  Ns_Log(Error, "%d: SSL session init error for %s: [%s]", sock->sock, ns_inet_ntoa(sock->sa.sin_addr), strerror(errno));
>                  return NS_DRIVER_ACCEPT_ERROR;
>              }
>              sock->arg = sslPtr;
> !             SSL_set_fd(sslPtr, sock->sock);
> !             SSL_set_accept_state(sslPtr);
> !
>          }
>          return NS_DRIVER_ACCEPT_DATA;
> --- 293,306 ----
>
>          if (sslPtr == NULL) {
> !             sslPtr = ns_calloc(1, sizeof(SSLContext));
> !             sslPtr->ssl = SSL_new(drvPtr->ctx);
>              if (sslPtr == NULL) {
>                  Ns_Log(Error, "%d: SSL session init error for %s: [%s]", sock->sock, ns_inet_ntoa(sock->sa.sin_addr), strerror(errno));
> +                 ns_free(sslPtr);
>                  return NS_DRIVER_ACCEPT_ERROR;
>              }
>              sock->arg = sslPtr;
> !             SSL_set_fd(sslPtr->ssl, sock->sock);
> !             SSL_set_accept_state(sslPtr->ssl);
>          }
>          return NS_DRIVER_ACCEPT_DATA;
> ***************


The check for (sslPtr == NULL) happens after it's already been used.

Re: [naviserver-devel] modules/nsssl nsssl.c,1.6,1.7

[Stephen D. <sd...@gm...>]

> Index: nsssl.c
> ===================================================================
> RCS file: /cvsroot/naviserver/modules/nsssl/nsssl.c,v
> retrieving revision 1.6
> retrieving revision 1.7
> diff -C2 -d -r1.6 -r1.7
> *** nsssl.c     22 Oct 2008 02:14:24 -0000      1.6
> --- nsssl.c     22 Oct 2008 03:23:06 -0000      1.7
> ***************
> --- 432,447 ----
>   */
>
> ! static int
> ! SendBufs(SOCKET sock, struct iovec *bufs, int nbufs, Ns_Time *timeoutPtr, int flags)
>  {
> !     Ns_Conn *conn = Ns_GetConn();
> !
> !     return Ns_ConnSend(conn, bufs, nbufs);
> ! }
> !


This should be Ns_SockSendBufs(), otherwise connPtr->nContentSent will
be double counted and the driver thread may block.

Although in this case it should actually be Send(), which is your SSL
specific implementation of Ns_SockSendBufs(). The way it is now static
files may be sent in the clear :-/


(Basically, you implement the SendFile() callback by calling
Ns_SockSendFileIndirect(), which is the default library implementation
which scrapes the bytes off disk, and pass it Send(), which is your
implementation of writing to the network.  nssock is maybe a little
confusing in that it doesn't actually do anything but call the default
implementations, because it doesn't have to...)

Re: [naviserver-devel] modules/nsssl nsssl.c,1.6,1.7

[Vlad S. <vl...@cr...>]

I tried to use Ns_SockSendBufs() but it sends data directly to socket 
where i need to send via driver's send proc.

Why connPtr->nContentSent is not updated, all send called with conn 
parameters should do this

Stephen Deasey wrote:
>> Index: nsssl.c
>> ===================================================================
>> RCS file: /cvsroot/naviserver/modules/nsssl/nsssl.c,v
>> retrieving revision 1.6
>> retrieving revision 1.7
>> diff -C2 -d -r1.6 -r1.7
>> *** nsssl.c     22 Oct 2008 02:14:24 -0000      1.6
>> --- nsssl.c     22 Oct 2008 03:23:06 -0000      1.7
>> ***************
>> --- 432,447 ----
>>   */
>>
>> ! static int
>> ! SendBufs(SOCKET sock, struct iovec *bufs, int nbufs, Ns_Time *timeoutPtr, int flags)
>>  {
>> !     Ns_Conn *conn = Ns_GetConn();
>> !
>> !     return Ns_ConnSend(conn, bufs, nbufs);
>> ! }
>> !
> 
> 
> This should be Ns_SockSendBufs(), otherwise connPtr->nContentSent will
> be double counted and the driver thread may block.
> 
> Although in this case it should actually be Send(), which is your SSL
> specific implementation of Ns_SockSendBufs(). The way it is now static
> files may be sent in the clear :-/
> 
> 
> (Basically, you implement the SendFile() callback by calling
> Ns_SockSendFileIndirect(), which is the default library implementation
> which scrapes the bytes off disk, and pass it Send(), which is your
> implementation of writing to the network.  nssock is maybe a little
> confusing in that it doesn't actually do anything but call the default
> implementations, because it doesn't have to...)
> 
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel
> 

Re: [naviserver-devel] modules/nsssl nsssl.c,1.6,1.7

[Stephen D. <sd...@gm...>]

On Wed, Oct 22, 2008 at 9:03 PM, Vlad Seryakov <vl...@cr...> wrote:
> I tried to use Ns_SockSendBufs() but it sends data directly to socket
> where i need to send via driver's send proc.


Just pass your Send() proc directly, and delete SendBufs():

 static ssize_t
 SendFile(Ns_Sock *sock, Ns_FileVec *bufs, int nbufs, Ns_Time
*timeoutPtr, int flags)
 {
-    return Ns_SockSendFileBufsIndirect(sock->sock, bufs, nbufs,
timeoutPtr, flags, SendBufs);
+    return Ns_SockSendFileBufsIndirect(sock->sock, bufs, nbufs,
timeoutPtr, flags, Send);
 }


> Why connPtr->nContentSent is not updated, all send called with conn
> parameters should do this


Yes, but Ns_ConnSend() is higher up the stack, so when sending data it
looks something like this:

return.c: Ns_ConnReturnData
connio.c: Ns_ConnWriteData
connio.c: Ns_ConnWriteVData
connio.c: Ns_ConnSend
driver.c: NsDriverSend
nsssl/nsssl.c: Send


By the time your Send() is called we've already passed through
Ns_ConnSend(). nContentSent will be updated as the stack unwinds.

If you call Ns_ConnSend() from within Send(), that's a loop...

Re: [naviserver-devel] modules/nsssl nsssl.c,1.6,1.7

[Vlad S. <vl...@cr...>]

The first argument is different, Ns_Sock vs SOCKET

Stephen Deasey wrote:
> On Wed, Oct 22, 2008 at 9:03 PM, Vlad Seryakov <vl...@cr...> wrote:
>> I tried to use Ns_SockSendBufs() but it sends data directly to socket
>> where i need to send via driver's send proc.
> 
> 
> Just pass your Send() proc directly, and delete SendBufs():
> 
>  static ssize_t
>  SendFile(Ns_Sock *sock, Ns_FileVec *bufs, int nbufs, Ns_Time
> *timeoutPtr, int flags)
>  {
> -    return Ns_SockSendFileBufsIndirect(sock->sock, bufs, nbufs,
> timeoutPtr, flags, SendBufs);
> +    return Ns_SockSendFileBufsIndirect(sock->sock, bufs, nbufs,
> timeoutPtr, flags, Send);
>  }
> 
> 
>> Why connPtr->nContentSent is not updated, all send called with conn
>> parameters should do this
> 
> 
> Yes, but Ns_ConnSend() is higher up the stack, so when sending data it
> looks something like this:
> 
> return.c: Ns_ConnReturnData
> connio.c: Ns_ConnWriteData
> connio.c: Ns_ConnWriteVData
> connio.c: Ns_ConnSend
> driver.c: NsDriverSend
> nsssl/nsssl.c: Send
> 
> 
> By the time your Send() is called we've already passed through
> Ns_ConnSend(). nContentSent will be updated as the stack unwinds.
> 
> If you call Ns_ConnSend() from within Send(), that's a loop...
> 
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel
> 

Re: [naviserver-devel] modules/nsssl nsssl.c,1.6,1.7

[Stephen D. <sd...@gm...>]

On Wed, Oct 22, 2008 at 8:58 PM, Vlad Seryakov <vl...@cr...> wrote:
> Stephen Deasey wrote:
>> On Wed, Oct 22, 2008 at 9:03 PM, Vlad Seryakov <vl...@cr...> wrote:
>>> I tried to use Ns_SockSendBufs() but it sends data directly to socket
>>> where i need to send via driver's send proc.
>>
>>
>> Just pass your Send() proc directly, and delete SendBufs():
>>
>>  static ssize_t
>>  SendFile(Ns_Sock *sock, Ns_FileVec *bufs, int nbufs, Ns_Time
>> *timeoutPtr, int flags)
>>  {
>> -    return Ns_SockSendFileBufsIndirect(sock->sock, bufs, nbufs,
>> timeoutPtr, flags, SendBufs);
>> +    return Ns_SockSendFileBufsIndirect(sock->sock, bufs, nbufs,
>> timeoutPtr, flags, Send);
>>  }
>>
>>
>>> Why connPtr->nContentSent is not updated, all send called with conn
>>> parameters should do this
>>
>>
>> Yes, but Ns_ConnSend() is higher up the stack, so when sending data it
>> looks something like this:
>>
>> return.c: Ns_ConnReturnData
>> connio.c: Ns_ConnWriteData
>> connio.c: Ns_ConnWriteVData
>> connio.c: Ns_ConnSend
>> driver.c: NsDriverSend
>> nsssl/nsssl.c: Send
>>
>>
>> By the time your Send() is called we've already passed through
>> Ns_ConnSend(). nContentSent will be updated as the stack unwinds.
>>
>> If you call Ns_ConnSend() from within Send(), that's a loop...
>>
>
> The first argument is different, Ns_Sock vs SOCKET
>


Fixed. For SSL (and for all drivers) just leave sendfFileProc as NULL
and your sendProc will be called as needed to send file/buffer chunks.

If you were writing something like an FTP server, then you would want
to copy nssock and call Ns_SockSendFileBufs to optimise the
reading-from-disk/writing-to-network. Leaving it NULL will always be
correct though.

Re: [naviserver-devel] modules/nsssl nsssl.c,1.6,1.7

[Vlad S. <vl...@cr...>]

Great, thanks

> 
> 
> Fixed. For SSL (and for all drivers) just leave sendfFileProc as NULL
> and your sendProc will be called as needed to send file/buffer chunks.
> 
> If you were writing something like an FTP server, then you would want
> to copy nssock and call Ns_SockSendFileBufs to optimise the
> reading-from-disk/writing-to-network. Leaving it NULL will always be
> correct though.
> 
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel
> 

Re: [naviserver-devel] modules/nsssl nsssl.c,1.6,1.7

[Vlad S. <vl...@cr...>]

> The check for (sslPtr == NULL) happens after it's already been used.


Oh, late night work :-((