412311 - Replace PR_INTERVAL_NO_WAIT with PR_INTERVAL_NO_TIMEOUT in client initialization calls

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Alexei Volkov]

PR_INTERVAL_NO_WAIT socket option makes client to use non-blocking IO. It is still untested(and apparently broken) feature, and so it should not be used until verified(or fixed).

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Alexei, this bug is WAY too vague.  It sounds like you're saying that 
non-blocking IO is not supported anywhere by NSS.  But in fact, 
non-blocking IO has been working for years in libSSL.  

I suspect you are referring to some specific function, or small group
of functions, but I don't know in what libraries or test programs.
So, please correct this bug to be very specific about the issue.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

After reading bug 408434 comment 8, I gather this bug is describing 
function pkix_pl_AiaMgr_FindLDAPClient in 
nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_aiamgr.c .
Is there any other function that is covered by this bug?

Comment 3

[Alexei Volkov]

> Alexei, this bug is WAY too vague.  It sounds like you're saying that 
> non-blocking IO is not supported anywhere by NSS.  But in fact, 
> non-blocking IO has been working for years in libSSL.  

This bug speaks only about libpkix non-blocking functionality.

> Is there any other function that is covered by this bug?
No. Just this one place. Bug any IO feature of libpkix(cert, crl fetching, ocsp revocation checking) can, by design, use NBIO. We just need to make sure that libpkix does not use NBIO before this libpkix feature get tested.  

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

We need to think more about how timeouts are set.

Comment 5

[Alexei Volkov]

Retargeting to 3.12.1. Want to make sure timeouts are properly set.

Comment 6

[Alexei Volkov]

Attachment: Patch v1 use global pkix timeout

The set of changes: create global pkix structure that keeps values for pkix communication timeout and maximum transfer length. Use those values for ocsp 
responses and cert/crl transfers.

Comment 7

[Alexei Volkov]

Comment on attachment 343132 [details] [diff] [review]
Patch v1 use global pkix timeout

The intention of the setter function is to change default values, but those can only use before/immediately after pkix initialization.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 343132 [details] [diff] [review]
Patch v1 use global pkix timeout

Let's not define a new global, except as a _DEFAULT_ value.
Let's put the actual values used the PL Context struct.
CERT_PKIXVerifyCert should have a way for the caller to 
specify values, and if he doesn't then the defaults should 
be used.  It's OK for the defaults to be settable, but 
setting the defaults should not be the only way to change 
these values.

Comment 9

[Alexei Volkov]

Attachment: Patch v2 - use nsscontext stored timeout

The patch also cleans up some function in pkix_pl_aiamgr.c and fixes http context leak in pkix_pl_httpcertstore.c.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 343984 [details] [diff] [review]
Patch v2 - use nsscontext stored timeout

r+

I think maybe the setter functions below belong in a public header?

>--- lib/libpkix/pkix_pl_nss/module/pkix_pl_nsscontext.h	31 Aug 2007 03:21:38 -0000	1.3
>+++ lib/libpkix/pkix_pl_nss/module/pkix_pl_nsscontext.h	20 Oct 2008 22:42:10 -0000
>@@ -73,6 +73,12 @@ pkix_pl_NssContext_GetWincx(PKIX_PL_NssC
> PKIX_Error *
> pkix_pl_NssContext_SetWincx(void *wincx, PKIX_PL_NssContext *nssContext);
> 
>+PKIX_Error *
>+pkix_pl_NssContext_SetTimeout(PKIX_UInt32 timeout, PKIX_PL_NssContext *nssContext);
>+
>+PKIX_Error *
>+pkix_pl_NssContext_SetMaxResponseLen(PKIX_UInt32 len, PKIX_PL_NssContext *nssContext);
>+

Comment 11

[Alexei Volkov]

Attachment: Patch v3 - integrated