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.

Sign up for GitHub

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

Jump to bottom

Allow reading data group from eMRTD with npa-tool #2257

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Allow reading data group from eMRTD with npa-tool #2257

wants to merge 3 commits into from

Conversation

frankmorgner
Copy link
Member

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

@frankmorgner frankmorgner changed the title Fix read dg Fix reading data group with npa-tool Mar 10, 2021
@frankmorgner frankmorgner mentioned this pull request Mar 10, 2021
Open
@frankmorgner frankmorgner changed the title Fix reading data group with npa-tool Allow reading data group from eMRTD with npa-tool Mar 10, 2021
@lgtm-com
Copy link

lgtm-com bot commented Mar 10, 2021

This pull request introduces 1 alert when merging 1cc2b9e into 1ef79e9 - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

@AKing23T
Copy link

AKing23T commented Mar 11, 2021 •
edited

I change and compile the branch code, now npa-tool show nothing when read dg2 or dg14.

./npa-tool --can=xxxxxx --read-dg2
Using reader with a card: Feitian R805 (MadeInChina) 00 00
Established PACE channel with CAN.
Selected eMRTD application.
Read

@AKing23T
Copy link

this is my result:
./npa-tool --can=010624 --read-dg1 --read-dg2 --read-dg3 --read-dg4 --read-dg5 --read-dg6 --read-dg7 --read-dg8 --read-dg9 --read-dg10 --read-dg11 --read-dg12 --read-dg13 --read-dg14 --read-dg15 --read-dg16 --read-dg17 --read-dg18 --read-dg19 --read-dg20 --read-dg21

Using reader with a card: Feitian R805 (MadeInChina) 00 00
Established PACE channel with CAN.
Selected eMRTD application.
Read xxxxxxxxxx.......................xxxxxxx -----> my card mrz value
Read ------>dg2 nothing
Could not read DG 03 Date of Expiry (Security status not satisfied)
Could not read DG 04 Given Names (File not found)
Could not read DG 05 Family Names (File not found)
Could not read DG 06 Religious/Artistic Name (File not found)
Read ------->dg7 nothing
Could not read DG 08 Date of Birth (File not found)
Could not read DG 09 Place of Birth (File not found)
Could not read DG 10 Nationality (File not found)
Read xxxxxxxxxx.......................xxxxxxx -----> my card value
Could not read DG 12 Optional Data (File not found)
Read ------> dg13 nothing
Read ------->dg14 nothing
Could not read DG 15 DG 15 (File not found)
Could not read DG 16 DG 16 (File not found)
Could not read DG 17 Normal Place of Residence (File not found)
Could not read DG 18 Community ID (File not found)
Could not read DG 19 Residence Permit I (File not found)
Could not read DG 20 Residence Permit II (File not found)
Could not read DG 21 Optional Data (File not found)

@frankmorgner
Copy link
Member Author

This may be a parsing problem after successfully reading the DG, could you post a log?

@AKing23T
Copy link

AKing23T commented Mar 12, 2021 •
edited

from https://github.com/frankmorgner/OpenSC/tree/fix_read_dg to git clone https://github.com/frankmorgner/OpenSC.git

log for pcsc
pcsc.log

log for opensc
tw-z-opensc-debug.log

@frankmorgner
Copy link
Member Author

frankmorgner commented Mar 15, 2021 •
edited

The log indicates that you're not using the code from this pull request.

P:160015; T:0x139669973612352 09:37:20.892 [npa-tool] card.c:666:sc_read_binary: returning with: -1415 (EF offset too large)

See

OpenSC/src/libopensc/card.c

Line 666 in 1cc2b9e

todo -= (size_t) r;

Please try again...

@frankmorgner
Copy link
Member Author

@AKing23T do you have any update on this?

@AKing23T
Copy link

I doubt it is right to clone code from the site https://github.com/frankmorgner/OpenSC/tree/fix_read_dg, or u can tell me the site and I modify some files.

@frankmorgner
npa-tool: allow reading DGs from eMRTD
42375da 
Adds the new options --application and --read-all-dgs

fixes OpenSC#2253
@frankmorgner
fixed bad error translation
635df88 
a negative return value was incorrectly translated to a huge buffer
length, which then errors with "offset too large"
@frankmorgner
iso7816: if index is beyond file, return EOF
60bf597 
commands for data unit handling may return 6B00 if the specified index
is too large (ISO 7816-4). Maybe SC_ERROR_OFFSET_TOO_LARGE would be more
appropriate, but this is (1) currently not used only for internal errors
and (2) this error code is not currently handled by sc_read_binary.
Hence, we stick to SC_ERROR_FILE_END_REACHED.
@frankmorgner
Copy link
Member Author

I've fixed printing the output by supplying a large enough buffer and tried some better integration. Use the following for testing:

npa-tool --read-all-dgs --application=eMRTD

@lgtm-com
Copy link

lgtm-com bot commented Dec 12, 2021

This pull request introduces 1 alert when merging 60bf597 into 2cc7b10 - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

@frankmorgner frankmorgner mentioned this pull request Dec 28, 2021
Closed
Jakuje
Jakuje requested changes Oct 21, 2022
View reviewed changes
src/tools/npa-tool-cmdline.h
char * cvc_dir_orig; /**< @brief Where to look for the CVCA's certificate original value given at command line. */
const char *cvc_dir_help; /**< @brief Where to look for the CVCA's certificate help description. */
char * x509_dir_arg; /**< @brief Where to look for the CSCA's certificate (default=''). */
char * x509_dir_arg; /**< @brief Where to look for the CSCA's certificate (default='/home/fm/.local/etc/eac/x509'). */
Copy link
Member

Choose a reason for hiding this comment

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

I think these paths should not be included

src/tools/npa-tool-cmdline.c
" --cvc-dir=DIRECTORY Where to look for the CVCA's certificate\n (default=`')",
" --x509-dir=DIRECTORY Where to look for the CSCA's certificate\n (default=`')",
" --cvc-dir=DIRECTORY Where to look for the CVCA's certificate\n (default=`/home/fm/.local/etc/eac/cvc')",
" --x509-dir=DIRECTORY Where to look for the CSCA's certificate\n (default=`/home/fm/.local/etc/eac/x509')",
Copy link
Member

Choose a reason for hiding this comment

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

same here -- hardcoded path to your home.

src/tools/npa-tool-cmdline.c
args_info->cvc_dir_orig = NULL;
args_info->x509_dir_arg = gengetopt_strdup ("");
args_info->x509_dir_arg = gengetopt_strdup ("/home/fm/.local/etc/eac/x509");
Copy link
Member

Choose a reason for hiding this comment

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

bogus paths?

src/tools/npa-tool-cmdline.c
++found;
last = i;
if (strlen(values[i]) == len)
return i; /* exact macth no need to check more */
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return i; /* exact macth no need to check more */
return i; /* exact match no need to check more */

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jakuje Jakuje Jakuje requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@frankmorgner @AKing23T @Jakuje