[Bug] Validador retorna `La firma del documento no tiene el Policy Id` al usar certificados con serialNumber grandes
fdelapena opened this issue · 6 comments
fdelapena commented
Describa el problema (bug)
Ver #155
Pasos para reproducir
Ver #155 (comment)
Versión API: Cualquiera
Sistema operativo: Cualquiera
PHP versión: Cualquiera
MySQL versión: Cualquiera
Comportamiento esperado
El validador de Hacienda acepta el fichero sin problemas y el serialNumber es integer.
Capturas de pantalla o registro de bitácora
La firma del documento no tiene el Policy Id
fdelapena commented
Parche:
diff --git a/api/contrib/firmarXML/xmlseclibs/src/XMLSecurityDSig.php b/api/contrib/firmarXML/xmlseclibs/src/XMLSecurityDSig.php
index 0fd4426..0dd8088 100644
--- a/api/contrib/firmarXML/xmlseclibs/src/XMLSecurityDSig.php
+++ b/api/contrib/firmarXML/xmlseclibs/src/XMLSecurityDSig.php
@@ -949,6 +949,26 @@ class XMLSecurityDSig
}
+ // Allows converting large hexadecimal string to large decimal string
+ // without precision issues or requiring PHP extensions like BC Math or GMP
+ public function stringHex2StringDec($hex) {
+ $dec = [];
+ $hexLen = strlen($hex);
+ for ($h = 0; $h < $hexLen; ++$h) {
+ $carry = hexdec($hex[$h]);
+ for ($i = 0; $i < count($dec); ++$i) {
+ $val = $dec[$i] * 16 + $carry;
+ $dec[$i] = $val % 10;
+ $carry = (int) ($val / 10);
+ }
+ while ($carry > 0) {
+ $dec[] = $carry % 10;
+ $carry = (int) ($carry / 10);
+ }
+ }
+ return join("", array_reverse($dec));
+ }
+
/**
* @param XMLSecurityKey $objKey
* @param null|DOMNode $parent
@@ -1134,7 +1154,12 @@ class XMLSecurityDSig
$x509SubjectNode = $baseDoc->createElementNS(self::XMLDSIGNS, $dsig_pfx.'X509SubjectName', $subjectNameValue);
$x509DataNode->appendChild($x509SubjectNode);
}
- if ($issuerSerial && ! empty($certData['issuer']) && ! empty($certData['serialNumber'])) {
+ if (strpos($certData['serialNumber'], "0x") === false) { // https://bugs.php.net/bug.php?id=77411
+ $serialNumber = $certData['serialNumber'];
+ } else {
+ $serialNumber = stringHex2StringDec($certData['serialNumber']);
+ }
+ if ($issuerSerial && ! empty($certData['issuer']) && ! empty($serialNumber)) {
if (is_array($certData['issuer'])) {
$parts = array();
foreach ($certData['issuer'] AS $key => $value) {
@@ -1150,7 +1175,7 @@ class XMLSecurityDSig
$x509Node = $baseDoc->createElementNS(self::XMLDSIGNS, $dsig_pfx.'X509IssuerName', $issuerName);
$x509IssuerNode->appendChild($x509Node);
- $x509Node = $baseDoc->createElementNS(self::XMLDSIGNS, $dsig_pfx.'X509SerialNumber', $certData['serialNumber']);
+ $x509Node = $baseDoc->createElementNS(self::XMLDSIGNS, $dsig_pfx.'X509SerialNumber', $serialNumber);
$x509IssuerNode->appendChild($x509Node);
}
}
@@ -1346,6 +1371,11 @@ class XMLSecurityDSig
$digestValueNode = $this->createNewSignNode('DigestValue', $digestValue);
$certDigestNode->appendChild($digestValueNode);
$certData = openssl_x509_parse($certInfo["publicKey"]);
+ if (strpos($certData['serialNumber'], "0x") === false) { // https://bugs.php.net/bug.php?id=77411
+ $serialNumber = $certData['serialNumber'];
+ } else {
+ $serialNumber = stringHex2StringDec($certData['serialNumber']);
+ }
$certIssuer = [];
foreach ($certData['issuer'] as $item => $value) {
$certIssuer[] = $item . '=' . $value;
@@ -1353,7 +1383,7 @@ class XMLSecurityDSig
$certIssuer = implode(', ', array_reverse($certIssuer));
$X509IssuerNameNode = $this->createNewSignNode('X509IssuerName', $certIssuer);
$issuerSerialNode->appendChild($X509IssuerNameNode);
- $X509SerialNumber = $this->createNewSignNode('X509SerialNumber', $certData['serialNumber']);
+ $X509SerialNumber = $this->createNewSignNode('X509SerialNumber', $serialNumber);
$issuerSerialNode->appendChild($X509SerialNumber);
$signaturePolicyIdentifierNode = $this->createNewXadesNode('SignaturePolicyIdentifier');
$signedSignaturePropertiesNode->appendChild($signaturePolicyIdentifierNode);
diff --git a/api/contrib/signXML/Firmadohaciendacr.php b/api/contrib/signXML/Firmadohaciendacr.php
index 6323667..5d69bf1 100644
--- a/api/contrib/signXML/Firmadohaciendacr.php
+++ b/api/contrib/signXML/Firmadohaciendacr.php
@@ -59,6 +59,26 @@ class Firmadocr
return base64_encode(hash('sha256', $strcadena, true));
}
+ // Allows converting large hexadecimal string to large decimal string
+ // without precision issues or requiring PHP extensions like BC Math or GMP
+ public function stringHex2StringDec($hex) {
+ $dec = [];
+ $hexLen = strlen($hex);
+ for ($h = 0; $h < $hexLen; ++$h) {
+ $carry = hexdec($hex[$h]);
+ for ($i = 0; $i < count($dec); ++$i) {
+ $val = $dec[$i] * 16 + $carry;
+ $dec[$i] = $val % 10;
+ $carry = (int) ($val / 10);
+ }
+ while ($carry > 0) {
+ $dec[] = $carry % 10;
+ $carry = (int) ($carry / 10);
+ }
+ }
+ return join("", array_reverse($dec));
+ }
+
public function firmar($certificadop12, $clavecertificado, $xmlsinfirma, $tipodoc)
{
if (!$pfx = file_get_contents($certificadop12)) {
@@ -147,6 +167,12 @@ class Firmadocr
$certIssuer = implode(', ', array_reverse($certIssuer));
+ if (strpos($certData['serialNumber'], "0x") === false) { // https://bugs.php.net/bug.php?id=77411
+ $serialNumber = $certData['serialNumber'];
+ } else {
+ $serialNumber = stringHex2StringDec($certData['serialNumber']);
+ }
+
$prop = '<xades:SignedProperties Id="' . $this->SignedProperties . '">' .
'<xades:SignedSignatureProperties>'.
'<xades:SigningTime>' . $signTime1 . '</xades:SigningTime>' .
@@ -158,7 +184,7 @@ class Firmadocr
'</xades:CertDigest>'.
'<xades:IssuerSerial>' .
'<ds:X509IssuerName>' . $certIssuer . '</ds:X509IssuerName>'.
- '<ds:X509SerialNumber>' . $certData['serialNumber'] . '</ds:X509SerialNumber>' .
+ '<ds:X509SerialNumber>' . $serialNumber . '</ds:X509SerialNumber>' .
'</xades:IssuerSerial>'.
'</xades:Cert>'.
'</xades:SigningCertificate>' .JeanCarlosChavarriaHughes commented
Se requiere crear un Pull Request, con el cambio sugerido por @fdelapena y verificar el correcto funcionamiento.
JeanCarlosChavarriaHughes commented
JeanCarlosChavarriaHughes commented
@fdelapena puedes revisar este PR?
fdelapena commented
@JeanCarlosChavarriaHughes disculpas por la demora, el parche se ve bien aplicado. ¡Gracias!
fdelapena commented
Solo para confirmar, @JeanCarlosChavarriaHughes si planea aplicarlo a este repositorio también, porque el merge solo parece estar aplicado a su fork. Gracias.
Edit: ya lo vi, forma parte de #161, gracias.