NBIO: can not return to interrupted libpkix code path with completed IO

NEW
Assigned to

Status

P2
normal
11 years ago
8 years ago

People

(Reporter: alvolkov.bgs, Assigned: alvolkov.bgs)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PKIX nbioContext)

(Assignee)

Description

11 years ago
There are no state for some code paths in pkix_build.c that can be interrupted with blocked IO. I suspect the program will not be able to return to such code paths after an IO completion. 

Example from pkix_build.c: we will not get to the code that continued cert chain validation if enter the function with non-null state.

 3589 static PKIX_Error *
3590 pkix_Build_InitiateBuildChain(
3591         PKIX_ProcessingParams *procParams,
3592         void **pNBIOContext,
3593         PKIX_ForwardBuilderState **pState,
3594         PKIX_BuildResult **pBuildResult,
3595         PKIX_VerifyNode **pVerifyNode,
3596         void *plContext)
....
3640         if (*pState != NULL) {
3641             state = *pState;
3642             *pState = NULL; /* no net change in reference count */
3643             /* attempted shortcut ran into non-blocking I/O */
3644         } else {
3645 
3646             PKIX_CHECK(PKIX_ProcessingParams_GetDate
3647                     (procParams, &testDate, plContext),
3648                     PKIX_PROCESSINGPARAMSGETDATEFAILED);
....
3935             /* Check whether this cert verification has been cached. */
3936             PKIX_CHECK(pkix_CacheCertChain_Lookup
3937                     (targetCert,
3938                     anchors,
3939                     testDate,
3940                     &cacheHit,
3941                     &buildResult,
3942                     plContext),
3943                     PKIX_CACHECERTCHAINLOOKUPFAILED);
3944     
3945             if (cacheHit) {
3946                     /*
3947                      * We found something in cache. Verify that the anchor
3948                      * cert is still trusted,
3949                      */
3950                     PKIX_CHECK(PKIX_BuildResult_GetValidateResult
3951                             (buildResult, &valResult, plContext),
3952                             PKIX_BUILDRESULTGETVALIDATERESULTFAILED);
3953     
3954                     PKIX_CHECK(PKIX_ValidateResult_GetTrustAnchor
3955                             (valResult, &matchingAnchor, plContext),
3956                             PKIX_VALIDATERESULTGETTRUSTANCHORFAILED);
3957     
3958                     PKIX_DECREF(valResult);
3959     
3960                     PKIX_CHECK(PKIX_TrustAnchor_GetTrustedCert
3961                             (matchingAnchor, &trustedCert, plContext),
3962                             PKIX_TRUSTANCHORGETTRUSTEDCERTFAILED);
3963     
3964                     PKIX_CHECK(PKIX_PL_Cert_IsCertTrusted
3965                             (trustedCert, &trusted, plContext),
3966                             PKIX_CERTISCERTTRUSTEDFAILED);
3967     
3968                     if (trusted == PKIX_TRUE) {
3969                             /*
3970                              * Since the key usage may vary for different
3971                              * applications, we need to verify the chain again.
3972                              */
3973                             PKIX_CHECK(PKIX_BuildResult_GetCertChain
3974                                     (buildResult, &certList, plContext),
3975                                     PKIX_BUILDRESULTGETCERTCHAINFAILED);
3976     
3977                             PKIX_CHECK(pkix_Build_ValidationCheckers
3978                                     (state,
3979                                     certList,
3980                                     matchingAnchor,
3981                                     plContext),
3982                                     PKIX_BUILDVALIDATIONCHECKERSFAILED);
3983     
3984                             PKIX_CHECK_ONLY_FATAL(pkix_Build_ValidateEntireChain
3985                                     (state,
3986                                     matchingAnchor,
3987                                     &nbioContext,
3988                                     &valResult,
3989                                     verifyNode,
3990                                     plContext),
3991                                     PKIX_BUILDVALIDATEENTIRECHAINFAILED);
3992 
3993                         if (nbioContext != NULL) {
3994                                 /* IO still pending, resume later */
3995                                 *pNBIOContext = nbioContext;
3996                                 goto cleanup;
3997                             } else {
3998                                 PKIX_DECREF(state->reversedCertChain);
.... 
4115 cleanup:
4116 
4117         PKIX_DECREF(targetConstraints);
4118         PKIX_DECREF(targetParams);
4119         PKIX_DECREF(anchors);
4120         PKIX_DECREF(targetSubjNames);
4121         PKIX_DECREF(targetCert);
4122         PKIX_DECREF(crlChecker);
4123         PKIX_DECREF(certStores);
4124         PKIX_DECREF(certStore);
4125         PKIX_DECREF(userCheckers);
4126         PKIX_DECREF(hintCerts);
4127         PKIX_DECREF(firstHintCert);
4128         PKIX_DECREF(testDate);
4129         PKIX_DECREF(targetPubKey);
4130         PKIX_DECREF(tentativeChain);
4131         PKIX_DECREF(valResult);
4132         PKIX_DECREF(certList);
4133         PKIX_DECREF(matchingAnchor);
4134         PKIX_DECREF(trustedCert);
4135         PKIX_DECREF(state);
4136 
4137         PKIX_RETURN(BUILD);
4138 }
(Assignee)

Updated

11 years ago
Priority: -- → P2
Whiteboard: PKIX
(Assignee)

Updated

11 years ago
Blocks: 390888
Summary: NBIO: can not return to an interrupted code path with completed IO → NBIO: can not return to interrupted libpkix code path with completed IO
Version: 3.12 → trunk
(Assignee)

Updated

11 years ago
No longer blocks: 390888
(Assignee)

Updated

11 years ago
Priority: P2 → P1
(Assignee)

Updated

11 years ago
Priority: P1 → P2
Target Milestone: 3.12 → 3.12.2
(Assignee)

Comment 1

10 years ago
Another place where it seems the NBIO code was not tested:

File pkix_pl_httpdefaultclient.c: functions pkix_pl_HttpDefaultClient_RecvHdrContinue and  pkix_pl_HttpDefaultClient_RecvBodyContinue write received bytes in to the buffer pointer referenced from PKIX_PL_Socket object. It is incorrect, since the header parsing function works only with a buffer referenced from PKIX_PL_HttpDefaultClient object. 
(Assignee)

Comment 2

10 years ago
Yet another place: most of the code that follows PKIX_CHECK_ONLY_FATAL macro does not clear pkixTermErrorRecieved variable. This will make libpkix to throw an error each time when a no-fatal error happen and returned by a function surrounded by the macro.

In addition, all places in the code where pkixTermErrorRecieved is set to true should be checked for similar condiditons.
Unsetting target milestone in unresolved bugs whose targets have passed.
Target Milestone: 3.12.2 → ---
(Assignee)

Updated

9 years ago
Whiteboard: PKIX → PKIX nbioContext
You need to log in before you can comment on or make changes to this bug.