Uncontroversial changes from PR #850 #996
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sc_pkcs15_unblock_pin() in libopensc/pkcs15-pin.c wants to associate PIN to be unblocked with its PUK to check, for example, whether provided PUK conforms to its policy. When this function is not able to find a relevant PUK is uses policy for PIN to be unblocked instead to check provided PUK which causes problems if PIN and PUK policies differ. Set PIN-PUK association for cards where it was unset and where this association was either obvious, described in code or specs or provided by the community. Signed-off-by: Maciej S. Szmigiero <[email protected]>
OpenSC used SUSv3 "z" printf length modifier for printing size_t variables,
however this modifier is not available on Windows ("I" must be used
instead), at least for now.
Introduce SC_FORMAT_LEN_SIZE_T define for that purpose and convert existing
code to use it when printing size_t variables.
This define can't go into libopensc/internal.h since tools use it, too.
Signed-off-by: Maciej S. Szmigiero <[email protected]>
Minidriver installer custom action library needs WiX SDK to build. Since installer is an optional component anyway let's detect whether WiX SDK is present on build platform and then decide whether to build installer custom action library or not. Signed-off-by: Maciej S. Szmigiero <[email protected]>
Currently, minidriver build is broken on mingw. Let's make it work again. For this, include adapted cardmod-mingw-compat.h with few function argument decorations from Alon Bar-Lev's old build repository to make mingw build almost self-contained - still requires cardmod.h from CNG, however. Signed-off-by: Maciej S. Szmigiero <[email protected]>
This commit fixes most of warnings shown by GCC on 64-bit Linux, 32-bit and 64-bit mingw builds (with SM and OpenSSL enabled). These warnings were mostly caused by missing casts. In minidriver there was also a bit of unused variables and dead code. Remaining warnings on mingw are mostly caused by GCC not recognizing on this platform "ll" size specifier (present at least since Visual Studio 2005, also in mingw own CRT) and "z" size specifier (this one will be fixed in next commits). There is also a warning about pointer truncation on Win64 when making PKCS#11 object handle from pointer to this object. This is a legitimate warning, since it could result in the same handles being generated from different pointers and so from different objects. Signed-off-by: Maciej S. Szmigiero <[email protected]>
SM test in configure.ac makes use of LIB_PRE and DYN_LIB_EXT variables so let's move it further down in this file, just after these variables are assigned. Signed-off-by: Maciej S. Szmigiero <[email protected]>
C_OpenSession() creates a long int session handle from address of allocated session struct, however it has to be taken into consideration that on Win64 long int is still 32-bit, so the address is going to be truncated and because of that not guaranteed to be unique. Add session handle uniqueness check to catch when there is already a session with the same handle present. This also fixes a warning when building on 64-bit mingw. Signed-off-by: Maciej S. Szmigiero <[email protected]>
Add "--reset" parameter with optional argument to opensc-tool which resets a card in reader. Both cold or warm resets are possible (cold is default). Signed-off-by: Maciej S. Szmigiero <[email protected]>
Some of existing code prints pointer differences, but without taking into account that printf length modifier required for this differs between systems. Add SC_FORMAT_LEN_PTRDIFF_T macro for this, just as we have for size_t variables. Signed-off-by: Maciej S. Szmigiero <[email protected]>
Minidriver contained a hack since commit 7ef766b in 2010 to print to debug file directly under mingw (instead of using normal OpenSC logging system), as there was problem with "%S" format specifier then. However, on recent mingw versions "%S" format works fine so let's remove this hack. Signed-off-by: Maciej S. Szmigiero <[email protected]>
Minidriver currently has basic support for unblocking card PIN by providing PUK as an administrator password to CardUnblockPin() function. However, this doesn't work for example when trying to unblock PIN via system smartcard PIN unblock screen accessible after pressing Ctrl+Alt+Del as it wants to use challenge / response authentication. MS Smart Card Minidriver specification (version 7.07) explicitly says that challenge / response is the only authentication mode that Windows uses to authenticate an administrator. Unfortunately, this way of unblocking PIN seems to not be widely supported by cards. However, we can simply treat the provided response to challenge as PUK. Because (at least) Ctrl+Alt+Del PIN unblock screen accepts only hex string, every PUK digit X has to be input as '3X' (without quotes) there. Also the response string is not hidden behind asterisks on this screen as it should been. Signed-off-by: Maciej S. Szmigiero <[email protected]>
GCC can check format and parameter correctness in printf()-like functions for us so let's add necessary attributes to our log functions to emit a warning where their way of being called is likely in need to be inspected for correctness. Signed-off-by: Maciej S. Szmigiero <[email protected]>
Since "Add GCC format checking attributes to log functions" commit GCC warns us about problems with format strings and their arguments provided to OpenSC message logging functions. This commit fixes all cases where GCC warned about incorrect format on 64-bit Linux, 32-bit and 64-bit mingw builds (with SM and OpenSSL enabled). Well, almost all since on mingw GCC does not recognize "ll" size specifier (present at least since Visual Studio 2005, also in mingw own CRT) so these (few) warnings about it remain. In most cases format size specifier for size_t type was missing (usually size was left at default int level, with is different on 64-bit x86). Some formats had too few / too many arguments. In some cases pointers were printed as integers. Some long variables were missing "l" prefix (especially with regard to %x format). Signed-off-by: Maciej S. Szmigiero <[email protected]>
Looks like Travis CI build server found a few cases of log function format string not being a string literal (now that log functions have necessary attributes to check for such things). Some instances clearly aren't a real problem, but to be future-proof and to avoid compiler warnings let's fix all of them (that I was able to find in code). Signed-off-by: Maciej S. Szmigiero <[email protected]>
Mingw currently links to msvcrt.dll as C runtime. This library is documented by Microsoft as off-limits to applications and its feature set vary between Windows versions. Due to this, presence of particular printf() format string directives depends on which Windows version the code is run. This is, naturally, bad, so mingw developers introduced ability to replace formatted output functions with built-in equivalents with defined feature set by setting "__USE_MINGW_ANSI_STDIO" macro to 1. There are, however, no built-in equivalents for "_s" suffixed functions. Fortunately, they are used only a few times in minidriver so let's simply replace them with equivalent code using standard functions. This also allows skipping "MINGW_HAS_SECURE_API" macro definition so any future uses will be caught by compiler. Signed-off-by: Maciej S. Szmigiero <[email protected]>
Commit "Add GCC format checking attributes to log functions" added format and parameter checking to OpenSC log functions. Minidriver, however, logs most of its output via a dedicated log function, so this function needs such attributes, too. Signed-off-by: Maciej S. Szmigiero <[email protected]>
Since last commit GCC warns us about problems with format strings and their arguments in minidriver, so let's fix these warnings just as we did in rest of the OpenSC code. Most of these warnings were about DWORDs being printed as ints, there were also some format directives and size_t size specifiers missing and various misc format / parameter disagreements. Attempt was made to keep log strings as-is, only the most obvious typos were fixed. Signed-off-by: Maciej S. Szmigiero <[email protected]>
According to minidriver specs CardGetChallenge() method parameters are purely for output and do not have a meaning of requested challenge length, so remove a misleading log line. There is also no need to have a special case for pcbChallengeData being NULL since in this case the function would have exited early anyway with SCARD_E_INVALID_PARAMETER (also, it was just dereferenced in the previous code line). Signed-off-by: Maciej S. Szmigiero <[email protected]>
According to minidriver specs CardReadFile() method output parameters are optional so don't return SCARD_E_INVALID_PARAMETER when they are NULL. Also, use this opportunity to walk through this function helpers to make sure they correctly return error status. Signed-off-by: Maciej S. Szmigiero <[email protected]>
|
thanks, I'll try to look at the changes next week. |
frankmorgner
approved these changes
Mar 20, 2017
|
Thanks. |
|
resolved conflicts, and merged the changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request contains uncontroversial commits from PR #850 as suggested by @frankmorgner .
Specifically, it excludes the following commits:
Other than pulling out these commits and rebasing on top of current master there are also two
additional changes from last version in PR #850: