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

#850 accepted and rebased #979

Closed
wants to merge 15 commits into from

Conversation

frankmorgner
Copy link
Member

This PR covers most patches from #850 that have been discussed and accepted. These commits were deliberately excluded:

I omitted some other Patches from #850, because they resulted in conflicts due to changes that were not accepted. If you could help resolve the conflicts, @maciejsszmigiero, we can include those as well.

Currently, the unblocking per 2a9e175 is undocumented. That should be written down somewhere...

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 or described in code or specs.

This leaves SmartCard-HSM for which the same change should possibly be
done, but wasn't since I don't have enough information about it.

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]>
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]>
@frankmorgner
Copy link
Member Author

Actually, the cherry-picked changes don't compile. @maciejsszmigiero could you create a PR without those three commits resolving the conflicts? This could then be quickly merged.

@frankmorgner frankmorgner deleted the 850-accepted-rebased branch July 31, 2017 23:33
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

2 participants