Hi,
While reading through client/QCNG.cpp I noticed something that looks like a copy-paste typo, but I want to ask before assuming — the file is from 2022 and nobody has reported anything, so I might be missing something Windows-specific.
In the RSA-PSS padding setup for NCryptSignHash:
https://github.com/open-eid/DigiDoc4-Client/blob/master/client/QCNG.cpp#L287-L295
BCRYPT_PSS_PADDING_INFO rsaPSS { NCRYPT_SHA256_ALGORITHM, 32 };
switch(type)
{
case QCryptographicHash::Sha224: rsaPSS = { L"SHA224", 24 }; break;
case QCryptographicHash::Sha256: rsaPSS = { NCRYPT_SHA256_ALGORITHM, 32 }; break;
case QCryptographicHash::Sha384: rsaPSS = { NCRYPT_SHA384_ALGORITHM, 48 }; break;
case QCryptographicHash::Sha512: rsaPSS = { NCRYPT_SHA256_ALGORITHM, 64 }; break;
default: return NTE_INVALID_PARAMETER;
}
The Sha512 branch sets pszAlgId = NCRYPT_SHA256_ALGORITHM while salt length is 64. The three sibling branches follow the pattern {SHA-N, N/8}, so SHA-512 looks like it should be { NCRYPT_SHA512_ALGORITHM, 64 }.
Two reasons I think this is a bug rather than intentional:
The PKCS#11 path of the same project handles SHA-512 PSS consistently:
https://github.com/open-eid/DigiDoc4-Client/blob/master/client/QPKCS11.cpp#L462-L464
case QCryptographicHash::Sha512:
data = QByteArray::fromHex("3051300d060960864801650304020305000440");
pssParams = { CKM_SHA512, CKG_MGF1_SHA512, 64 };
break;
So a card signing through opensc-pkcs11 (Linux/macOS) gets MGF1-SHA512, but the same card on Windows through CNG would get MGF1-SHA256 with the SHA-512 digest. The XML-DSIG method advertised by QSigner is sha512-rsa-MGF1 either way (https://github.com/open-eid/DigiDoc4-Client/blob/master/client/QSigner.cpp#L139), so a verifier would expect MGF1-SHA512.
BCRYPT_PSS_PADDING_INFO::pszAlgId controls both the digest and MGF1 hash, so MGF1 ends up SHA-256 in this branch.
Questions:
Is this intentional (e.g. a workaround for some minidriver that doesn’t accept SHA-512 in pszAlgId)?
Or is it just a leftover typo from when the switch was introduced in df35a89 (#1002)?
I haven’t been able to test on Windows hardware with a card that supports RSA-PSS + SHA-512, so I can’t confirm whether the resulting signature actually fails verification in practice. If somebody on the team can quickly check (sign a test container with SHA-512 selected as the hash algorithm on Windows, then verify it elsewhere), that would settle it.
Happy to send a one-line PR (NCRYPT_SHA256_ALGORITHM → NCRYPT_SHA512_ALGORITHM) if confirmed.
Thanks!
Hi,
While reading through client/QCNG.cpp I noticed something that looks like a copy-paste typo, but I want to ask before assuming — the file is from 2022 and nobody has reported anything, so I might be missing something Windows-specific.
In the RSA-PSS padding setup for NCryptSignHash:
https://github.com/open-eid/DigiDoc4-Client/blob/master/client/QCNG.cpp#L287-L295
The Sha512 branch sets pszAlgId = NCRYPT_SHA256_ALGORITHM while salt length is 64. The three sibling branches follow the pattern {SHA-N, N/8}, so SHA-512 looks like it should be { NCRYPT_SHA512_ALGORITHM, 64 }.
Two reasons I think this is a bug rather than intentional:
The PKCS#11 path of the same project handles SHA-512 PSS consistently:
https://github.com/open-eid/DigiDoc4-Client/blob/master/client/QPKCS11.cpp#L462-L464
So a card signing through opensc-pkcs11 (Linux/macOS) gets MGF1-SHA512, but the same card on Windows through CNG would get MGF1-SHA256 with the SHA-512 digest. The XML-DSIG method advertised by QSigner is sha512-rsa-MGF1 either way (https://github.com/open-eid/DigiDoc4-Client/blob/master/client/QSigner.cpp#L139), so a verifier would expect MGF1-SHA512.
BCRYPT_PSS_PADDING_INFO::pszAlgId controls both the digest and MGF1 hash, so MGF1 ends up SHA-256 in this branch.
Questions:
Is this intentional (e.g. a workaround for some minidriver that doesn’t accept SHA-512 in pszAlgId)?
Or is it just a leftover typo from when the switch was introduced in df35a89 (#1002)?
I haven’t been able to test on Windows hardware with a card that supports RSA-PSS + SHA-512, so I can’t confirm whether the resulting signature actually fails verification in practice. If somebody on the team can quickly check (sign a test container with SHA-512 selected as the hash algorithm on Windows, then verify it elsewhere), that would settle it.
Happy to send a one-line PR (NCRYPT_SHA256_ALGORITHM → NCRYPT_SHA512_ALGORITHM) if confirmed.
Thanks!