Skip to content

Commit

Permalink
Fix issues highlighted in PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
akarabashov committed Apr 9, 2024
1 parent b24b414 commit a1b9a20
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 27 deletions.
14 changes: 7 additions & 7 deletions docs/transactions.md
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,7 @@ If `crlSignerCertificate` is a PAA (root certificate), then it must be present o
If `crlSignerCertificate` is a PAI (intermediate certificate) or delegated by PAA, then it must be chained back to a valid PAA (root certificate) present on DCL.
In this case `crlSignerCertificate` is not required to be present on DCL, and will not be added to DCL as a result of this transaction.
If PAI needs to be added to DCL, it should be done via [ADD_X509_CERT](#add_x509_cert) transaction.
If the `crlSignerCertificate` is delegated by a PAI, the delegator certificate must be provided using the `certificate-delegator` field.
If the `crlSignerCertificate` is delegated by a PAI, the delegator certificate must be provided using the `crlSignerDelegator` field.
Additionally, the `crlSignerCertificate` must be chained back to the PAA through the delegator certificate, the PAA must be present on the DCL.

Publishing the revocation distribution endpoint doesn't automatically remove PAI (Intermediate certificates)
Expand All @@ -1074,8 +1074,8 @@ and DACs (leaf certificates) added to DCL if they are revoked in the CRL identif
- pid: `optional(uint16)` - Product ID (positive non-zero). Must be empty if `IsPAA` is true. Must be equal to a `pid` field in `CRLSignerCertificate`.
- isPAA: `bool` - True if the revocation information distribution point relates to a PAA
- label: `string` - A label to disambiguate multiple revocation information partitions of a particular issuer.
- certificate: `string` - The issuer certificate whose revocation information is provided in the distribution point entry, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data.
- certificate-delegator: `optional(string)` - The delegator certificate of CRL signer Certificate which must be chained back to approved certificate in the ledger, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data.
- crlSignerCertificate: `string` - The issuer certificate whose revocation information is provided in the distribution point entry, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data. Please note that if crlSignerCertificate is a delegated certificate by a PAI, the delegator certificate must be provided using the `crlSignerDelegator` field.
- crlSignerDelegator: `optional(string)` - If crlSignerCertificate is a delegated certificate by a PAI, then crlSignerDelegator must contain the delegator PAI certificate which must be chained back to an approved certificate in the ledger, encoded in X.509v3 PEM format. Otherwise this field can be omitted. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data.
- issuerSubjectKeyID: `string` - Uniquely identifies the PAA or PAI for which this revocation distribution point is provided. Must consist of even number of uppercase hexadecimal characters ([0-9A-F]), with no whitespace and no non-hexadecimal characters., e.g: `5A880E6C3653D07FB08971A3F473790930E62BDB`.
- dataUrl: `string` - The URL where to obtain the information in the format indicated by the RevocationType field. Must start with either `http` or `https`. Must be unique for all pairs of VendorID and IssuerSubjectKeyID.
- dataFileSize: `optional(uint64)` - Total size in bytes of the file found at the DataUrl. Must be omitted if RevocationType is 1.
Expand All @@ -1088,7 +1088,7 @@ and DACs (leaf certificates) added to DCL if they are revoked in the CRL identif
- `pki/RevocationDistributionPoint/value/<IssuerSubjectKeyID>/<vid>/<label>`-> Revocation Distribution Point
- CLI command:
- `dcld tx pki add-revocation-point --vid=<uint16> --pid=<uint16> --issuer-subject-key-id=<string> --is-paa=<bool> --label=<string>
--certificate=<string-or-path> --data-url=<string> --revocation-type=1 --from=<account>`
--certificate=<string-or-path> --certificate-delegator=<string-or-path> --data-url=<string> --revocation-type=1 --from=<account>`

#### ASSIGN_VID

Expand Down Expand Up @@ -1119,8 +1119,8 @@ Updates an existing PKI Revocation distribution endpoint (such as RFC5280 Certif
- vid: `uint16` - Vendor ID (positive non-zero). Must be the same as Vendor account's VID and `vid` field in the VID-scoped `CRLSignerCertificate`. Must be the same as a `vid` associated with non-VID scoped `CRLSignerCertificate` on the ledger.
- label: `string` - A label to disambiguate multiple revocation information partitions of a particular issuer.
- issuerSubjectKeyID: `string` - Uniquely identifies the PAA or PAI for which this revocation distribution point is provided. Must consist of even number of uppercase hexadecimal characters ([0-9A-F]), with no whitespace and no non-hexadecimal characters., e.g: `5A880E6C3653D07FB08971A3F473790930E62BDB`.
- certificate: `optional(string)` - The issuer certificate whose revocation information is provided in the distribution point entry, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data.
- certificate-delegator: `optional(string)` - The delegator certificate of CRL signer Certificate which must be chained back to approved certificate in the ledger, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data.
- crlSignerCertificate: `optional(string)` - The issuer certificate whose revocation information is provided in the distribution point entry, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data. Please note that if crlSignerCertificate is a delegated certificate by a PAI, the delegator certificate must be provided using the `crlSignerDelegator` field.
- crlSignerDelegator: `optional(string)` - If crlSignerCertificate is a delegated certificate by a PAI, then crlSignerDelegator must contain the delegator PAI certificate which must be chained back to an approved certificate in the ledger, encoded in X.509v3 PEM format. Otherwise this field can be omitted. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data.
- dataUrl: `optional(string)` - The URL where to obtain the information in the format indicated by the RevocationType field. Must start with either `http` or `https`. Must be unique for all pairs of VendorID and IssuerSubjectKeyID.
- dataFileSize: `optional(uint64)` - Total size in bytes of the file found at the DataUrl. Must be omitted if RevocationType is 1.
- dataDigest: `optional(string)` - Digest of the entire contents of the associated file downloaded from the DataUrl. Must be omitted if RevocationType is 1. Must be provided if and only if the `DataFileSize` field is present.
Expand All @@ -1131,7 +1131,7 @@ Updates an existing PKI Revocation distribution endpoint (such as RFC5280 Certif
- `pki/RevocationDistributionPoint/value/<IssuerSubjectKeyID>/<vid>/<label>` -> Revocation Distribution Point
- CLI command:
- `dcld tx pki update-revocation-point --vid=<uint16> --issuer-subject-key-id=<string> --label=<string>
--data-url=<string> --certificate=<string-or-path> --from=<account>`
--data-url=<string> --certificate=<string-or-path> --certificate-delegator=<string-or-path> --from=<account>`

### DELETE_DELETE_PKI_REVOCATION_DISTRIBUTION_POINT

Expand Down
14 changes: 11 additions & 3 deletions types/pki/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,10 @@ func NewErrRootCertVidNotEqualToAccountVid(rootVID int32, accountVID int32) erro
rootVID, accountVID)
}

