Skip to content

QCNG.cpp: SHA-512 RSA-PSS uses NCRYPT_SHA256_ALGORITHM — typo or intentional? #1408

Description

@kloploid

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!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions