parallaxsecond / parallaxsecond/rust-cryptoki

Pack structs in C bindings

Open
#70 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
115
Forks
96
Avg merge
22h 56m
Merged PRs (30d)
2

Description

This creates a separate issue out of the latter part of this comment and the one that follows.

When bindings are generated from the C headers in cryptoki-sys, they do so using whatever structure alignment and packing is assumed for the target platform unless packing is made explicit in the headers for bindgen to read. Currently, packing is only specified for Windows and left implied everywhere else.

You can confirm that forcing a 1-byte alignment for structs on other platforms that it does indeed produce bindings that differ in terms of size and field offsets, and is not the implicit default.

Meanwhile, the PKCS#11 standard (both 2.x and 3.x, Section 2.1) are very clear that

Cryptoki structures are packed to occupy as little space as is possible. Cryptoki structures SHALL be packed with 1-byte alignment.

This would seem to imply that packing be explicit for all target bindings. But when this is done, several problems arise:

  1. Rust assumes a >1-byte minimum alignment for struct members, which makes referencing anything beyond the 0th item undefined behavior. Each such instance of this (hundreds in the auto-generated tests) produces a valid, unsuppressible compiler warning. This is a known issue for bindgen that doesn't appear to be nearing a solution any time soon.
  2. Tests written for this crate using SoftHSM seg fault. Whether this is at the rust level (dereferencing unaligned addresses) or at the C level (mismatch with struct packing internal to SoftHSM) is unclear. The latter doesn't seem like it should be the case, but the fact that tests are currently passing would seem to be an endorsement of implicit, non-compact alignment.

So, something is incorrect here, but what exactly that is needs to be investigated. Even if it turns out the way the bindings are currently generated is correct, that fact should still be documented conspicuously to avoid further misconception.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in cryptoki-sys where bindgen generates bindings from the C headers, then compare struct sizes and field offsets with and without explicit 1-byte packing. Run the SoftHSM-backed tests and inspect the generated tests for unaligned-access warnings. Done means establishing the correct PKCS#11 packing behavior, fixing the bindings and tests if needed, or documenting conspicuously why the current generation is correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, rust
Domain
cryptography, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.