card-jpki: Use sc_file_free() to release virtual MF#8
Closed
hamano wants to merge 4371 commits into
Closed
Conversation
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
The maximum path length is 16 bytes and if we want to extend the path, we need to make sure its not too long. Thanks oss-fuzz. https://issues.oss-fuzz.com/issues/467161860 Signed-off-by: Jakub Jelen <jjelen@redhat.com>
... to better support FIPS mode Fixes: OpenSC#3500 Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Raul Metsma <raul@metsma.ee>
Signed-off-by: Raul Metsma <raul@metsma.ee>
Signed-off-by: Raul Metsma <raul@metsma.ee>
A cast introduced in commit 767c340 causes padding errors to return immediately on 64-bit platforms, breaking the constant-time handling of PKCS#1 v1.5 decryption errors. On 64-bit systems, casting the result of constant_time_eq_i() to size_t before applying bitwise NOT produces 0xffffffff00000000 instead of zero when the values match, causing the wrong branch to be taken. This timing discrepancy creates a padding oracle that can be exploited for Bleichenbacher/Marvin-style attacks against PKCS#1 v1.5 decryption when software unpadding is used. The fix uses constant_time_eq_s with size_t arguments to ensure proper full-width mask handling on 64-bit platforms. Reported-by: Pavel Kohout, Aisle Research, www.aisle.com
The autoconf in Fedora rawhide does not seem to correctly detect the support for `const`: ``` checking for an ANSI C-conforming const... no ``` which is resulting in weird errors where we are passing const to non-const arguments, while having `const` in place. The autoconf documentation says this macro is obsolescent so lets drop it before it will cause more issues: https://www.gnu.org/software/autoconf/manual/autoconf-2.60/html_node/C-Compiler.html Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Turns out softhsm returns function list with version 3 from GetFunctionList, which will cause we overrun the structure we get from `C_LoadModule()`. This is fixed by copying the provided function list to our memory and checking that we really have the Interface function before calling them. softhsm/SoftHSMv2#839
/* filename must be "http://" <DNS name> "/" <ASCII-HEX encoded SHA-256 hash of OffCardKeyHistoryFile> On branch piv-history-fix Changes to be committed: modified: card-piv.c
Fixed CID 501272: Resource leaks
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Allow reading data group from eMRTD with npa-tool
On branch piv-history-fix Changes to be committed: modified: card-piv.c
Changes to be committed: modified: card-piv.c On branch piv-history-fix Changes to be committed: modified: card-piv.c
PIV: fix possible stack buffer overflow when handling history object Thanks to Nicholas Carlini <npc@anthropic.com> for reporting this issue
This detects changes of reader/card without actually querying the device and avoids IO latencies.
…#3697) * CI: Added workflow to manage inactive PR/Issues automatically * removed unnecessary permissions * updated timeframes per discussion
If we detect an OpenSSL error print out the OpenSSL error message.
Originally reported by @TristanInSec. Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Originally reported by @TristanInSec. Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Thanks oss-fuzz https://issues.oss-fuzz.com/issues/525823616 Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Bumps [actions/cache](https://github.com/actions/cache) from 5 to 6. - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](actions/cache@v5...v6) --- updated-dependencies: - dependency-name: actions/cache dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
It is not working with jCardSim anymore, see OpenSC#3632 (comment)
Origianlly reported and patch provided by Mitchell Benjamin of Revamp Studio
Origianlly reported by Mitchell Benjamin of Revamp Studio
Origianlly reported and initial patch provided by Mitchell Benjamin of Revamp Studio
Origianlly reported by Mitchell Benjamin of Revamp Studio
Origianlly reported by Mitchell Benjamin of Revamp Studio
This fixes the overrun, but writing 2k nor 3k RSA keys is not possible due to the fixed buffer being too small, see the FIXME comment. Origianlly reported by Mitchell Benjamin of Revamp Studio
Signed-off-by: olszomal <Malgorzata.Olszowka@stunnel.org>
The configure.ac tries to make sure that at most 1 reader is selected, but misses the cases when all are disabled. Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
…ard) Windows selects the smart card minidriver primarily by ATR, so the minidriver registration needs the ATRs of the JPKI cards. Add the contact and contactless ATRs of the My Number Card. The contactless ATRs are those constructed from the ATQB by PC/SC part 3 compliant readers, which are stable per card type. Readers that embed the random PUPI into the ATR cannot be matched this way. Card detection in card-jpki.c continues to rely on AID selection; this only affects the Windows minidriver registration. Refs: OpenSC#3021
jpki_finish() released drvdata->mf with plain free(), but the file is created by sc_file_new() in jpki_init() and has five ACL entries attached via sc_file_add_acl_entry(). Plain free() releases only the top-level struct, leaking the ACL entries (and any *_attr allocations). Use sc_file_free(), which properly clears ACL entries and sub-allocations. It also handles NULL internally, so the explicit NULL check is dropped.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
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.
Summary
jpki_finish()releaseddrvdata->mfwith a plainfree(). However, thefile is created via
sc_file_new()injpki_init()and has five ACLentries attached through
sc_file_add_acl_entry()(
SELECT,LIST_FILES,LOCK,DELETE,CREATE).free()only releases the top-levelsc_file_tstruct, leaking the linkedACL entries (and any
*_attrallocations). This is a resource leak and anAPI contract violation.
Fix
Use
sc_file_free(), which clears ACL entries and sub-allocations beforefreeing the struct. Since
sc_file_free()handlesNULLinternally, theexplicit NULL check is dropped as well.
Found during a security review of the JPKI driver. No memory-corruption
class issue; this is a leak / correctness fix.