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.

Sign up for GitHub

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

Jump to bottom

Refactor AttestationVerifier to employ an AttestationTrustStore interface #11913

Closed
tcarmelveilleux opened this issue Nov 17, 2021 · 0 comments · Fixed by #12277
Closed

Refactor AttestationVerifier to employ an AttestationTrustStore interface #11913

tcarmelveilleux opened this issue Nov 17, 2021 · 0 comments · Fixed by #12277
Labels
attestation

Comments

@tcarmelveilleux
Copy link
Contributor

Problem

Current example implementation of DeviceAttestationVerifier employs a very simplistic built-in PAA root store. This root store should be abstracted to let attestation verifiers get a list in a platform-specific way.

Proposed Solution

  • Create a AttestationTrustStore interface used by the DeviceAttestationVerifier interface. Require capabilities:
    • Look-up PAA by SKID
    • Determine if a PAA is revoked, by SKID
    • Determine if a PAI is revoked, by TBD
    • Determine if a DAC is revoked, by TBD
    • Interface should be asynchronous (since it's IO that may include look-ups)
  • Implement a "fixed trust store" version (i.e. the current example code with known single dev root)
  • Implement a POSIX version that can search through DER files in a particular directory for PAAs
tcarmelveilleux added a commit to tcarmelveilleux/connectedhomeip that referenced this issue Nov 25, 2021
@tcarmelveilleux
Make PAA store configurable
83591d7 
PAA store used by DefaultDeviceAttestationVerifier could not be
replaced, forcing a few fixed test roots to always be used and
nothing else, unless completely forking the
DefaultDeviceAttestationVerifier.

- This PR introduces the `PaaRootStore` interface, which the
  default `DeviceAttestationVerifier` expects to get configured
  at in constructor.
- Examples were modified to use the default test PAA root store
- Unit tests updated to use the testing root store
- Refactored simple array-based Root store to self-extract
  the SKID

Testing done: added new units tests which pass, ran cert tests,
validated attestation succeeds the same as before with test keys.

Fixed project-chip#11913
@tcarmelveilleux tcarmelveilleux mentioned this issue Nov 25, 2021
Merged
andy31415 pushed a commit that referenced this issue Nov 27, 2021
@tcarmelveilleux @restyled-commits
Make PAA trust store configurable (#12277)
ab5734c 
* Make PAA store configurable

PAA store used by DefaultDeviceAttestationVerifier could not be
replaced, forcing a few fixed test roots to always be used and
nothing else, unless completely forking the
DefaultDeviceAttestationVerifier.

- This PR introduces the `PaaRootStore` interface, which the
  default `DeviceAttestationVerifier` expects to get configured
  at in constructor.
- Examples were modified to use the default test PAA root store
- Unit tests updated to use the testing root store
- Refactored simple array-based Root store to self-extract
  the SKID

Testing done: added new units tests which pass, ran cert tests,
validated attestation succeeds the same as before with test keys.

Fixed #11913

* Restyled by clang-format

* Address review comments

- Rename PaaRootStore to AttestationTrustStore
- Add comments about ArrayAttestationtTrustStore lifecycle
- Remove debug print

* Fix python build

* Fix tv-app scoping issue

* Attempt to debug Darwin error

* Restyled by clang-format

* Remove debug logging used to diagnose CI

Co-authored-by: Restyled.io <commits@restyled.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
attestation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make PAA trust store configurable tcarmelveilleux/connectedhomeip
1 participant
@tcarmelveilleux