Re: [6/6] secur32: Implement schannel DecryptMessage().

[Rob Shearman]

Sorry, this should have been in reply to patch [6/6].

2008/12/29 Henri Verbeet :
> 2008/12/29 Rob Shearman :
> > 2008/12/28 Henri Verbeet :
> > > - while (received < data_size)
> > > - {
> > > -
> > >    ret = pgnutls_record_recv(ctx->session, data + received, data_size - received);
> > > -
> > >    if (ret < 0)
> > > -
> > >    {
> > > -
> > >        if (ret == GNUTLS_E_AGAIN)
> > > -
> > >        {
> > > -
> > >            if (!received)
> >
> > Shouldn't this be "if (received == data_size)"?
> > > -
> > >            {
> > > -
> > >                pgnutls_perror(ret);
> > > -
> > >                HeapFree(GetProcessHeap(), 0, data);
> > > -
> > >                TRACE("Returning SEC_E_INCOMPLETE_MESSAGE\n");
> > > -
> > >                return SEC_E_INCOMPLETE_MESSAGE;
> > > -
> > >            }
> > > -
> > >            break;
> > > -
> > >        }
> > > -
> > >        else
> > > -
> > >        {
> > > -
> > >            pgnutls_perror(ret);
> > > -
> > >            HeapFree(GetProcessHeap(), 0, data);
> > > -
> > >            ERR("Returning SEC_E_INTERNAL_ERROR\n");
> > > -
> > >            return SEC_E_INTERNAL_ERROR;
> > > -
> > >        }
> > > -
> > >    }
> > > -
> > >    received += ret;
> > > - }
>
> The idea was to only return SEC_E_INCOMPLETE_MESSAGE if there's less
> than one complete TLS record in the buffer, it shouldn't be a problem
> to have eg. one complete message plus some bytes from the next one,
> setting the buffer size further down is supposed to take care of that.

Is it really expected for gnutls_record_recv to return GNUTLS_E_AGAIN
on the first call and for the code to have to return that to the
caller? That is what the code as it is at the moment does. It seems to
me as though you should return SEC_E_INCOMPLETE_MESSAGE if you
decrypted all of the data in the supplied buffer, but gnutls still
returns GNUTLS_E_AGAIN. That then signals to the app to download more
data and call DecryptMessage() again with the new data.

[6/6] secur32: Implement schannel DecryptMessage().

[Henri Verbeet]

2008/12/29 Rob Shearman :
> Is it really expected for gnutls_record_recv to return GNUTLS_E_AGAIN
> on the first call and for the code to have to return that to the
> caller? That is what the code as it is at the moment does. It seems to
> me as though you should return SEC_E_INCOMPLETE_MESSAGE if you
> decrypted all of the data in the supplied buffer, but gnutls still
> returns GNUTLS_E_AGAIN. That then signals to the app to download more
> data and call DecryptMessage() again with the new data.

No, gnutls_record_recv() is only supposed to return GNUTLS_E_AGAIN
when schan_pull() can't find any more data to read (ie, once we've
read data from all the relevant buffers in the buffer desc) and the
record it's decryping is still incomplete.