Skip to content
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

go/common/crypto/signature: Add and use the plugin backed signer #3120

Merged
merged 3 commits into from
Jul 22, 2020

Conversation

Yawning
Copy link
Contributor

@Yawning Yawning commented Jul 21, 2020

Bloating the repository with a ton of different HSM (etc) signing backends doesn't make sense, and is a maintenance burden. Use the runtime's built-in DLSO support to be able to separate out the non-esssential implementations.

  • Add the plugin signer implementation
  • Change oasis-node to instantiate the plugin signer.
  • Add tests for the plugin signer.

After merge:

Notes:

  • The plugin loader enforces (rather strictly) that the package versions of plugins are the same as what the loading application was built with. This will require everything be built with -trimpath, but we should be doing that already anyway. See plugin: panic: plugin was built with a different version of package golang/go#27751 for more information.
  • This is undocumented because anyone that can't figure it out, has zero business writing anything that is even vaguely security critical. That said there's tests and an example now.
  • golang-ci-lint got mad when linting the plugin because it doesn't have main defined, so it is skipped for now.

@Yawning Yawning force-pushed the yawning/feature/die-ledger-die branch 4 times, most recently from 79247c6 to da297d4 Compare July 21, 2020 08:46
@codecov
Copy link

codecov bot commented Jul 21, 2020

Codecov Report

Merging #3120 into master will increase coverage by 0.34%.
The diff coverage is 13.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3120      +/-   ##
==========================================
+ Coverage   68.60%   68.95%   +0.34%     
==========================================
  Files         375      376       +1     
  Lines       37037    37059      +22     
==========================================
+ Hits        25409    25553     +144     
+ Misses       8389     8273     -116     
+ Partials     3239     3233       -6     
Impacted Files Coverage Δ
...o/common/crypto/signature/signers/plugin/plugin.go 0.00% <0.00%> (ø)
go/oasis-node/cmd/common/signer/signer.go 52.68% <37.50%> (-12.43%) ⬇️
go/oasis-node/cmd/common/metrics/disk.go 65.51% <0.00%> (-20.69%) ⬇️
...n/crypto/signature/signers/ledger/ledger_signer.go 0.00% <0.00%> (-14.00%) ⬇️
go/runtime/host/sandbox/sandbox.go 67.28% <0.00%> (-10.04%) ⬇️
go/oasis-node/cmd/common/metrics/resource.go 84.00% <0.00%> (-8.00%) ⬇️
go/runtime/tagindexer/tagindexer.go 68.47% <0.00%> (-4.35%) ⬇️
.../consensus/tendermint/apps/epochtime_mock/state.go 74.46% <0.00%> (-4.26%) ⬇️
go/storage/mkvs/tree.go 89.28% <0.00%> (-3.58%) ⬇️
... and 44 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5045419...9d0f823. Read the comment docs.

@Yawning Yawning force-pushed the yawning/feature/die-ledger-die branch 4 times, most recently from 99735b8 to 0de853b Compare July 21, 2020 11:16
@Yawning Yawning force-pushed the yawning/feature/die-ledger-die branch 4 times, most recently from 44669cd to e9d230d Compare July 21, 2020 13:04
@Yawning Yawning marked this pull request as ready for review July 21, 2020 13:05
go/.golangci.yml Show resolved Hide resolved
Bloating the repository with a ton of different HSM (etc) signing
backends doesn't make sense, and is a maintenance burden.  Use the
runtime's built-in DLSO support to be able to separate out the
non-esssential implementations.
@Yawning Yawning force-pushed the yawning/feature/die-ledger-die branch from e9d230d to 9d0f823 Compare July 22, 2020 08:16
@Yawning Yawning merged commit 9a177cb into master Jul 22, 2020
@Yawning Yawning deleted the yawning/feature/die-ledger-die branch July 22, 2020 08:34
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.

None yet

2 participants