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

PIV: Fixed detection of PIV AID via empty discovery #2441

Closed
wants to merge 1 commit into from

Conversation

frankmorgner
Copy link
Member

@frankmorgner frankmorgner commented Nov 11, 2021

Since we are using the discovery object to detect whether it's a proper
PIV application, piv_parse_discovery() should not return SC_SUCCESS on
empty files or on files with some ASN.1 structure... Instead, at least
the encoding of the PIV AID should be strictly according NIST SP 800-73-4

Checklist
  • Documentation is added or updated
  • New files have a LGPL 2.1 license statement
  • PKCS#11 module is tested
  • Windows minidriver is tested
  • macOS tokend is tested

Since we are using the discovery object to detect whether it's a proper
PIV application, `piv_parse_discovery()` should not return SC_SUCCESS on
empty files or on files with *some* ASN.1 structure... Instead, at least
the encoding of the PIV AID should be strictly according NIST SP 800-73-4
@dengert
Copy link
Member

dengert commented Nov 11, 2021

This was in #2053

@frankmorgner
Copy link
Member Author

Well, partially. piv_parse_discovery() still returns SC_SUCCESS for an empty file.

if (SC_SUCCESS != sc_asn1_read_tag(&body, rbuflen, &cla, &tag, &body_len)
|| cla+tag != 0x7E
|| NULL == (aid = sc_asn1_find_tag(card->ctx, body, body_len, 0x4F, &aid_len))
|| aid_len < piv_aid.len || memcmp(aid, piv_aid.value, piv_aid.len) != 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, this fails to build:

card-piv.c:2588:17: error: use of undeclared identifier 'piv_aid'
                        || aid_len < piv_aid.len || memcmp(aid, piv_aid.value, piv_aid.len) != 0)
                                     ^
card-piv.c:2588:44: error: use of undeclared identifier 'piv_aid'
                        || aid_len < piv_aid.len || memcmp(aid, piv_aid.value, piv_aid.len) != 0)
                                                                ^
card-piv.c:2588:59: error: use of undeclared identifier 'piv_aid'
                        || aid_len < piv_aid.len || memcmp(aid, piv_aid.value, piv_aid.len) != 0)

@frankmorgner
Copy link
Member Author

I'll open a unified draft PR with all simplifications.

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

Successfully merging this pull request may close these issues.

None yet

3 participants