Skip to content

hide pkcs11 behind a build tag so that fsc is pure go by default #621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

arner
Copy link
Contributor

@arner arner commented Jul 9, 2024

The github.com/hyperledger/fabric/bccsp/pkcs11 dependency is the only reason we need CGO_ENABLED=1 when building an application that imports FSC. It makes things like cross-architecture builds unnecessarily difficult.

This PR makes it so that if you want to use pkcs11, you have to build the binary with go build -tags pkcs11. It includes a default implementation that panics if the user configures HSM in a binary that's not built with pkcs11 as a build tag.

Copy link
Contributor

@adecaro adecaro left a comment

Choose a reason for hiding this comment

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

So, can we test this PR against the token-sdk? The token-sdk has a few integration tests where pkcs11 is used. I would like to make sure this works there too.

@arner
Copy link
Contributor Author

arner commented Jul 31, 2024

So, can we test this PR against the token-sdk? The token-sdk has a few integration tests where pkcs11 is used. I would like to make sure this works there too.

This branch uses the current FSC commit and the code for FTS to support pkcs11 too: https://github.com/hyperledger-labs/fabric-token-sdk/actions/runs/10175721245?pr=721. As far as I can tell, the only failing tests are due to flakiness, but HSM works.

Copy link
Contributor

@adecaro adecaro left a comment

Choose a reason for hiding this comment

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

LGTM

@adecaro adecaro merged commit ae3778f into main Aug 5, 2024
18 checks passed
@adecaro adecaro deleted the f-opt-in-hsm branch August 5, 2024 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants