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/registry: Avoid storing full TLS certificates #2914

Merged
merged 3 commits into from
May 20, 2020

Conversation

kostko
Copy link
Member

@kostko kostko commented May 15, 2020

Fixes #2556

Previously the node registry descriptor contained full TLS certificates for
talking with nodes via gRPC. This changes it so that only TLS public keys are
used when verifying peer certificates for TLS authentication.

This makes the registry descriptors smaller and also makes it easier to pass
around TLS identities (as public keys are much shorter).

Obviously, this change BREAKS the consensus protocol and all previously
signed node descriptors.

The following configuration changes are needed due to this change:

  • In oasis-node registry node CLI, the --node.committee_address option
    has been renamed to --node.tls_address and the format has changed from
    <certificate>@ip:port to <pubkey>@ip:port.

  • For configuring sentry nodes on the workers, the
    --worker.sentry.cert_file has been removed. Instead, the
    --worker.sentry.address now takes the same address format as specified
    above (<pubkey>@ip:port).

Previously signed node descriptors (v0) are considered valid at genesis time
iff the node is exclusively a validator node as indicated by the role bits.
Other nodes will need to be removed from genesis.

TODO

@kostko kostko added s:blocked Status: blocked on other work c:breaking/consensus Category: breaking consensus changes c:breaking/cfg Category: breaks configuration and removed s:blocked Status: blocked on other work labels May 15, 2020
@kostko kostko force-pushed the kostko/feature/reg-tls-keys branch 6 times, most recently from 75ae996 to b2ee5dc Compare May 19, 2020 08:27
@codecov
Copy link

codecov bot commented May 19, 2020

Codecov Report

Merging #2914 into master will increase coverage by 0.57%.
The diff coverage is 68.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2914      +/-   ##
==========================================
+ Coverage   67.81%   68.38%   +0.57%     
==========================================
  Files         358      360       +2     
  Lines       34909    34937      +28     
==========================================
+ Hits        23673    23893     +220     
+ Misses       8175     7965     -210     
- Partials     3061     3079      +18     
Impacted Files Coverage Δ
go/consensus/tendermint/apps/scheduler/query.go 69.44% <0.00%> (ø)
go/worker/common/configparser/configparser.go 60.00% <ø> (-0.98%) ⬇️
go/common/crypto/tls/verify.go 32.69% <32.69%> (ø)
go/registry/api/legacy_v0.go 37.83% <37.83%> (ø)
.../consensus/tendermint/apps/registry/state/state.go 58.87% <40.00%> (-0.67%) ⬇️
go/sentry/api/grpc.go 54.16% <41.66%> (ø)
go/common/entity/entity.go 68.23% <45.45%> (+5.73%) ⬆️
go/common/node/address.go 71.96% <50.00%> (-2.35%) ⬇️
go/common/accessctl/accessctl.go 84.61% <55.55%> (-15.39%) ⬇️
go/registry/api/api.go 39.42% <63.82%> (+1.09%) ⬆️
... and 62 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 2341d2b...476b7ec. Read the comment docs.

@kostko kostko force-pushed the kostko/feature/reg-tls-keys branch 3 times, most recently from 988c67b to 944a023 Compare May 19, 2020 12:07
@kostko kostko marked this pull request as ready for review May 19, 2020 12:08
docs/oasis-node/metrics.md Outdated Show resolved Hide resolved
Previously the node registry descriptor contained full TLS certificates for
talking with nodes via gRPC. This changes it so that only TLS public keys are
used when verifying peer certificates for TLS authentication.

This makes the registry descriptors smaller and also makes it easier to pass
around TLS identities (as public keys are much shorter).

Obviously, this change BREAKS the consensus protocol and all previously
signed node descriptors.
@kostko kostko force-pushed the kostko/feature/reg-tls-keys branch from 944a023 to 55086bf Compare May 19, 2020 16:11
@kostko kostko force-pushed the kostko/feature/reg-tls-keys branch from 55086bf to 0f8381b Compare May 20, 2020 07:42
@kostko kostko merged commit 08e237f into master May 20, 2020
@kostko kostko deleted the kostko/feature/reg-tls-keys branch May 20, 2020 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:breaking/cfg Category: breaks configuration c:breaking/consensus Category: breaking consensus changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

registry: avoid storing full tls certificates
2 participants