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

ias: Access control #2018

Merged
merged 16 commits into from
Aug 28, 2019
Merged

ias: Access control #2018

merged 16 commits into from
Aug 28, 2019

Conversation

Yawning
Copy link
Contributor

@Yawning Yawning commented Aug 19, 2019

Intended to fix #1998, #1843 (partial).

  • go/worker/compute: Defer runtime registration till Start(). (This causes mountains of pain, revisit)
  • Add the MRSIGNER/MRENCLAVE pair to the registry.
  • Allow using the IAS proxy in "mock" mode.
  • Enforce MRENCLAVE/MRSIGNER validity (IAS proxy).
    • (Debug) Allow disabling enforcement.
    • From a genesis document (Mostly for testing convenience).
    • Using the registry.
  • Use TLS for the link to the IAS proxy (ias: The IAS proxy needs TLS certs #2027).
  • Limit attestations to compute/keymanager nodes. (Moved to followup issue registry: Node registration process should be 2 stage #2039)
  • Enforce MRENCLAVE/MRSIGNER validity (worker host)? (Unrelated to IAS)

Breaking due to:

  • --ias.debug.skip_verify used in favor of EKIDEN_UNSAFE_SKIP_AVR_VERIFY at runtime.
  • --runtime.version.enclave required when provisioning runtimes, unless IAS proxy authentication is disabled. The ability to omit this flag may go away in the near future.
  • --genesis_file -> --genesis.file when provisioning the genesis file.
  • --ias.tls is required for workers that wish to use the IAS proxy.

@Yawning Yawning added p:1 Priority: core feature c:runtime/compute Category: runtime compute worker c:registry Category: entity/node/runtime registry service golang c:breaking/consensus Category: breaking consensus changes labels Aug 19, 2019
@Yawning Yawning self-assigned this Aug 19, 2019
@Yawning Yawning force-pushed the yawning/feature/runtime-tee-version branch 7 times, most recently from 07abec4 to 6fc2622 Compare August 22, 2019 12:52
@codecov
Copy link

codecov bot commented Aug 22, 2019

Codecov Report

Merging #2018 into master will decrease coverage by 1.99%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2018     +/-   ##
=========================================
- Coverage   55.97%   53.97%     -2%     
=========================================
  Files         247      242      -5     
  Lines       25089    23743   -1346     
=========================================
- Hits        14043    12815   -1228     
- Misses       9534     9545     +11     
+ Partials     1512     1383    -129
Impacted Files Coverage Δ
go/registry/api/runtime.go 69.69% <80%> (+0.73%) ⬆️
go/storage/mkvs/urkel/writelog/writelog.go 0% <0%> (-100%) ⬇️
go/storage/mkvs/urkel/syncer/stats.go 0% <0%> (-75%) ⬇️
go/storage/mkvs/urkel/db/badger/badger.go 0% <0%> (-63.4%) ⬇️
go/storage/mkvs/urkel/db/lru/lru.go 1.49% <0%> (-62.28%) ⬇️
go/storage/mkvs/urkel/syncer/syncer.go 25% <0%> (-25%) ⬇️
go/storage/mkvs/urkel/node/node.go 54.84% <0%> (-13.02%) ⬇️
go/storage/mkvs/urkel/lookup.go 81.81% <0%> (-9.1%) ⬇️
go/scheduler/tendermint/tendermint.go 61.22% <0%> (-7.15%) ⬇️
... and 80 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 5032575...6fc2622. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 22, 2019

Codecov Report

Merging #2018 into master will decrease coverage by 0.5%.
The diff coverage is 21.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2018      +/-   ##
==========================================
- Coverage   55.46%   54.96%   -0.51%     
==========================================
  Files         250      255       +5     
  Lines       25245    25537     +292     
==========================================
+ Hits        14002    14036      +34     
- Misses       9722     9977     +255     
- Partials     1521     1524       +3
Impacted Files Coverage Δ
go/common/sgx/common.go 15.18% <0%> (-1.03%) ⬇️
go/ekiden/cmd/ias/auth_registry.go 0% <0%> (ø)
go/ekiden/cmd/ias/auth_genesis.go 0% <0%> (ø)
go/common/sgx/ias/grpc.go 0% <0%> (ø) ⬆️
go/worker/keymanager/keymanager.go 21.59% <0%> (+0.08%) ⬆️
go/ekiden/cmd/common/grpc/grpc.go 74.41% <0%> (ø) ⬆️
go/keymanager/client/client.go 30.2% <0%> (ø) ⬆️
go/worker/common/enclaverpc/enclaverpc.go 0% <0%> (ø) ⬆️
go/ekiden/cmd/ias/auth.go 0% <0%> (ø)
go/common/node/node.go 27.06% <0%> (-2.29%) ⬇️
... and 28 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 6e779a2...0d2ba3c. Read the comment docs.

@Yawning Yawning force-pushed the yawning/feature/runtime-tee-version branch 15 times, most recently from 67f78ef to 951c88a Compare August 26, 2019 10:51
@Yawning Yawning force-pushed the yawning/feature/runtime-tee-version branch 6 times, most recently from 909c945 to 563e134 Compare August 28, 2019 10:27
Yawning added 16 commits August 28, 2019 13:59
If you are going to do a dump/restore, actually migrating the keymanager
database is probably a good idea.
 * Instead of a mountain of args, this now takes a `Config` struct
 * The attestation related worker host routines now live in a separate
   interface.
The IAS proxy also needs to deal with TLS certificates.
10 mins to run the go CI tests is excessive, and this test appears to
blame.  Running the profiler shows that the GC is going crazy, due to a
combination of snappy and go-codec (probably to be expected), and the CI
host has less resources than typical development hosts.
 * Move the mock AVR generator to `common/sgx/ias`
 * Add `ias.debug.skip_verify` since using the env var for configuration
   there is weird and inconsistent.

Breaking due to:
 * Env var being replaced with a config option.
This also will enable the IAS proxy, in mock mode for the e2e tests when
SGX is configured if `EKIDEN_UNSAFE_SKIP_VERIFY` is set.
This is now a struct so that MRSIGNER/MRENCLAVE pairs (and etc) can also
be included.
 * Ability to bypass auth (for easy testing/development)
 * Ability to just use a genesis document (for easy testing/development)
Currently mostly pointless, using the genesis file is easier and more
lightweight.  But this is the "right" way to do things, assuming
runtimes can be registered/updated without a redeploy.
@Yawning Yawning force-pushed the yawning/feature/runtime-tee-version branch from 563e134 to 0d2ba3c Compare August 28, 2019 14:00
Copy link
Contributor

@pro-wh pro-wh left a comment

Choose a reason for hiding this comment

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

alright thanks for the changes

@ravenac95 ravenac95 merged commit 40eecc2 into master Aug 28, 2019
@Yawning Yawning deleted the yawning/feature/runtime-tee-version branch August 28, 2019 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:breaking/consensus Category: breaking consensus changes c:registry Category: entity/node/runtime registry service c:runtime/compute Category: runtime compute worker golang p:1 Priority: core feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include valid MRENCLAVE/MRSIGNERs in the runtime descriptor
4 participants