-
Notifications
You must be signed in to change notification settings - Fork 716
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
Allow reading data group from eMRTD with npa-tool #2257
base: master
Are you sure you want to change the base?
Conversation
This pull request introduces 1 alert when merging 1cc2b9e into 1ef79e9 - view on LGTM.com new alerts:
|
I change and compile the branch code, now npa-tool show nothing when read dg2 or dg14. ./npa-tool --can=xxxxxx --read-dg2 |
this is my result:
|
This may be a parsing problem after successfully reading the DG, could you post a log? |
from https://github.com/frankmorgner/OpenSC/tree/fix_read_dg to git clone https://github.com/frankmorgner/OpenSC.git log for pcsc log for opensc |
The log indicates that you're not using the code from this pull request.
See Line 666 in 1cc2b9e
Please try again... |
@AKing23T do you have any update on this? |
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. |
Adds the new options --application and --read-all-dgs fixes OpenSC#2253
a negative return value was incorrectly translated to a huge buffer length, which then errors with "offset too large"
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.
1cc2b9e
to
60bf597
Compare
I've fixed printing the output by supplying a large enough buffer and tried some better integration. Use the following for testing:
|
This pull request introduces 1 alert when merging 60bf597 into 2cc7b10 - view on LGTM.com new alerts:
|
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'). */ |
There was a problem hiding this comment.
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
" --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')", |
There was a problem hiding this comment.
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.
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"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bogus paths?
++found; | ||
last = i; | ||
if (strlen(values[i]) == len) | ||
return i; /* exact macth no need to check more */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return i; /* exact macth no need to check more */ | |
return i; /* exact match no need to check more */ |
Checklist