[gnutls-devel] gnutls-cli OCSP test code for branch 'ocsp2'

[Nikos Mavrogiannopoulos]

On Wed, 2015-02-04 at 14:23 +0100, Tim Ruehsen wrote:
> On Wednesday 04 February 2015 12:34:15 Nikos Mavrogiannopoulos wrote:
> > On Wed, 2015-02-04 at 11:35 +0100, Tim Ruehsen wrote:
> > > On Wednesday 04 February 2015 11:22:29 Nikos Mavrogiannopoulos wrote:
> > > > On Wed, 2015-02-04 at 11:05 +0100, Tim Ruehsen wrote:
> > > > > > I don't think that this is related. However, at the current state
> > > > > > the
> > > > > > packets generated seem to be in accordance with wireshark, so as far
> > > > > > as
> > > > > > I understand, it remains to properly support it on the server side
> > > > > > by
> > > > > > enhancing the ocsptool to generate a combined status request, as
> > > > > > well as
> > > > > > accounting the multiple OCSP responses received on peer's
> > > > > > certificate
> > > > > > verification.
> > > > > 
> > > > > The fix just handles the case where status_request and
> > > > > status_request_v2
> > > > > both are sent (client hello), but the answer (server hello) just
> > > > > includes
> > > > > status_request.
> > > > > I guess, that will be pretty common the next few months/years.
> > > > 
> > > > Ok, thanks. I've handled it a bit differently.
> > > 
> > > Sorry, Nikos.
> > > I just pulled it and it does not work.
> > > gnutls_ocsp_status_request_is_checked still returns 0 though there is a
> > > status_request answer.
> > > Just apply my first patch 0001-add-OCSP-multi-stapling-test-code.patch ...
> > > it just adds some printf to show that.
> > 
> > Ok. It was only working when the V2 request wasn't sent. I've now
> > updated the fix to work in all scenarios.
> 
> Thanks, Nikos !
> 
> IMO, there are two little glitch in your code.
> Guess idx==0 and the first ret becomes < 0... this is not catched, than you 
> will use priv = epriv.ptr though epriv is uninitialized (or NULL).
> Looks like a crash in this case.

Wouldn't that be caught by:
if (idx == 0 && (ret < 0 ||
((status_request_ext_st*)epriv.ptr)->responses_size == 0))
? (the idx == 0 && ret < 0 part).

> Also in
> 	if (idx == 0 && (ret < 0 || ((status_request_ext_st*)epriv.ptr)-
> >responses_size == 0)) {
> 'epriv.ptr' should be checked for NULL (else crash).

Thanks, I've updated the patch.

regards,
Nikos