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

Update transactions.md document #553

Merged
merged 9 commits into from
Apr 3, 2024
Merged

Conversation

akarabashov
Copy link
Collaborator

No description provided.

- time: `optional(int64)` - proposal time (number of nanoseconds elapsed since January 1, 1970 UTC). CLI uses the current time for that field.
- vid: `uint16` - Vendor ID (positive non-zero). Must be the same as Vendor account's VID. The value must be within the range of 1 to 65535.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please mention that it must be equal to the Certificate's VID for VID scope certificates.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -903,8 +906,6 @@ The certificate is not reject until sufficient number of Trustees reject it.
Adds an intermediate or leaf X509 certificate signed by a chain of certificates which must be
already present on the ledger.

The certificate is immutable. It can only be revoked by either the owner or a quorum of Trustees.

- Who can send: Vendor account
- PAA (Root certificates) are VID-scoped:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move this paragraph to Validation?
Also I think it needs to be clarified.
For example, it's not clear what does "PAA (Root certificates) are VID-scoped" mean.
I would writ something like

  • If parent root certificate is VID scoped:
    • cert must be VID scoped
    • vid in the subject of the root certificate must be equal to the the vid in the subject of the cert
    • vid in the certificate subjects must be equal to the sender Vendor account's VID
  • If parent root certificate is not VID scoped, but has an associated VID
    • cert can be either VID scoped or non-VID scoped
    • if cert is VID scoped, vid in the subject of the cert must be equal to the VID associated with the root certificate, as well as to the sender Vendor account's VID
  • If parent root certificate is non VID scoped and doesn't have an associated VID - error

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -925,9 +926,10 @@ The certificate is immutable. It can only be revoked by either the owner or a qu
- no existing certificate with the same `<Certificate's Issuer>:<Certificate's Serial Number>` combination.
- if certificates with the same `<Certificate's Subject>:<Certificate's Subject Key ID>` combination already exist:
- the existing certificate must not be NOC certificate
- the sender's VID must match the `vid` field of the existing certificates.
- the sender's VID must match the VID of the existing certificate's owner.
- the signature (self-signature) and expiration date are valid.
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't have self-signature for Intermediate/Leaf certs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed

@@ -903,8 +906,6 @@ The certificate is not reject until sufficient number of Trustees reject it.
Adds an intermediate or leaf X509 certificate signed by a chain of certificates which must be
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be great to add some description to distinguish these certificates from NOC/ICA certificates.

@@ -827,13 +827,14 @@ will be in a pending state until sufficient number of approvals is received.

The certificate is immutable. It can only be revoked by either the owner or a quorum of Trustees.

Copy link
Contributor

@ashcherbakov ashcherbakov Mar 20, 2024

Choose a reason for hiding this comment

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

What if we rename the commands as follows:

X509 PKI

Device Attestation Certificates (DA)

PROPOSE_ADD_PAA

APPROVE_ADD_PAA

REJECT_ADD_PAA

PROPOSE_REVOKE_PAA

APPROVE_REVOKE_PAA

ASSIGN_VID_TO_PAA

ADD_REVOCATION_DISTRIBUTION_POINT

UPDATE_REVOCATION_DISTRIBUTION_POINT

DELETE_REVOCATION_DISTRIBUTION_POINT

ADD_PAI

REVOKE_PAI

REMOVE_PAI

GET_REVOCATION_DISTRIBUTION_POINT

GET_REVOCATION_DISTRIBUTION_POINT_BY_SKID

GET_ALL_REVOCATION_DISTRIBUTION_POINT

GET_PROPOSED_PAA

GET_REJECTED_PAA

GET_PROPOSED_REVOKE_PAA

GET_ALL_PAA

GET_ALL_REVOKED_PAA

GET_ALL_PROPOSED_PAA

GET_ALL_REJECTED_PAA

GET_ALL_PROPOSED_REVOKE_PAA

E2E (NOC)

ADD_NOC_ROOT

REVOKE_NOC_ROOT

ADD_NOC_ICA

GET_NOC_ROOT_BY_VID

GET_NOC_ICA_BY_VID

GET_REVOKED_NOC_ROOT

GET_ALL_NOC_ROOT

GET_ALL_NOC_ICA

GET_ALL_REVOKED_NOC_ROOT

Common

GET_CERT

GET_REVOKED_CERT

GET_CERTS_BY_SKID

GET_CERTS_BY_SUBJECT

GET_CHILD_CERTS

GET_ALL_CERTS

GET_ALL_REVOKED_CERTS

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed

- `pki/NOCCertificates/value/<VID>`
- `pki/ChildCertificates/value/<Certificate's Subject>/<Certificate's Subject Key ID>`
- CLI Command:
- `dcld tx pki add-noc-x509-cert --certificate=<string-or-path> --from=<account>`

Copy link
Contributor

@ashcherbakov ashcherbakov Mar 20, 2024

Choose a reason for hiding this comment

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

Please rename NOC to NOC_ROOT and non-root NOC to NOC_ICA (in doc, code, state, CLI command).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will be implemented in a separate pull request.

akarabashov

This comment was marked as off-topic.


**Status: Implemented**

Proposes a new self-signed root certificate.
Proposes a new PAA certificate.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clarify that a PAA is a root self-signed certificate.

### REVOKE_NOC_X509_ROOT_CERT
> **_Note:_** Multiple certificates can refer to the same `<Certificate's Subject>:<Certificate's Subject Key ID>` combination.

#### REVOKE_PAI
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a clarification how it's different from REMOVE_PAI

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done


### GET_ALL_SUBJECT_X509_CERTS
#### REMOVE_PAI
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a clarification how it's different from REVOKE_PAI


### GET_PKI_REVOCATION_DISTRIBUTION_POINTS_BY_SUBJECT_KEY_ID
#### REVOKE_NOC_ROOT
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clarify that revoke here means soft-delete (revoke certificates can be queried)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

- CLI Command:
- `dcld tx pki add-noc-x509-cert --certificate=<string-or-path> --from=<account>`

#### REVOKE_NOC_ICA
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clarify that revoke here means soft-delete (revoke certificates can be queried)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

- REST API:
- GET `/dcl/pki/revoked-certificates/{subject}/{subject_key_id}`

#### GET_CERTS_BY_SKID
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clarify that it works for all types of certificates (PAA, PAI, NOC_ROOT, NOC_ICA).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

- REST API:
- GET `/dcl/pki/certificates?subjectKeyId={subjectKeyId}`

#### GET_CERTS_BY_SUBJECT
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clarify that it works for all types of certificates (PAA, PAI, NOC_ROOT, NOC_ICA).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

- REST API:
- GET `/dcl/pki/certificates/{subject}`

#### GET_CHILD_CERTS
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clarify that it works for all types of certificates (PAA, PAI, NOC_ROOT, NOC_ICA).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

- REST API:
- GET `/dcl/pki/child-certificates/{subject}/{subject_key_id}`

#### GET_ALL_CERTS
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clarify that it returns all certificates (PAA, PAI, NOC_ROOT, NOC_ICA).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

- REST API:
- GET `/dcl/pki/certificates`

#### GET_ALL_REVOKED_CERTS
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clarify that it returns all revoked certificates (PAA, PAI, NOC_ROOT, NOC_ICA).

@akarabashov akarabashov merged commit 17e3203 into master Apr 3, 2024
8 checks passed
@akarabashov akarabashov deleted the improve_transaction_document branch April 11, 2024 06:31
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.

2 participants