func NewErrCRLSignerCertificateInvalidFormat() error {
func NewErrCRLSignerCertificateInvalidFormat(description string) error {
return sdkerrors.Wrapf(
ErrCRLSignerCertificateInvalidFormat,
"Invalid CRL Signer Certificate format",
ErrCRLSignerCertificateInvalidFormat, "Invalid CRL Signer Certificate format: %v",
description,
)
}

Expand Down Expand Up @@ -298,6 +298,14 @@ func NewErrDataFieldPresented(revocationType uint32) error {
}

func NewErrWrongSubjectKeyIDFormat() error {
return sdkerrors.Wrapf(
ErrWrongSubjectKeyIDFormat,
"Wrong SubjectKeyID format. It must consist of even number of uppercase hexadecimal characters ([0-9A-F]), "+
"with no whitespace and no non-hexadecimal characters",
)
}

func NewErrWrongIssuerSubjectKeyIDFormat() error {
return sdkerrors.Wrapf(
ErrWrongSubjectKeyIDFormat,
"Wrong IssuerSubjectKeyID format. It must consist of even number of uppercase hexadecimal characters ([0-9A-F]), "+
Expand Down
4 changes: 2 additions & 2 deletions x/pki/client/cli/tx_add_pki_revocation_distribution_point.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ func CmdAddPkiRevocationDistributionPoint() *cobra.Command {
"Product ID (positive non-zero). Must be empty if IsPAA is true. Must be equal to a pid field in CRLSignerCertificate")
cmd.Flags().BoolVar(&isPAA, FlagIsPAA, true, "True if the revocation information distribution point relates to a PAA (Root certificate)")
cmd.Flags().StringVarP(&label, FlagLabel, FlagLabelShortcut, "", " A label to disambiguate multiple revocation information partitions of a particular issuer")
cmd.Flags().StringVarP(&crlSignerCertificate, FlagCertificate, FlagCertificateShortcut, "", "The issuer certificate whose revocation information is provided in the distribution point entry, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data")
cmd.Flags().StringVar(&crlSignerDelegator, FlagCertificateDelegator, "", "The delegator certificate of CRL signer Certificate which must be chained back to approved certificate in the ledger, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data")
cmd.Flags().StringVarP(&crlSignerCertificate, FlagCertificate, FlagCertificateShortcut, "", "The issuer certificate whose revocation information is provided in the distribution point entry, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data. Please note that if crlSignerCertificate is a delegated certificate by a PAI, the delegator certificate must be provided using the `crlSignerDelegator` field")
cmd.Flags().StringVar(&crlSignerDelegator, FlagCertificateDelegator, "", "If crlSignerCertificate is a delegated certificate by a PAI, then crlSignerDelegator must contain the delegator PAI certificate which must be chained back to an approved certificate in the ledger, encoded in X.509v3 PEM format. Otherwise this field can be omitted. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data")
cmd.Flags().StringVar(&issuerSubjectKeyID, FlagIssuerSubjectKeyID, "", "Uniquely identifies the PAA or PAI for which this revocation distribution point is provided. Must consist of even number of uppercase hexadecimal characters ([0-9A-F]), with no whitespace and no non-hexadecimal characters., e.g: 5A880E6C3653D07FB08971A3F473790930E62BDB")
cmd.Flags().StringVar(&dataURL, FlagDataURL, "", "The URL where to obtain the information in the format indicated by the RevocationType field. Must start with either http or https")
cmd.Flags().Uint64Var(&dataFileSize, FlagDataFileSize, 0, "Total size in bytes of the file found at the DataURL. Must be omitted if RevocationType is 1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ func CmdUpdatePkiRevocationDistributionPoint() *cobra.Command {
cmd.Flags().Int32Var(&vid, FlagVid, 0,
"Vendor ID (positive non-zero). Must be the same as Vendor account's VID and vid field in the VID-scoped CRLSignerCertificate")
cmd.Flags().StringVarP(&label, FlagLabel, FlagLabelShortcut, "", " A label to disambiguate multiple revocation information partitions of a particular issuer")
cmd.Flags().StringVarP(&crlSignerCertificate, FlagCertificate, FlagCertificateShortcut, "", "The issuer certificate whose revocation information is provided in the distribution point entry, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data")
cmd.Flags().StringVar(&crlSignerDelegator, FlagCertificateDelegator, "", "The delegator certificate of CRL signer Certificate which must be chained back to approved certificate in the ledger, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data")
cmd.Flags().StringVarP(&crlSignerCertificate, FlagCertificate, FlagCertificateShortcut, "", "The issuer certificate whose revocation information is provided in the distribution point entry, encoded in X.509v3 PEM format. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data. Please note that if crlSignerCertificate is a delegated certificate by a PAI, the delegator certificate must be provided using the `crlSignerDelegator` field")
cmd.Flags().StringVar(&crlSignerDelegator, FlagCertificateDelegator, "", "If crlSignerCertificate is a delegated certificate by a PAI, then crlSignerDelegator must contain the delegator PAI certificate which must be chained back to an approved certificate in the ledger, encoded in X.509v3 PEM format. Otherwise this field can be omitted. The corresponding CLI parameter can contain either a PEM string or a path to a file containing the data")
cmd.Flags().StringVar(&issuerSubjectKeyID, FlagIssuerSubjectKeyID, "", "Uniquely identifies the PAA or PAI for which this revocation distribution point is provided. Must consist of even number of uppercase hexadecimal characters ([0-9A-F]), with no whitespace and no non-hexadecimal characters., e.g: 5A880E6C3653D07FB08971A3F473790930E62BDB")
cmd.Flags().StringVar(&dataURL, FlagDataURL, "", "The URL where to obtain the information in the format indicated by the RevocationType field. Must start with either http or https")
cmd.Flags().Uint64Var(&dataFileSize, FlagDataFileSize, 0, "Total size in bytes of the file found at the DataURL. Must be omitted if RevocationType is 1")
Expand Down
45 changes: 34 additions & 11 deletions x/pki/types/message_add_pki_revocation_distribution_point.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package types

import (
"crypto/ecdsa"
"crypto/elliptic"
x509std "crypto/x509"
"encoding/asn1"
"strings"
Expand All @@ -16,6 +18,8 @@ const TypeMsgAddPkiRevocationDistributionPoint = "add_pki_revocation_distributio

var _ sdk.Msg = &MsgAddPkiRevocationDistributionPoint{}

var oidKeyUsage = asn1.ObjectIdentifier{2, 5, 29, 15}

func NewMsgAddPkiRevocationDistributionPoint(signer string, vid int32, pid int32, isPAA bool, label string,
crlSignerCertificate string, crlSignerDelegator string, issuerSubjectKeyID string, dataURL string, dataFileSize uint64, dataDigest string,
dataDigestType uint32, revocationType uint32, schemaVersion uint32) *MsgAddPkiRevocationDistributionPoint {
Expand Down Expand Up @@ -67,7 +71,7 @@ func (msg *MsgAddPkiRevocationDistributionPoint) verifyPAA(cert *x509.Certificat

pid, _ := x509.GetPidFromSubject(cert.SubjectAsText)
if pid != 0 {
return pkitypes.NewErrNotEmptyPidForNonRootCertificate()
return pkitypes.NewErrNotEmptyPidForRootCertificate()
}

// verify VID
Expand Down Expand Up @@ -137,23 +141,40 @@ func VerifyCRLSignerCertFormat(certificate *x509.Certificate) error {

cert := certificate.Certificate
if cert.Version != 3 {
return pkitypes.NewErrCRLSignerCertificateInvalidFormat()
return pkitypes.NewErrCRLSignerCertificateInvalidFormat(
"The version field SHALL be set to 2 to indicate v3 certificate",
)
}

if cert.SignatureAlgorithm != x509std.ECDSAWithSHA256 {
return pkitypes.NewErrCRLSignerCertificateInvalidFormat()
return pkitypes.NewErrCRLSignerCertificateInvalidFormat(
"The signature field SHALL contain the identifier for signatureAlgorithm ecdsa-with-SHA256",
)
}

// Type assert to get the ECDSA public key
ecdsaPubKey, ok := cert.PublicKey.(*ecdsa.PublicKey)
if !ok {
return pkitypes.NewErrCRLSignerCertificateInvalidFormat(
"Public key is not of type ECDSA",
)
}

if cert.PublicKeyAlgorithm != x509std.ECDSA {
return pkitypes.NewErrCRLSignerCertificateInvalidFormat()
// Check if the curve parameters match prime256v1 (secp256r1 / P-256)
if ecdsaPubKey.Curve != elliptic.P256() {
return pkitypes.NewErrCRLSignerCertificateInvalidFormat(
"The public key must use prime256v1 curve",
)
}

// Basic Constraint extension should be marked critical and have the cA field set to false
if !cert.BasicConstraintsValid || cert.IsCA {
return pkitypes.NewErrCRLSignerCertificateInvalidFormat()
return pkitypes.NewErrCRLSignerCertificateInvalidFormat(
"Basic Constraint extension's cA field SHALL be set to FALSE",
)
}

// Key Usage extension should be marked critical
oidKeyUsage := asn1.ObjectIdentifier{2, 5, 29, 15}
isCritical := false
for _, ext := range cert.Extensions {
if ext.Id.Equal(oidKeyUsage) {
Expand All @@ -162,15 +183,17 @@ func VerifyCRLSignerCertFormat(certificate *x509.Certificate) error {
break
}
}

if !isCritical {
return pkitypes.NewErrCRLSignerCertificateInvalidFormat()
return pkitypes.NewErrCRLSignerCertificateInvalidFormat("Basic Constraint extension SHALL be marked critical")
}

if cert.KeyUsage&x509std.KeyUsageCRLSign == 0 {
return pkitypes.NewErrCRLSignerCertificateInvalidFormat()
return pkitypes.NewErrCRLSignerCertificateInvalidFormat("The cRLSign bits SHALL be set in the KeyUsage bitstring")
}

if cert.KeyUsage&^(x509std.KeyUsageCRLSign|x509std.KeyUsageDigitalSignature) != 0 {
return pkitypes.NewErrCRLSignerCertificateInvalidFormat()
return pkitypes.NewErrCRLSignerCertificateInvalidFormat("The KeyUsage bitstring can only include the cRLSign and digitalSignature bits")
}

return nil
Expand Down Expand Up @@ -248,7 +271,7 @@ func (msg *MsgAddPkiRevocationDistributionPoint) verifyFields() error {
match := VerifyRevocationPointIssuerSubjectKeyIDFormat(msg.IssuerSubjectKeyID)

if !match {
return pkitypes.NewErrWrongSubjectKeyIDFormat()
return pkitypes.NewErrWrongIssuerSubjectKeyIDFormat()
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (msg *MsgDeletePkiRevocationDistributionPoint) ValidateBasic() error {
match := VerifyRevocationPointIssuerSubjectKeyIDFormat(msg.IssuerSubjectKeyID)

if !match {
return pkitypes.NewErrWrongSubjectKeyIDFormat()
return pkitypes.NewErrWrongIssuerSubjectKeyIDFormat()
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (msg *MsgUpdatePkiRevocationDistributionPoint) ValidateBasic() error {
match := VerifyRevocationPointIssuerSubjectKeyIDFormat(msg.IssuerSubjectKeyID)

if !match {
return pkitypes.NewErrWrongSubjectKeyIDFormat()
return pkitypes.NewErrWrongIssuerSubjectKeyIDFormat()
}

return nil
Expand Down

0 comments on commit a1b9a20

Please sign in to comment.