Skip to content

feat: HSM auth - c8y connection init & built in bridge#3366

Merged
Bravo555 merged 7 commits intothin-edge:mainfrom
Bravo555:feat/hsm-auth
Feb 27, 2025
Merged

feat: HSM auth - c8y connection init & built in bridge#3366
Bravo555 merged 7 commits intothin-edge:mainfrom
Bravo555:feat/hsm-auth

Conversation

@Bravo555
Copy link
Copy Markdown
Member

@Bravo555 Bravo555 commented Jan 28, 2025

TODO

  • add documentation wrote a reference doc and the draft of a user guide, subject to change because details related to the UNIX socket setup are still being hashed out
  • resolve current hacks and workarounds in places where we assume private key always comes from a file there were some hacks but they're now removed
  • Add new cargo feature called "cryptoki" where all of the dependencies related to the cryptoki are hidden below it (and the feature is not activated by default). Configuration can be marked as hidden until configuration names are agreed upon

Follow-up items

  • return a config error before connecting (i.e. at the config parsing stage) if device.cryptoki.enable is true, but module path is not set
  • include the HSM configuration in the tedge connect summary
  • refactor TLS configuration setup 2c27e33
  • upstream libloading musl patch
  • extend support to aws and azure clouds
  • testing: use softHSM for mocking HSMs and testing of the signing flow

Proposed changes

Allow the use of Hardware Security Modules for MQTT client authentication when using c8y cloud.

We add the following config settings:

     device.cryptoki.enable  Use a Hardware Security Module for authenticating the MQTT connection with the cloud.  When set to true, `key_path` option is ignored as PKCS#11 module is used for signing. 
                             Examples: true, false
device.cryptoki.module_path  A path to the PKCS#11 module used for interaction with the HSM. 
                             Example: /usr/lib/x86_64-linux-gnu/opensc-pkcs11.so
        device.cryptoki.pin  Pin value for logging into the HSM. 
                             Example: 123456
     device.cryptoki.serial  A serial number of a Personal Identity Verification (PIV) device to be used.  Necessary if two or more modules are connected. 
                             Example: 123456789

which are then used for configuring HSM authentication parameters.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#3363

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I ran cargo fmt as mentioned in CODING_GUIDELINES
  • I used cargo clippy as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 28, 2025

@Bravo555 Bravo555 changed the title feat: HSM auth - c8y direct connection feat: HSM auth - c8y connection init & built in bridge Jan 29, 2025
@Bravo555 Bravo555 temporarily deployed to Test Pull Request January 30, 2025 13:45 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 30, 2025

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
583 0 3 583 100 1h38m56.929774999s

@reubenmiller
Copy link
Copy Markdown
Contributor

reubenmiller commented Feb 13, 2025

@Bravo555 We need to check the naming of the device.cryptoki.* settings, and get a clear idea on how to configure multiple cloud connection which use different certificates.

Some thoughts on the subject:

  • Enforcing the usage of the HSM for all cloud profiles might be worth while so that if it is used for cloud A, that people don't forget to use it for cloud B.
  • Support configuring different certificate serial numbers per cloud profile (as we already support using independent cloud certificates so we should support the same thing when using a HSM)

@reubenmiller reubenmiller added the theme:security Theme: Security related topics label Feb 14, 2025
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that PKCS#11 support is behind a feature, I see no blockers to merge this PR.

It would be good to fix the many TODO(marcel) added along this work. And to remove dead code.

Copy link
Copy Markdown
Contributor

@reubenmiller reubenmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

I've tested the latest changes using a Yubikey 5C and inside a container (using p11-kit server to setup a unix socket which is passed into the container)..the test was done using https://github.com/reubenmiller/hsm-research/tree/main/tedge.

Overall a nice addition to lay the groundwork for the next phase of implementation.

Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Thank you for taking on this PKCS#11 integration challenge. A first step but a decisive one.

Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
This new draft reference document aims to track the state of HSM/PKCS#11
support in thin-edge and will be updated along with the implementation.

Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
Adding cryptoki configuration to the method is non-trivial and the
method IMO doesn't provide much value anyway, since the errors it
reports are going to be returned a bit later when the bridge actually
tries to start.

Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
@Bravo555
Copy link
Copy Markdown
Member Author

failed due to flaky test #3399; retrying merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Change request skip-release-notes Don't include the ticket in the auto generated release notes theme:security Theme: Security related topics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants