Fix handling empty AllowedMechanisms attribute

[Jakuje]

As returned by SoftHSM: softhsm/SoftHSMv2#825

Fixes: #323

[Jakuje]

OTOH this could be considered a softhsm bug (or at least oddness) and as a rust-cryptoki, we might want to shield a users from such pkcs11-module-specific issues. But then we would have to filter this out early in get_attributes, which does not sound great either.

[nwalfield]

You already pointed out that at least two attributes can definitely be zero length. It thus seems like a bug in rust-cryptoki that it tries to allocate 0 bytes and expects to get a pointer that can be cast to a raw pointer and passed to std::slice::from_raw_parts:

Behavior is undefined if any of the following conditions are violated:

    data must be non-null, valid for reads for len * size_of::<T>() many bytes, and it must be properly aligned

[wiktor-k]

You already pointed out that at least two attributes can definitely be zero length. It thus seems like a bug in rust-cryptoki that it tries to allocate 0 bytes and expects to get a pointer that can be cast to a raw pointer and passed to std::slice::from_raw_parts:

Behavior is undefined if any of the following conditions are violated:

    data must be non-null, valid for reads for len * size_of::<T>() many bytes, and it must be properly aligned

Validity rules for reads explicitly point out that:

For memory accesses of size zero, every pointer is valid, including the null pointer.

I'm okay with the change here, with a bit of docs around it, since at first sight it looks like a premature optimization 🤔

[nwalfield]

You already pointed out that at least two attributes can definitely be zero length. It thus seems like a bug in rust-cryptoki that it tries to allocate 0 bytes and expects to get a pointer that can be cast to a raw pointer and passed to std::slice::from_raw_parts:

Behavior is undefined if any of the following conditions are violated:

    data must be non-null, valid for reads for len * size_of::<T>() many bytes, and it must be properly aligned

Validity rules for reads explicitly point out that:

For memory accesses of size zero, every pointer is valid, including the null pointer.

I'm okay with the change here, with a bit of docs around it, since at first sight it looks like a premature optimization 🤔

That's not the only relevant thing. From slice::from_raw_parts:

data must be non-null and aligned even for zero-length slices or slices of ZSTs.

And that's the bug that we are seeing.

[neverpanic]

Typo, rest lg2m.

[hug-dev]

Thanks for the fix and the comment ⭐

[Jakuje]

@wiktor-k can you have a look and approve if this looks ok?

[wiktor-k]

I think this looks good 👌 a couple of small suggestions, if you don't mind.

Btw, thanks for the ping, quite a lot on the plate recently 😅

[Jakuje]

oh, no. Something in the ubuntu broke the installation of the 32b arch softhsm ...

[hug-dev]

Is it this line failing? Might be ok to ignore for now...

sudo dpkg --add-architecture i386

[Jakuje]

Is it this line failing? Might be ok to ignore for now...

sudo dpkg --add-architecture i386

Hard to say. Would have to try that locally or to get more information from the CI. I am also curious about the kryoptic failure that was not there in #328, in CI which ran last week.

[Jakuje]

Is it this line failing? Might be ok to ignore for now...

sudo dpkg --add-architecture i386

Looks like it comes from the last line:

+ sudo apt-get install -y -qq gcc-multilib:i386 libsofthsm2:i386 gcc:i386
E: Error, pkgProblemResolver::Resolve generated breaks, this may be caused by held packages.

Removing the -qq gives more information, but they look related to the github build image (but I am really not sure):

+ sudo apt-get install -y gcc-multilib:i386 libsofthsm2:i386 gcc:i386
Reading package lists...
Building dependency tree...
Reading state information...
Some packages could not be installed. This may mean that you have
requested an impossible situation or if you are using the unstable
distribution that some required packages have not yet been created
or been moved out of Incoming.
The following information may help to resolve the situation:

The following packages have unmet dependencies:
 binutils : Conflicts: binutils:i386 but 2.42-4ubuntu2.7 is to be installed
 binutils:i386 : Conflicts: binutils but 2.42-4ubuntu2.6 is to be installed
 binutils-common : Breaks: binutils-common:i386 (!= 2.42-4ubuntu2.6) but 2.42-4ubuntu2.7 is to be installed
 binutils-common:i386 : Breaks: binutils-common (!= 2.42-4ubuntu2.7) but 2.42-4ubuntu2.6 is to be installed
 libbinutils : Breaks: libbinutils:i386 (!= 2.42-4ubuntu2.6) but 2.42-4ubuntu2.7 is to be installed
 libbinutils:i386 : Breaks: libbinutils (!= 2.42-4ubuntu2.7) but 2.42-4ubuntu2.6 is to be installed
 libctf0 : Breaks: libctf0:i386 (!= 2.42-4ubuntu2.6) but 2.42-4ubuntu2.7 is to be installed
 libctf0:i386 : Breaks: libctf0 (!= 2.42-4ubuntu2.7) but 2.42-4ubuntu2.6 is to be installed
 libgprofng0 : Breaks: libgprofng0:i386 (!= 2.42-4ubuntu2.6) but 2.42-4ubuntu2.7 is to be installed
 libgprofng0:i386 : Breaks: libgprofng0 (!= 2.42-4ubuntu2.7) but 2.42-4ubuntu2.6 is to be installed
E: Error, pkgProblemResolver::Resolve generated breaks, this may be caused by held packages.

[hug-dev]

I think it should be ok to remove those two failing 32 bits and open an issue to see how we can fix it until then. What about the failing kryoptic test though, do you have any idea of what could be wrong there?

[Jakuje]

I think it should be ok to remove those two failing 32 bits and open an issue to see how we can fix it until then.

Looking at the CI configuration, these are not required to pass before merging. So we can probably keep them aronud

What about the failing kryoptic test though, do you have any idea of what could be wrong there?

I tried to reproduce it locally, but I was not successful (but I have likely slightly different openssl version). Its the ed25519 key. We consider these approved under FIPS so it should not be an issue. I will try to dig into that later today.

[Jakuje]

The softhsm issue is solved, but the kryoptic issue looks like failing inside of the openssl with debug logging turned on:

Error: 0 [ERROR] Openssl Error(50856204): "FIPS internal library context, Algorithm (ED25519CTX : 0), Properties (<null>)"
Error: 0 [ERROR] ossl/src/signature.rs:37: EVP_SIGNATURE_fetch() failed: []
Error: 0 [ERROR] Generic CK error, CKR_DEVICE_ERROR(48)
test sign_verify_eddsa_with_ed25519_schemes ... FAILED

Note, that this is already the second iteration in the test. The EddsaSignatureScheme::Ed25519 worked ok, but the ddsaSignatureScheme::Ed25519ctx(b"context") does not look like working now.

I guess this is some change inside of OpenSSL: And looks like its this one, which landed in openssl 3.5 branch during last two weeks:

openssl/openssl@1b67b76

That said, we will have to skip this signature scheme with fips kryoptic. Let me do that in separate PR so we can merge it irrespective of this PR.

[Jakuje]

filled #331. And rebased this one on top of #331.

[Jakuje]

Rebased. Should be also ready