Status access violation

[leonardodepaula]

When I updated from version 0.7.0 to 0.8.0 I ran into this error: (exit code: 0xc0000005, STATUS_ACCESS_VIOLATION)

fn main() {
    let pkcs11client = Pkcs11::new("C:/Windows/System32/eTPKCS11.dll").unwrap();
    pkcs11client.initialize(CInitializeArgs::OsThreads).unwrap();

    let slot = pkcs11client.get_slots_with_token().unwrap().remove(0);

    let session = pkcs11client.open_ro_session(slot).unwrap();
    session.login(UserType::User, None).unwrap();

    println!("{:?}", session);
}

Platform: Windows 11

The code above runs perfectly on version 0.7.0, but it doesn't on version 0.8.0.

[hug-dev]

Hey!

I tried your example but it fails with SoftHSM on both versions :( Couldn't find a commit which changed parts that would cause this to fail between 0.7.0 and 0.8.0. If you have time, you could git bisect to check...

Which line is failing in your code?
Have you also checked that giving None for pin would require CKF_PROTECTED_AUTHENTICATION_PATH (as shown here)?

[leonardodepaula]

Hello, @hug-dev!

First of all, thanks for the reply and the amazing work on the crate.

I'm prototyping here, so git bisect won't help.

This is the line failing:

let pkcs11client = Pkcs11::new("C:/Windows/System32/eTPKCS11.dll").unwrap();

I also tried setting the pin and the error persisted.

I made some tests and this is the last commit fully working:

cryptoki = { git = "https://github.com/parallaxsecond/rust-cryptoki.git", rev="024976fc892b96ad72fbc4af38cf9449c42034c4" }

So, the probable bug was introduced here: 3ec788c

[hug-dev]

ah nice find! Interesting, I did not spot anything in the commit that could have caused this isse 🤔
Maybe not the issue but #230 also changed those parts recently

[hug-dev]

Do you also know if your PKCS11 implementation (eTPKCS11.dll) implements the PKCS11 3.0 headers? That could be the issue with the commit you linked (although it was supposed to be retrocompatible)

[leonardodepaula]

Do you also know if your PKCS11 implementation (eTPKCS11.dll) implements the PKCS11 3.0 headers? That could be the issue with the commit you linked (although it was supposed to be retrocompatible)

I don't know. It is provided by Gemalto's Safenet Authentication Client. How can I check that?

I forked this repository and made two tests. Upgraded the pkcs11-headers to the version 3.1 and downgraded the headers to the version 2.40. On both scenarios the error persisted. I don't know the ins and outs of the cryptoki or the concrete implementation of this crate, but I think it indicates that the incompatibility doesn't come from the "cryptoki-sys/vendor/pkcs11.h" file.

[hug-dev]

How can I check that?

At least here in Supported APIs they say PKCS11 v2.20 😢

But I don't see how you would get an error from it..

[leonardodepaula]

I tried your example but it fails with SoftHSM on both versions :( Couldn't find a commit which changed parts that would cause this to fail between 0.7.0 and 0.8.0. If you have time, you could git bisect to check...

Are you getting an error with SoftHSM as well? What is the error?

[hug-dev]

I get:

thread 'main' panicked at cryptoki/examples/generate_key_pair.rs:27:41:
called `Result::unwrap()` on an `Err` value: Pkcs11(ArgumentsBad, Login)

on line

    session.login(UserType::User, None).unwrap();

[jrozner]

I'm also running into this on windows 11 using ykcs11. It works fine using the library on linux and macos.

[hug-dev]

I'm also running into this on windows 11 using ykcs11. It works fine using the library on linux and macos.

Do you also have the same offending commit? I don't have access to a Windows machine to test sadly 😢
We should probably run the CI on Windows too

[jrozner]

I confirmed that it doesn't work in 3ec788c but does work in it's parent commit 024976f.

[jrozner]

I enabled JIT debugging on windows and was able to catch the crash. The crash is here. I'm not clear exactly what this is doing.

The root cause is an indirect call to unmapped memory. This is the disassembly.

rax has 7FFF184196E00000 which is just wrong. The address looks shifted 16bits to the left.

Looking at the memory map, this aligns. Shifting it back would put it in ykcs11 (the pkcs11 library I'm using)

If I do the shift and look up the address it corresponds to C_Finalize within the library. Is that what it's supposed to be calling?

Seems like maybe a bug in the bindings or linking/loading? If you have some availability, I'm happy to do Windows testing/debugging for you. I just don't know enough about how the bindings are built to know exactly where to start.

[leonardodepaula]

Great work, @jrozner! I'm travelling with my family and will be able to help in two weeks. Thanks!

[leonardodepaula]

@Direktor799, since you commited 3ec788c, do you have any idea what kind of tests we could run to find what was the breaking change?

[Direktor799]

@Direktor799, since you commited 3ec788c, do you have any idea what kind of tests we could run to find what was the breaking change?

Hi, sorry for the delay, I might have some idea about this. Afaik pkcs11 standard does not define the alignment of the fields inside a struct, but normally vendors tend to use no alignment on Windows platforms at all.

See this from the old v2.4 header.

rust-cryptoki/cryptoki-sys/pkcs11.h

Lines 79 to 82 in 024976f

	#if defined(_WIN32) || defined(CRYPTOKI_FORCE_WIN32)
	/* There is a matching pop below. */
	#pragma pack(push, cryptoki, 1)

This does not exist in the new header. Considering the address shift issue @jrozner mentioned above, this is very likely to be the issue and adding the macros back should fix it. Does anyone want to try it? Or I can do it in a few weeks.

[jrozner]

I put up a PR that makes this change here. When I ran the script to re-generate the bindings, all the PTR types are missing. Is there something special I have to do to have bindgen create those? @hug-dev or someone else who is familiar with the build process?

[hug-dev]

First time I see something like that! Having looked a bit more at your change, it seems to be only the _PTR types declared with the ULONGDEF macro which are not created, but the _PTR-less types are 🤔 Something wonky with the pre-processor that bindgen uses? Maybe the syntax of the macro is not fully compliant?
You could try to first execute a pre-processor on the file (if it works) and then uses that.

About structure packing in general, we got this a while back.

edit: it also happens with STRUCTDEF macro so probably something with macro preprocessing!

[gshaurya18]

so probably something with macro preprocessing

That seems likely since ULONGDEF is a function like macro:

rust-cryptoki/cryptoki-sys/vendor/pkcs11.h

Lines 35 to 37 in 4cb53df

	#define ULONGDEF(__name__) \
	typedef CK_ULONG __name__; \
	typedef __name__ * __name__ ## _PTR;

and so is STRUCTDEF:

rust-cryptoki/cryptoki-sys/vendor/pkcs11.h

Lines 1066 to 1070 in 4cb53df

	#define STRUCTDEF(__name__) \
	struct __name__; \
	typedef struct __name__ __name__; \
	typedef struct __name__ * __name__ ## _PTR; \
	typedef struct __name__ ** __name__ ## _PTR_PTR;

I think adding clang_macro_fallback to the builder will likely get the the missing bindings as well.

Reference bindgen's Workaround for expansion of function-like macros

Edit: This option seems to only be available in bindgen 0.70+ (crate is on 0.69.4)

rust-cryptoki/cryptoki-sys/Cargo.toml

Line 16 in 4cb53df

bindgen = { version = "0.69.4", optional = true }