Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pesigchek signature validation fails for binary signed by expired certificate #61

Open
iokomin opened this issue Sep 18, 2020 · 2 comments

Comments

@iokomin
Copy link

iokomin commented Sep 18, 2020

pesigcheck failed to validate signatures done by expired certificate.

Per my understanding validation supposed to work for test certificate in order to verify that signatures work with expired certificates in the hardware.

$ certutil -L -d pesign-test -n "Test Certificate" -r > ./pesign_test.der
$ openssl x509 -in pesign_test.der -inform DER -text | grep -A2 Validity
        Validity
            Not Before: Sep  7 23:00:00 2019 GMT
            Not After : Sep  7 23:00:00 2020 GMT
$ pesign -s -n pesign-test/ -c "Test Certificate" -i vmlinuz.unsigned -o vmlinuz.signed
$ pesign -S -i vmlinuz.signed
---------------------------------------------
certificate address is 0x7f52c47b9988
Content was not encrypted.
Content is detached; signature cannot be verified.
The signer's common name is Test Certificate
No signer email address.
Signing time: Thu Sep 17, 2020
There were certs or crls included.
---------------------------------------------
$ pesigcheck -n 0 -c pesign_test.der -i vmlinuz.signed -v
Searching db pesign_test.der
Searching db pesign_test.der
Signature has impossible time constraint: 1600385627 <= 1599519600
Peer's Certificate has expired.
No matching whitelist entry.
pesigcheck: "vmlinuz.signed" is invalid.

Please confirm that pesigcheck expected to validate signatures done with expired certificate. Based on implementation I came to conclusion it was presumed while there is a "atTime" calculation issue in the code.

pesign/src/certdb.c

Lines 369 to 391 in e0ea290

notBefore = earlyNow;
notAfter = lateNow;
find_cert_times(cinfo, &notBefore, &notAfter);
if (earlyNow < notBefore)
earlyNow = notBefore;
if (lateNow > notAfter)
lateNow = notAfter;
// atTime = determine_reasonable_time(cert);
eTime = SEC_PKCS7GetSigningTime(cinfo);
if (eTime != NULL) {
if (DER_DecodeTimeChoice (&atTime, eTime) == SECSuccess) {
if (earlyNow < atTime)
earlyNow = atTime;
if (lateNow > atTime)
lateNow = atTime;
}
}
if (lateNow < earlyNow)
printf("Signature has impossible time constraint: %lld <= %lld\n",
earlyNow / 1000000LL, lateNow / 1000000LL);
atTime = earlyNow / 2 + lateNow / 2;

@frozencemetery
Copy link
Member

pesigcheck failed to validate signatures done by expired certificate.

I don't know why you'd expect anything different. Note that pesign isn't what's actually running and performing validation on the machine during boot - pesign is in userspace, for performing and checking signatures.

@iokomin
Copy link
Author

iokomin commented Apr 1, 2022

@frozencemetery thanks for looking into it, your evaluation makes sense to me if signature validation done by expired certificate outside of the validity time frame considered as invalid.

Trying to get desired behavior taking into account test certificate in the pesign package is intentionally expired, see https://bugzilla.redhat.com/show_bug.cgi?id=1411213#c5.
Could you please take a look at the atTime calculation logic and share your thoughts on the target purpose? See snippet

pesign/src/certdb.c

Lines 369 to 391 in e0ea290

notBefore = earlyNow;
notAfter = lateNow;
find_cert_times(cinfo, &notBefore, &notAfter);
if (earlyNow < notBefore)
earlyNow = notBefore;
if (lateNow > notAfter)
lateNow = notAfter;
// atTime = determine_reasonable_time(cert);
eTime = SEC_PKCS7GetSigningTime(cinfo);
if (eTime != NULL) {
if (DER_DecodeTimeChoice (&atTime, eTime) == SECSuccess) {
if (earlyNow < atTime)
earlyNow = atTime;
if (lateNow > atTime)
lateNow = atTime;
}
}
if (lateNow < earlyNow)
printf("Signature has impossible time constraint: %lld <= %lld\n",
earlyNow / 1000000LL, lateNow / 1000000LL);
atTime = earlyNow / 2 + lateNow / 2;

Curious about line 391 assignment for atTime variable.

@frozencemetery frozencemetery reopened this Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants