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

Broken design pattern in the certificate.proto #611

Closed
robszewczyk opened this issue Oct 15, 2024 · 1 comment
Closed

Broken design pattern in the certificate.proto #611

robszewczyk opened this issue Oct 15, 2024 · 1 comment
Milestone

Comments

@robszewczyk
Copy link

robszewczyk commented Oct 15, 2024

Problem

Version 1.4 of the DCL introduced extensions to certificate.proto :

  bool isNoc = 15;
  uint32 schemaVersion = 16;

The design pattern is short-sighted and error prone:

  • extending this pattern to other types of PKI would suggest additional key types are specified through boolean flags -- e.g. isVidSigner
  • specifying additional flags gets very problematic in cases where exactly one of the flags must be true.
  • consumers of the API are lulled into a false sense of security w.r.t. default cases -- even in the current design, the client deduces that the certificate comes from the DA PKI by checking isNoc == false. This scheme is destined to break backward compatibility if we introduce any additional certificate chain.

Proposed solution

  • Introduce enumeration CertificateType. On day 0, that enumeration would have three values:
enum CertificateType {
    DeviceAttestationPKI = 0;
    OperationalPKI = 1;
    VIDSignerPKI = 2;
}
  • Rename isNoc to certificateType:
    CertificateType certificateType = 15;

This remains wire compatible with the previous isNoc because both bool and enums are encoded as varints.

@hawk248
Copy link
Collaborator

hawk248 commented Oct 31, 2024

DCL TT : change isNoc to enum as specified here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

4 participants