From 8bcafc8bc497d76dbd68b02d84b4a30e709310a3 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Thu, 7 Jul 2016 21:25:10 +0200 Subject: [PATCH] Revert "populate KeyInfo node before calculating Reference nodes" This reverts commit 8f6c95a90735c4d6e13bddf84de7a5284132826c. This is needed till LO code depends on the undocumented xmlsec behavior that throwing a binary PNG image on the XML parser returns with an error *before* it calls xmlSecDSigCtxProcessKeyInfoNode. Conflicts: src/xmldsig.c --- src/xmldsig.c | 123 +++++++++++++++------------------------------------------- 1 file changed, 32 insertions(+), 91 deletions(-) diff --git a/src/xmldsig.c b/src/xmldsig.c index faf5545..3c4b236 100644 --- a/src/xmldsig.c +++ b/src/xmldsig.c @@ -39,8 +39,7 @@ static int xmlSecDSigCtxProcessSignatureNode (xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node); static int xmlSecDSigCtxProcessSignedInfoNode (xmlSecDSigCtxPtr dsigCtx, - xmlNodePtr node, - xmlNodePtr * firstReferenceNode); + xmlNodePtr node); static int xmlSecDSigCtxProcessKeyInfoNode (xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node); static int xmlSecDSigCtxProcessObjectNode (xmlSecDSigCtxPtr dsigCtx, @@ -48,9 +47,6 @@ static int xmlSecDSigCtxProcessObjectNode (xmlSecDSigCtxPtr dsigCt static int xmlSecDSigCtxProcessManifestNode (xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node); -static int xmlSecDSigCtxProcessReferences (xmlSecDSigCtxPtr dsigCtx, - xmlNodePtr firstReferenceNode); - /* The ID attribute in XMLDSig is 'Id' */ static const xmlChar* xmlSecDSigIds[] = { xmlSecAttrId, NULL }; @@ -474,7 +470,6 @@ xmlSecDSigCtxProcessSignatureNode(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node) { xmlSecTransformDataType firstType; xmlNodePtr signedInfoNode = NULL; xmlNodePtr keyInfoNode = NULL; - xmlNodePtr firstReferenceNode = NULL; xmlNodePtr cur; int ret; @@ -563,7 +558,7 @@ xmlSecDSigCtxProcessSignatureNode(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node) { } /* now validated all the references and prepare transform */ - ret = xmlSecDSigCtxProcessSignedInfoNode(dsigCtx, signedInfoNode, &firstReferenceNode); + ret = xmlSecDSigCtxProcessSignedInfoNode(dsigCtx, signedInfoNode); if(ret < 0) { xmlSecError(XMLSEC_ERRORS_HERE, NULL, @@ -572,12 +567,15 @@ xmlSecDSigCtxProcessSignatureNode(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node) { XMLSEC_ERRORS_NO_MESSAGE); return(-1); } + /* references processing might change the status */ + if(dsigCtx->status != xmlSecDSigStatusUnknown) { + return(0); + } /* as the result, we should have sign and c14n methods set */ xmlSecAssert2(dsigCtx->signMethod != NULL, -1); xmlSecAssert2(dsigCtx->c14nMethod != NULL, -1); - /* now read key info node */ ret = xmlSecDSigCtxProcessKeyInfoNode(dsigCtx, keyInfoNode); if(ret < 0) { xmlSecError(XMLSEC_ERRORS_HERE, @@ -590,21 +588,6 @@ xmlSecDSigCtxProcessSignatureNode(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node) { /* as the result, we should have a key */ xmlSecAssert2(dsigCtx->signKey != NULL, -1); - /* now actually process references and calculate digests */ - ret = xmlSecDSigCtxProcessReferences(dsigCtx, firstReferenceNode); - if(ret < 0) { - xmlSecError(XMLSEC_ERRORS_HERE, - NULL, - "xmlSecDSigCtxProcessReferences", - XMLSEC_ERRORS_R_XMLSEC_FAILED, - XMLSEC_ERRORS_NO_MESSAGE); - return(-1); - } - /* references processing might change the status */ - if(dsigCtx->status != xmlSecDSigStatusUnknown) { - return(0); - } - /* if we need to write result to xml node then we need base64 encode result */ if(dsigCtx->operation == xmlSecTransformOperationSign) { xmlSecTransformPtr base64Encode; @@ -700,18 +683,18 @@ xmlSecDSigCtxProcessSignatureNode(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node) { * */ static int -xmlSecDSigCtxProcessSignedInfoNode(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node, xmlNodePtr * firstReferenceNode) { - xmlSecSize refNodesCount = 0; +xmlSecDSigCtxProcessSignedInfoNode(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node) { + xmlSecDSigReferenceCtxPtr dsigRefCtx; xmlNodePtr cur; + int ret; xmlSecAssert2(dsigCtx != NULL, -1); xmlSecAssert2(dsigCtx->status == xmlSecDSigStatusUnknown, -1); xmlSecAssert2(dsigCtx->signMethod == NULL, -1); xmlSecAssert2(dsigCtx->c14nMethod == NULL, -1); xmlSecAssert2((dsigCtx->operation == xmlSecTransformOperationSign) || (dsigCtx->operation == xmlSecTransformOperationVerify), -1); + xmlSecAssert2(xmlSecPtrListGetSize(&(dsigCtx->signedInfoReferences)) == 0, -1); xmlSecAssert2(node != NULL, -1); - xmlSecAssert2(firstReferenceNode != NULL, -1); - xmlSecAssert2((*firstReferenceNode) == NULL, -1); /* first node is required CanonicalizationMethod. */ cur = xmlSecGetNextElementNode(node->children); @@ -805,72 +788,12 @@ xmlSecDSigCtxProcessSignedInfoNode(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node, xm } dsigCtx->signMethod->operation = dsigCtx->operation; - /* read references */ + /* calculate references */ if(cur != NULL) { cur = xmlSecGetNextElementNode(cur->next); } while((cur != NULL) && (xmlSecCheckNodeName(cur, xmlSecNodeReference, xmlSecDSigNs))) { - /* record first reference node */ - if((*firstReferenceNode) == NULL) { - (*firstReferenceNode) = cur; - } - ++refNodesCount; - - /* go to next */ - cur = xmlSecGetNextElementNode(cur->next); - } - - /* check that we have at least one Reference */ - if(refNodesCount == 0) { - xmlSecError(XMLSEC_ERRORS_HERE, - NULL, - NULL, - XMLSEC_ERRORS_R_DSIG_NO_REFERENCES, - XMLSEC_ERRORS_NO_MESSAGE); - return(-1); - } - - /* if there is something left than it's an error */ - if(cur != NULL) { - xmlSecError(XMLSEC_ERRORS_HERE, - NULL, - xmlSecErrorsSafeString(xmlSecNodeGetName(cur)), - XMLSEC_ERRORS_R_UNEXPECTED_NODE, - XMLSEC_ERRORS_NO_MESSAGE); - return(-1); - } - - /* done */ - return(0); -} - - -static int -xmlSecDSigCtxProcessReferences(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr firstReferenceNode) { - xmlSecDSigReferenceCtxPtr dsigRefCtx; - xmlNodePtr cur; - int ret; - - xmlSecAssert2(dsigCtx != NULL, -1); - xmlSecAssert2(dsigCtx->status == xmlSecDSigStatusUnknown, -1); - xmlSecAssert2((dsigCtx->operation == xmlSecTransformOperationSign) || (dsigCtx->operation == xmlSecTransformOperationVerify), -1); - xmlSecAssert2(xmlSecPtrListGetSize(&(dsigCtx->signedInfoReferences)) == 0, -1); - xmlSecAssert2(firstReferenceNode != NULL, -1); - - /* process references */ - for(cur = firstReferenceNode; (cur != NULL); cur = xmlSecGetNextElementNode(cur->next)) { - /* already checked but we trust none */ - if(!xmlSecCheckNodeName(cur, xmlSecNodeReference, xmlSecDSigNs)) { - xmlSecError(XMLSEC_ERRORS_HERE, - NULL, - xmlSecErrorsSafeString(xmlSecNodeGetName(cur)), - XMLSEC_ERRORS_R_INVALID_NODE, - "expected=%s", - xmlSecErrorsSafeString(xmlSecNodeReference)); - return(-1); - } - - /* create reference */ + /* create reference */ dsigRefCtx = xmlSecDSigReferenceCtxCreate(dsigCtx, xmlSecDSigReferenceOriginSignedInfo); if(dsigRefCtx == NULL) { xmlSecError(XMLSEC_ERRORS_HERE, @@ -910,13 +833,31 @@ xmlSecDSigCtxProcessReferences(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr firstReferen dsigCtx->status = xmlSecDSigStatusInvalid; return(0); } + cur = xmlSecGetNextElementNode(cur->next); + } + + /* check that we have at least one Reference */ + if(xmlSecPtrListGetSize(&(dsigCtx->signedInfoReferences)) == 0) { + xmlSecError(XMLSEC_ERRORS_HERE, + NULL, + NULL, + XMLSEC_ERRORS_R_DSIG_NO_REFERENCES, + XMLSEC_ERRORS_NO_MESSAGE); + return(-1); } - /* done */ + /* if there is something left than it's an error */ + if(cur != NULL) { + xmlSecError(XMLSEC_ERRORS_HERE, + NULL, + xmlSecErrorsSafeString(xmlSecNodeGetName(cur)), + XMLSEC_ERRORS_R_UNEXPECTED_NODE, + XMLSEC_ERRORS_NO_MESSAGE); + return(-1); + } return(0); } - static int xmlSecDSigCtxProcessKeyInfoNode(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node) { int ret; -- 2.6.6