From bcb8b3420b47a14189f321cc484d1dde16f350e5 Mon Sep 17 00:00:00 2001 From: Harsh Kashyap Date: Fri, 12 Jun 2026 15:12:47 +0530 Subject: [PATCH 01/10] security: add certificate revocation record --- std/security/certificate.go | 41 ++++++++++++++++ std/security/certificate_test.go | 83 ++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/std/security/certificate.go b/std/security/certificate.go index 99060019..27666095 100644 --- a/std/security/certificate.go +++ b/std/security/certificate.go @@ -3,6 +3,7 @@ package security import ( "fmt" "io" + "sync" "time" enc "github.com/named-data/ndnd/std/encoding" @@ -12,6 +13,13 @@ import ( "github.com/named-data/ndnd/std/types/optional" ) +var revokedCertRecord = struct { + sync.RWMutex + names map[string]struct{} +}{ + names: map[string]struct{}{}, +} + // SignCertArgs are the arguments to SignCert. type SignCertArgs struct { // Signer is the private key used to sign the certificate. @@ -115,6 +123,39 @@ func CertIsExpired(cert ndn.Data) bool { return false } +// Revoke records the certificate name as revoked for this process. +func Revoke(cert ndn.Data) { + key, ok := revocationRecordKey(cert) + if !ok { + return + } + + revokedCertRecord.Lock() + defer revokedCertRecord.Unlock() + revokedCertRecord.names[key] = struct{}{} +} + +// IsRevoked reports whether the certificate name has been revoked in this process. +func IsRevoked(cert ndn.Data) bool { + key, ok := revocationRecordKey(cert) + if !ok { + return false + } + + revokedCertRecord.RLock() + defer revokedCertRecord.RUnlock() + _, ok = revokedCertRecord.names[key] + return ok +} + +func revocationRecordKey(cert ndn.Data) (string, bool) { + if cert == nil { + return "", false + } + + return cert.Name().TlvStr(), true +} + // getPubKey gets the public key from an NDN data. // returns [public key, key name, error]. func getPubKey(data ndn.Data) ([]byte, enc.Name, error) { diff --git a/std/security/certificate_test.go b/std/security/certificate_test.go index f3419c3b..6ea7f44f 100644 --- a/std/security/certificate_test.go +++ b/std/security/certificate_test.go @@ -2,7 +2,10 @@ package security_test import ( "encoding/base64" + "fmt" + "sync" "testing" + "time" enc "github.com/named-data/ndnd/std/encoding" "github.com/named-data/ndnd/std/ndn" @@ -174,6 +177,86 @@ func TestSignCertWithSignerCertName(t *testing.T) { require.True(t, tu.NoErr(signer.ValidateData(newCert, newSigCov, aliceCertData))) } +func TestCertificateRevocationRecord(t *testing.T) { + tu.SetT(t) + + aliceKey, _ := base64.StdEncoding.DecodeString(KEY_ALICE) + aliceKeyData, _, _ := spec_2022.Spec{}.ReadData(enc.NewBufferView(aliceKey)) + aliceSigner := tu.NoErr(signer.UnmarshalSecret(aliceKeyData)) + + aliceCertWire := tu.NoErr(sec.SignCert(sec.SignCertArgs{ + Signer: aliceSigner, + Data: aliceKeyData, + IssuerId: revocationTestIssuer(t), + NotBefore: T1, + NotAfter: T2, + })) + aliceCert, _, err := spec_2022.Spec{}.ReadData(enc.NewWireView(aliceCertWire)) + require.NoError(t, err) + + otherCertWire := tu.NoErr(sec.SignCert(sec.SignCertArgs{ + Signer: aliceSigner, + Data: aliceKeyData, + IssuerId: revocationTestIssuer(t), + NotBefore: T1, + NotAfter: T2, + })) + otherCert, _, err := spec_2022.Spec{}.ReadData(enc.NewWireView(otherCertWire)) + require.NoError(t, err) + + require.False(t, sec.IsRevoked(nil)) + require.NotPanics(t, func() { + sec.Revoke(nil) + }) + + require.False(t, sec.IsRevoked(aliceCert)) + require.False(t, sec.IsRevoked(otherCert)) + + sec.Revoke(aliceCert) + + require.True(t, sec.IsRevoked(aliceCert)) + require.False(t, sec.IsRevoked(otherCert)) +} + +func TestCertificateRevocationRecordConcurrentAccess(t *testing.T) { + tu.SetT(t) + + aliceKey, _ := base64.StdEncoding.DecodeString(KEY_ALICE) + aliceKeyData, _, _ := spec_2022.Spec{}.ReadData(enc.NewBufferView(aliceKey)) + aliceSigner := tu.NoErr(signer.UnmarshalSecret(aliceKeyData)) + + aliceCertWire := tu.NoErr(sec.SignCert(sec.SignCertArgs{ + Signer: aliceSigner, + Data: aliceKeyData, + IssuerId: revocationTestIssuer(t), + NotBefore: T1, + NotAfter: T2, + })) + aliceCert, _, err := spec_2022.Spec{}.ReadData(enc.NewWireView(aliceCertWire)) + require.NoError(t, err) + + var wg sync.WaitGroup + for range 50 { + wg.Add(2) + go func() { + defer wg.Done() + sec.Revoke(aliceCert) + }() + go func() { + defer wg.Done() + _ = sec.IsRevoked(aliceCert) + }() + } + wg.Wait() + + require.True(t, sec.IsRevoked(aliceCert)) +} + +func revocationTestIssuer(t *testing.T) enc.Component { + t.Helper() + return enc.NewGenericComponent(fmt.Sprintf("%s-%d", t.Name(), time.Now().UnixNano())) +} + func TestEncodeDecodeCertList(t *testing.T) { tu.SetT(t) n1 := tu.NoErr(enc.NameFromStr("/ndn/alice/KEY/aa/self/v=1")) From 347464371a16448bd05c835a99b8b69e7e315191 Mon Sep 17 00:00:00 2001 From: Harsh Kashyap Date: Thu, 18 Jun 2026 08:11:27 +0530 Subject: [PATCH 02/10] security: align revocation record with reference Co-authored-by: Cursor --- std/security/certificate.go | 74 +++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 7 deletions(-) diff --git a/std/security/certificate.go b/std/security/certificate.go index 27666095..fbb3c510 100644 --- a/std/security/certificate.go +++ b/std/security/certificate.go @@ -1,6 +1,7 @@ package security import ( + "crypto/sha256" "fmt" "io" "sync" @@ -13,11 +14,23 @@ import ( "github.com/named-data/ndnd/std/types/optional" ) +type revocationReason uint64 + +const revocationReasonUnspecified revocationReason = 0 + +type revocationRecord struct { + Name enc.Name + CertName enc.Name + PublicKeyHash [sha256.Size]byte + Reason revocationReason + Timestamp time.Time +} + var revokedCertRecord = struct { sync.RWMutex - names map[string]struct{} + records map[string]revocationRecord }{ - names: map[string]struct{}{}, + records: map[string]revocationRecord{}, } // SignCertArgs are the arguments to SignCert. @@ -125,14 +138,14 @@ func CertIsExpired(cert ndn.Data) bool { // Revoke records the certificate name as revoked for this process. func Revoke(cert ndn.Data) { - key, ok := revocationRecordKey(cert) + record, ok := makeRevocationRecord(cert, revocationReasonUnspecified) if !ok { return } revokedCertRecord.Lock() defer revokedCertRecord.Unlock() - revokedCertRecord.names[key] = struct{}{} + revokedCertRecord.records[record.Name.TlvStr()] = record } // IsRevoked reports whether the certificate name has been revoked in this process. @@ -144,16 +157,63 @@ func IsRevoked(cert ndn.Data) bool { revokedCertRecord.RLock() defer revokedCertRecord.RUnlock() - _, ok = revokedCertRecord.names[key] + _, ok = revokedCertRecord.records[key] return ok } +func makeRevocationRecord(cert ndn.Data, reason revocationReason) (revocationRecord, bool) { + recordName, ok := revocationRecordName(cert) + if !ok { + return revocationRecord{}, false + } + + return revocationRecord{ + Name: recordName, + CertName: stripImplicitDigest(cert.Name()), + PublicKeyHash: sha256.Sum256(cert.Content().Join()), + Reason: reason, + Timestamp: time.Now(), + }, true +} + func revocationRecordKey(cert ndn.Data) (string, bool) { - if cert == nil { + recordName, ok := revocationRecordName(cert) + if !ok { return "", false } - return cert.Name().TlvStr(), true + return recordName.TlvStr(), true +} + +func revocationRecordName(cert ndn.Data) (enc.Name, bool) { + if cert == nil { + return nil, false + } + + certName := stripImplicitDigest(cert.Name()) + identity, err := GetIdentityFromCertName(certName) + if err != nil || !certName.At(-1).IsVersion() { + return nil, false + } + + recordName := make(enc.Name, len(certName), len(certName)+1) + copy(recordName, certName) + recordName[len(identity)] = enc.NewGenericComponent("REVOKE") + return recordName.Append(certName.At(-2)), true +} + +func isRevocationRecordName(name enc.Name) bool { + name = stripImplicitDigest(name) + if len(name) < 6 || name.At(-1).Typ != enc.TypeGenericNameComponent || !name.At(-2).IsVersion() { + return false + } + + certName := make(enc.Name, len(name)-1) + copy(certName, name.Prefix(-1)) + certName[len(name)-5] = enc.NewGenericComponent("KEY") + + _, err := GetIdentityFromCertName(certName) + return err == nil } // getPubKey gets the public key from an NDN data. From 0e2c8b24515137c3ca7c06f92ef46769cf8121c7 Mon Sep 17 00:00:00 2001 From: Harsh Kashyap Date: Tue, 30 Jun 2026 23:30:55 +0530 Subject: [PATCH 03/10] security: integrate revocation checks into TrustConfig --- std/security/certificate.go | 5 ++ std/security/trust_config.go | 78 ++++++++++++++++++++++++++- std/security/trust_config_test.go | 89 +++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+), 1 deletion(-) diff --git a/std/security/certificate.go b/std/security/certificate.go index fbb3c510..3a6ee091 100644 --- a/std/security/certificate.go +++ b/std/security/certificate.go @@ -148,6 +148,11 @@ func Revoke(cert ndn.Data) { revokedCertRecord.records[record.Name.TlvStr()] = record } +// RevocationRecordName derives the REVOKE-style record name for a certificate. +func RevocationRecordName(cert ndn.Data) (enc.Name, bool) { + return revocationRecordName(cert) +} + // IsRevoked reports whether the certificate name has been revoked in this process. func IsRevoked(cert ndn.Data) bool { key, ok := revocationRecordKey(cert) diff --git a/std/security/trust_config.go b/std/security/trust_config.go index b8e4ba9a..c38e493e 100644 --- a/std/security/trust_config.go +++ b/std/security/trust_config.go @@ -94,6 +94,26 @@ func (tc *TrustConfig) Suggest(name enc.Name) ndn.Signer { return tc.schema.Suggest(name, tc.keychain) } +// InstallRevocationRecord stores a received revocation record in the keychain store. +// The wire must encode a Data packet whose name follows the REVOKE naming convention. +func (tc *TrustConfig) InstallRevocationRecord(wire enc.Wire) error { + if len(wire) == 0 { + return fmt.Errorf("revocation record wire is empty") + } + + data, _, err := spec.Spec{}.ReadData(enc.NewWireView(wire)) + if err != nil { + return fmt.Errorf("failed to parse revocation record: %w", err) + } + if !isRevocationRecordName(data.Name()) { + return fmt.Errorf("not a revocation record name: %s", data.Name()) + } + + tc.mutex.Lock() + defer tc.mutex.Unlock() + return tc.keychain.Store().Put(data.Name(), wire.Join()) +} + // SetSchema atomically replaces the trust schema. func (tc *TrustConfig) SetSchema(schema ndn.TrustSchema) { if schema == nil { @@ -182,6 +202,9 @@ func (tc *TrustConfig) Validate(args TrustConfigValidateArgs) { args.Callback(false, fmt.Errorf("certificate is expired: %s", args.Data.Name())) return } + if tc.rejectIfRevokedCert(args.Data, args.Callback) { + return + } } // Get the key locator @@ -193,6 +216,10 @@ func (tc *TrustConfig) Validate(args TrustConfigValidateArgs) { // If a certificate is provided, go directly to validation if args.cert != nil { + if tc.rejectIfRevokedCert(args.cert, args.Callback) { + return + } + certName := args.cert.Name() dataName := args.Data.Name() if len(args.OverrideName) > 0 { @@ -323,6 +350,10 @@ func (tc *TrustConfig) Validate(args TrustConfigValidateArgs) { // Check the validated memcache for the certificate if cachedCert, ok := tc.certCache.Get(keyLocator); ok { + if tc.rejectIfRevokedCert(cachedCert, args.Callback) { + return + } + // The cache always checks the expiry of the cert args.cert = cachedCert args.certIsValid = true @@ -338,7 +369,10 @@ func (tc *TrustConfig) Validate(args TrustConfigValidateArgs) { fwHint = []enc.Name{args.origDataName} } - // Cert not found, attempt to fetch from network + // Cert not found, attempt to fetch from network. + // TODO(revocation): the engine can express an Interest for the derived REVOKE + // record name when no local record is found. For now, only records installed + // via InstallRevocationRecord in keychain.Store() are checked. fetchCfg := &ndn.InterestConfig{ CanBePrefix: true, MustBeFresh: true, @@ -379,6 +413,10 @@ func (tc *TrustConfig) Validate(args TrustConfigValidateArgs) { // Fetched cert is fresh log.Debug(tc, "Fetched certificate from network", "cert", res.Data.Name()) + if tc.rejectIfRevokedCert(res.Data, args.Callback) { + return + } + // Call again with the fetched cert args.cert = res.Data args.certSigCov = res.SigCovered @@ -507,6 +545,44 @@ func (tc *TrustConfig) PromoteAnchor(cert ndn.Data, raw enc.Wire) { tc.roots = append(tc.roots, name) } +func (tc *TrustConfig) isTrustedAnchorCert(cert ndn.Data) bool { + if cert == nil { + return false + } + + name := stripImplicitDigest(cert.Name()) + tc.mutex.RLock() + defer tc.mutex.RUnlock() + for _, root := range tc.roots { + if root.Equal(name) { + return true + } + } + return false +} + +func (tc *TrustConfig) certIsRevoked(cert ndn.Data) bool { + if cert == nil || tc.isTrustedAnchorCert(cert) { + return false + } + + recordName, ok := revocationRecordName(cert) + if !ok { + return false + } + + wire, _ := tc.keychain.Store().Get(recordName, false) + return len(wire) > 0 +} + +func (tc *TrustConfig) rejectIfRevokedCert(cert ndn.Data, callback func(bool, error)) bool { + if tc.certIsRevoked(cert) { + callback(false, fmt.Errorf("certificate is revoked: %s", cert.Name())) + return true + } + return false +} + func (tc *TrustConfig) isTrustedAnchorKey(keyLocator enc.Name) bool { tc.mutex.RLock() defer tc.mutex.RUnlock() diff --git a/std/security/trust_config_test.go b/std/security/trust_config_test.go index 8ca26328..07a124d7 100644 --- a/std/security/trust_config_test.go +++ b/std/security/trust_config_test.go @@ -1044,3 +1044,92 @@ func TestTrustConfigLvsInter(t *testing.T) { testTrustConfigInter(t, schemaInter) } + +func makeRevocationRecordWire(t *testing.T, cert ndn.Data, signer ndn.Signer) enc.Wire { + t.Helper() + + recordName, ok := sec.RevocationRecordName(cert) + require.True(t, ok) + + pkt, err := spec.Spec{}.MakeData(recordName, &ndn.DataConfig{}, enc.Wire{[]byte{0x01}}, signer) + require.NoError(t, err) + return pkt.Wire +} + +func TestTrustConfigRevocation(t *testing.T) { + tu.SetT(t) + + clear(tcTestNetwork) + tcTestT = t + network := tcTestNetwork + store := storage.NewMemoryStore() + keychain := keychain.NewKeyChainMem(store) + tcTestKeyChain = keychain + + opts := SignCertOptions{ + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + } + + rootSigner, _ := signer.KeygenEd25519(sec.MakeKeyName(sname("/test"))) + rootCertWire, rootCertData, _ := signCert(rootSigner, tu.NoErr(signer.MarshalSecret(rootSigner)), opts) + network[rootCertData.Name().String()] = rootCertWire + require.NoError(t, keychain.InsertCert(rootCertWire.Join())) + + aliceSigner, _ := signer.KeygenEd25519(sec.MakeKeyName(sname("/test/alice"))) + aliceCertWire, aliceCertData, _ := signCert(rootSigner, tu.NoErr(signer.MarshalSecret(aliceSigner)), opts) + network[aliceCertData.Name().String()] = aliceCertWire + require.NoError(t, keychain.InsertCert(aliceCertWire.Join())) + require.NoError(t, keychain.InsertKey(aliceSigner)) + + bobSigner, _ := signer.KeygenEd25519(sec.MakeKeyName(sname("/test/bob"))) + bobCertWire, bobCertData, _ := signCert(rootSigner, tu.NoErr(signer.MarshalSecret(bobSigner)), opts) + network[bobCertData.Name().String()] = bobCertWire + + schema, err := trust_schema.NewLvsSchema(TRUST_CONFIG_INTRA_LVS) + require.NoError(t, err) + + trust, err := sec.NewTrustConfig(keychain, schema, []enc.Name{rootCertData.Name()}) + require.NoError(t, err) + tcTestTrustConfig = trust + + require.Error(t, trust.InstallRevocationRecord(nil)) + require.Error(t, trust.InstallRevocationRecord(aliceCertWire)) + + recordWire := makeRevocationRecordWire(t, aliceCertData, rootSigner) + require.NoError(t, trust.InstallRevocationRecord(recordWire)) + + recordName, ok := sec.RevocationRecordName(aliceCertData) + require.True(t, ok) + if buf, _ := keychain.Store().Get(recordName, false); buf == nil { + t.Fatal("revocation record not in store") + } + + require.False(t, validateSync(ValidateSyncOptions{ + name: "/test/alice/data1", + signer: aliceSigner, + })) + + // Trust anchors are not subject to revocation checks. + rootRecordWire := makeRevocationRecordWire(t, rootCertData, rootSigner) + require.NoError(t, trust.InstallRevocationRecord(rootRecordWire)) + _, rootSigCov, err := spec.Spec{}.ReadData(enc.NewWireView(rootCertWire)) + require.NoError(t, err) + require.True(t, validateCerts(rootCertData, rootSigCov, false)) + + // Cached certificate path. + tcTestFetchCount = 0 + require.True(t, validateSync(ValidateSyncOptions{ + name: "/test/bob/data1", + signer: bobSigner, + })) + require.Equal(t, 1, tcTestFetchCount) + + bobRecordWire := makeRevocationRecordWire(t, bobCertData, rootSigner) + require.NoError(t, trust.InstallRevocationRecord(bobRecordWire)) + require.False(t, validateSync(ValidateSyncOptions{ + name: "/test/bob/data2", + signer: bobSigner, + })) + require.Equal(t, 1, tcTestFetchCount) +} From 8b19f18e9ca8599b8ee25da878029a0c57ae20f7 Mon Sep 17 00:00:00 2001 From: Harsh Kashyap Date: Thu, 2 Jul 2026 20:09:46 +0530 Subject: [PATCH 04/10] security: encode revocation records as Data packets --- std/security/certificate.go | 213 ++++++++++++++++++++++++------ std/security/certificate_test.go | 12 ++ std/security/trust_config.go | 3 + std/security/trust_config_test.go | 10 +- 4 files changed, 192 insertions(+), 46 deletions(-) diff --git a/std/security/certificate.go b/std/security/certificate.go index 3a6ee091..5c7e483d 100644 --- a/std/security/certificate.go +++ b/std/security/certificate.go @@ -4,12 +4,12 @@ import ( "crypto/sha256" "fmt" "io" - "sync" "time" enc "github.com/named-data/ndnd/std/encoding" "github.com/named-data/ndnd/std/ndn" spec "github.com/named-data/ndnd/std/ndn/spec_2022" + "github.com/named-data/ndnd/std/object/storage" sig "github.com/named-data/ndnd/std/security/signer" "github.com/named-data/ndnd/std/types/optional" ) @@ -18,19 +18,32 @@ type revocationReason uint64 const revocationReasonUnspecified revocationReason = 0 -type revocationRecord struct { - Name enc.Name - CertName enc.Name - PublicKeyHash [sha256.Size]byte - Reason revocationReason - Timestamp time.Time -} +const ( + tlvRevocationTimestamp enc.TLNum = 201 + tlvPublicKeyHash enc.TLNum = 202 + tlvRevocationReason enc.TLNum = 203 + tlvRevocationNotBefore enc.TLNum = 205 +) + +const defaultRevocationFreshness = 8760 * time.Hour -var revokedCertRecord = struct { - sync.RWMutex - records map[string]revocationRecord -}{ - records: map[string]revocationRecord{}, +// revocationStore holds locally installed revocation record Data packets. +var revocationStore = storage.NewMemoryStore() + +// MakeRevocationRecordArgs are the arguments to MakeRevocationRecord. +type MakeRevocationRecordArgs struct { + // Cert is the certificate being revoked. + Cert ndn.Data + // Signer signs the revocation record Data packet. May be nil for unsigned records. + Signer ndn.Signer + // Reason is the revocation reason code. + Reason revocationReason + // Timestamp is the revocation timestamp. Defaults to now. + Timestamp optional.Optional[time.Time] + // NotBefore marks data produced before this timestamp as still valid. + NotBefore optional.Optional[time.Time] + // Freshness is the FreshnessPeriod of the record Data packet. + Freshness optional.Optional[time.Duration] } // SignCertArgs are the arguments to SignCert. @@ -136,16 +149,50 @@ func CertIsExpired(cert ndn.Data) bool { return false } -// Revoke records the certificate name as revoked for this process. +// Revoke records a certificate as revoked by storing a revocation record Data packet. func Revoke(cert ndn.Data) { - record, ok := makeRevocationRecord(cert, revocationReasonUnspecified) - if !ok { + wire, err := MakeRevocationRecord(MakeRevocationRecordArgs{Cert: cert}) + if err != nil { return } - revokedCertRecord.Lock() - defer revokedCertRecord.Unlock() - revokedCertRecord.records[record.Name.TlvStr()] = record + data, _, err := spec.Spec{}.ReadData(enc.NewWireView(wire)) + if err != nil { + return + } + + _ = revocationStore.Put(data.Name(), wire.Join()) +} + +// MakeRevocationRecord builds a revocation record Data packet for a certificate. +func MakeRevocationRecord(args MakeRevocationRecordArgs) (enc.Wire, error) { + if args.Cert == nil { + return nil, fmt.Errorf("certificate is nil") + } + + recordName, ok := revocationRecordName(args.Cert) + if !ok { + return nil, fmt.Errorf("invalid certificate name for revocation: %s", args.Cert.Name()) + } + + timestamp := args.Timestamp.GetOr(time.Now()) + hash := sha256.Sum256(args.Cert.Content().Join()) + content, err := encodeRevocationRecordContent(hash, args.Reason, timestamp, args.NotBefore) + if err != nil { + return nil, err + } + + cfg := &ndn.DataConfig{ + ContentType: optional.Some(ndn.ContentTypeKey), + Freshness: optional.Some(args.Freshness.GetOr(defaultRevocationFreshness)), + } + + encoded, err := spec.Spec{}.MakeData(recordName, cfg, content, args.Signer) + if err != nil { + return nil, err + } + + return encoded.Wire, nil } // RevocationRecordName derives the REVOKE-style record name for a certificate. @@ -153,41 +200,125 @@ func RevocationRecordName(cert ndn.Data) (enc.Name, bool) { return revocationRecordName(cert) } -// IsRevoked reports whether the certificate name has been revoked in this process. +// IsRevoked reports whether a revocation record Data packet is installed for the certificate. func IsRevoked(cert ndn.Data) bool { - key, ok := revocationRecordKey(cert) + recordName, ok := revocationRecordName(cert) if !ok { return false } - revokedCertRecord.RLock() - defer revokedCertRecord.RUnlock() - _, ok = revokedCertRecord.records[key] - return ok + wire, _ := revocationStore.Get(recordName, false) + return len(wire) > 0 } -func makeRevocationRecord(cert ndn.Data, reason revocationReason) (revocationRecord, bool) { - recordName, ok := revocationRecordName(cert) - if !ok { - return revocationRecord{}, false +func encodeRevocationRecordContent( + hash [sha256.Size]byte, + reason revocationReason, + timestamp time.Time, + notBefore optional.Optional[time.Time], +) (enc.Wire, error) { + var inner []byte + inner = appendTlvNat(inner, tlvRevocationTimestamp, uint64(timestamp.UnixMilli())) + inner = appendTlvNat(inner, tlvRevocationReason, uint64(reason)) + inner = appendTlvBytes(inner, tlvPublicKeyHash, hash[:]) + if nb, ok := notBefore.Get(); ok { + inner = appendTlvNat(inner, tlvRevocationNotBefore, uint64(nb.UnixMilli())) } - return revocationRecord{ - Name: recordName, - CertName: stripImplicitDigest(cert.Name()), - PublicKeyHash: sha256.Sum256(cert.Content().Join()), - Reason: reason, - Timestamp: time.Now(), - }, true + return enc.Wire{appendTlv(nil, enc.TLNum(0x15), inner)}, nil } -func revocationRecordKey(cert ndn.Data) (string, bool) { - recordName, ok := revocationRecordName(cert) - if !ok { - return "", false +func appendTlv(dst []byte, typ enc.TLNum, value []byte) []byte { + typeLen := typ.EncodingLength() + lengthLen := enc.TLNum(len(value)).EncodingLength() + out := make([]byte, len(dst)+typeLen+lengthLen+len(value)) + copy(out, dst) + pos := len(dst) + pos += typ.EncodeInto(out[pos:]) + pos += enc.TLNum(len(value)).EncodeInto(out[pos:]) + copy(out[pos:], value) + return out +} + +func appendTlvNat(dst []byte, typ enc.TLNum, n uint64) []byte { + nat := enc.Nat(n) + return appendTlv(dst, typ, nat.Bytes()) +} + +func appendTlvBytes(dst []byte, typ enc.TLNum, value []byte) []byte { + return appendTlv(dst, typ, value) +} + +func validateRevocationRecordData(data ndn.Data) error { + if data == nil { + return fmt.Errorf("revocation record is nil") + } + if !isRevocationRecordName(data.Name()) { + return fmt.Errorf("not a revocation record name: %s", data.Name()) + } + _, err := parseRevocationRecordContent(data.Content()) + return err +} + +func parseRevocationRecordContent(content enc.Wire) (map[enc.TLNum][]byte, error) { + if len(content) == 0 { + return nil, fmt.Errorf("revocation record content is empty") + } + + reader := enc.NewWireView(content) + typ, err := reader.ReadTLNum() + if err != nil { + return nil, err + } + if typ != enc.TLNum(0x15) { + return nil, fmt.Errorf("revocation record content is not a Content TLV") + } + + l, err := reader.ReadTLNum() + if err != nil { + return nil, err + } + + body := reader.Delegate(int(l)) + if body.Length() != int(l) { + return nil, io.ErrUnexpectedEOF + } + + fields := map[enc.TLNum][]byte{} + for !body.IsEOF() { + fieldType, err := body.ReadTLNum() + if err != nil { + return nil, err + } + fieldLen, err := body.ReadTLNum() + if err != nil { + return nil, err + } + fieldView := body.Delegate(int(fieldLen)) + if fieldView.Length() != int(fieldLen) { + return nil, io.ErrUnexpectedEOF + } + buf, err := fieldView.ReadBuf(fieldView.Length()) + if err != nil { + return nil, err + } + fields[fieldType] = buf + } + + if _, ok := fields[tlvRevocationTimestamp]; !ok { + return nil, fmt.Errorf("revocation record missing timestamp") + } + if _, ok := fields[tlvRevocationReason]; !ok { + return nil, fmt.Errorf("revocation record missing reason") + } + if _, ok := fields[tlvPublicKeyHash]; !ok { + return nil, fmt.Errorf("revocation record missing public key hash") + } + if len(fields[tlvPublicKeyHash]) != sha256.Size { + return nil, fmt.Errorf("revocation record public key hash has invalid length") } - return recordName.TlvStr(), true + return fields, nil } func revocationRecordName(cert ndn.Data) (enc.Name, bool) { diff --git a/std/security/certificate_test.go b/std/security/certificate_test.go index 6ea7f44f..efb7b5fe 100644 --- a/std/security/certificate_test.go +++ b/std/security/certificate_test.go @@ -216,6 +216,18 @@ func TestCertificateRevocationRecord(t *testing.T) { require.True(t, sec.IsRevoked(aliceCert)) require.False(t, sec.IsRevoked(otherCert)) + + recordWire, err := sec.MakeRevocationRecord(sec.MakeRevocationRecordArgs{Cert: aliceCert}) + require.NoError(t, err) + recordData, _, err := spec_2022.Spec{}.ReadData(enc.NewWireView(recordWire)) + require.NoError(t, err) + recordName, ok := sec.RevocationRecordName(aliceCert) + require.True(t, ok) + require.True(t, recordName.Equal(recordData.Name())) + contentType, ok := recordData.ContentType().Get() + require.True(t, ok) + require.Equal(t, ndn.ContentTypeKey, contentType) + require.NotEmpty(t, recordData.Content()) } func TestCertificateRevocationRecordConcurrentAccess(t *testing.T) { diff --git a/std/security/trust_config.go b/std/security/trust_config.go index c38e493e..faeae787 100644 --- a/std/security/trust_config.go +++ b/std/security/trust_config.go @@ -108,6 +108,9 @@ func (tc *TrustConfig) InstallRevocationRecord(wire enc.Wire) error { if !isRevocationRecordName(data.Name()) { return fmt.Errorf("not a revocation record name: %s", data.Name()) } + if err := validateRevocationRecordData(data); err != nil { + return err + } tc.mutex.Lock() defer tc.mutex.Unlock() diff --git a/std/security/trust_config_test.go b/std/security/trust_config_test.go index 07a124d7..87742c3d 100644 --- a/std/security/trust_config_test.go +++ b/std/security/trust_config_test.go @@ -1048,12 +1048,12 @@ func TestTrustConfigLvsInter(t *testing.T) { func makeRevocationRecordWire(t *testing.T, cert ndn.Data, signer ndn.Signer) enc.Wire { t.Helper() - recordName, ok := sec.RevocationRecordName(cert) - require.True(t, ok) - - pkt, err := spec.Spec{}.MakeData(recordName, &ndn.DataConfig{}, enc.Wire{[]byte{0x01}}, signer) + wire, err := sec.MakeRevocationRecord(sec.MakeRevocationRecordArgs{ + Cert: cert, + Signer: signer, + }) require.NoError(t, err) - return pkt.Wire + return wire } func TestTrustConfigRevocation(t *testing.T) { From 41be4ad41be370f6c93a1abe8e1f35187e298b29 Mon Sep 17 00:00:00 2001 From: Harsh Kashyap Date: Thu, 2 Jul 2026 20:15:19 +0530 Subject: [PATCH 05/10] security: clean up revocation code comments --- std/security/certificate.go | 38 +++++++------------------------ std/security/certificate_test.go | 3 --- std/security/trust_config.go | 9 ++------ std/security/trust_config_test.go | 8 ++----- 4 files changed, 12 insertions(+), 46 deletions(-) diff --git a/std/security/certificate.go b/std/security/certificate.go index 5c7e483d..7dc59d69 100644 --- a/std/security/certificate.go +++ b/std/security/certificate.go @@ -27,22 +27,14 @@ const ( const defaultRevocationFreshness = 8760 * time.Hour -// revocationStore holds locally installed revocation record Data packets. var revocationStore = storage.NewMemoryStore() -// MakeRevocationRecordArgs are the arguments to MakeRevocationRecord. type MakeRevocationRecordArgs struct { - // Cert is the certificate being revoked. - Cert ndn.Data - // Signer signs the revocation record Data packet. May be nil for unsigned records. - Signer ndn.Signer - // Reason is the revocation reason code. - Reason revocationReason - // Timestamp is the revocation timestamp. Defaults to now. + Cert ndn.Data + Signer ndn.Signer + Reason revocationReason Timestamp optional.Optional[time.Time] - // NotBefore marks data produced before this timestamp as still valid. NotBefore optional.Optional[time.Time] - // Freshness is the FreshnessPeriod of the record Data packet. Freshness optional.Optional[time.Duration] } @@ -67,7 +59,6 @@ type SignCertArgs struct { // SignCert signs a new NDN certificate with the given signer. // Data must have either a Key or Secret in the Content. func SignCert(args SignCertArgs) (enc.Wire, error) { - // Check all parameters (strict for certs) if args.Signer == nil || args.Data == nil || args.IssuerId.Typ == 0 { return nil, ndn.ErrInvalidValue{Item: "SignCertArgs", Value: args} } @@ -80,20 +71,17 @@ func SignCert(args SignCertArgs) (enc.Wire, error) { return nil, ndn.ErrInvalidValue{Item: "Expiry", Value: args.NotAfter} } - // Get public key bits and key name pk, keyName, err := getPubKey(args.Data) if err != nil { return nil, err } - // Get certificate name certName, err := MakeCertName(keyName, args.IssuerId, uint64(time.Now().UnixMilli())) if err != nil { return nil, err } // TODO: set description - // Create certificate data cfg := &ndn.DataConfig{ ContentType: optional.Some(ndn.ContentTypeKey), Freshness: optional.Some(time.Hour), @@ -131,7 +119,7 @@ func SelfSign(args SignCertArgs) (wire enc.Wire, err error) { return SignCert(args) } -// (AI GENERATED DESCRIPTION): Returns true if the certificate’s signature is nil or its validity period does not include the current time. +// CertIsExpired reports whether the certificate is outside its validity period. func CertIsExpired(cert ndn.Data) bool { if cert.Signature() == nil { return true @@ -177,10 +165,7 @@ func MakeRevocationRecord(args MakeRevocationRecordArgs) (enc.Wire, error) { timestamp := args.Timestamp.GetOr(time.Now()) hash := sha256.Sum256(args.Cert.Content().Join()) - content, err := encodeRevocationRecordContent(hash, args.Reason, timestamp, args.NotBefore) - if err != nil { - return nil, err - } + content := encodeRevocationRecordContent(hash, args.Reason, timestamp, args.NotBefore) cfg := &ndn.DataConfig{ ContentType: optional.Some(ndn.ContentTypeKey), @@ -216,16 +201,16 @@ func encodeRevocationRecordContent( reason revocationReason, timestamp time.Time, notBefore optional.Optional[time.Time], -) (enc.Wire, error) { +) enc.Wire { var inner []byte inner = appendTlvNat(inner, tlvRevocationTimestamp, uint64(timestamp.UnixMilli())) inner = appendTlvNat(inner, tlvRevocationReason, uint64(reason)) - inner = appendTlvBytes(inner, tlvPublicKeyHash, hash[:]) + inner = appendTlv(inner, tlvPublicKeyHash, hash[:]) if nb, ok := notBefore.Get(); ok { inner = appendTlvNat(inner, tlvRevocationNotBefore, uint64(nb.UnixMilli())) } - return enc.Wire{appendTlv(nil, enc.TLNum(0x15), inner)}, nil + return enc.Wire{appendTlv(nil, enc.TLNum(0x15), inner)} } func appendTlv(dst []byte, typ enc.TLNum, value []byte) []byte { @@ -245,10 +230,6 @@ func appendTlvNat(dst []byte, typ enc.TLNum, n uint64) []byte { return appendTlv(dst, typ, nat.Bytes()) } -func appendTlvBytes(dst []byte, typ enc.TLNum, value []byte) []byte { - return appendTlv(dst, typ, value) -} - func validateRevocationRecordData(data ndn.Data) error { if data == nil { return fmt.Errorf("revocation record is nil") @@ -362,7 +343,6 @@ func getPubKey(data ndn.Data) ([]byte, enc.Name, error) { switch contentType { case ndn.ContentTypeKey: - // Content is public key, return directly pub := data.Content().Join() keyName, err := GetKeyNameFromCertName(data.Name()) if err != nil { @@ -370,7 +350,6 @@ func getPubKey(data ndn.Data) ([]byte, enc.Name, error) { } return pub, keyName, nil case ndn.ContentTypeSigningKey: - // Content is private key, parse the signer signer, err := sig.UnmarshalSecret(data) if err != nil { return nil, nil, err @@ -381,7 +360,6 @@ func getPubKey(data ndn.Data) ([]byte, enc.Name, error) { } return pub, signer.KeyName(), nil default: - // Invalid content type return nil, nil, ndn.ErrInvalidValue{Item: "Data.ContentType", Value: contentType} } } diff --git a/std/security/certificate_test.go b/std/security/certificate_test.go index efb7b5fe..77d4daae 100644 --- a/std/security/certificate_test.go +++ b/std/security/certificate_test.go @@ -16,7 +16,6 @@ import ( "github.com/stretchr/testify/require" ) -// (AI GENERATED DESCRIPTION): Tests that the `SignCert` function correctly returns an error when invoked with a nil signer and nil data. func TestSignCertInvalid(t *testing.T) { tu.SetT(t) @@ -28,7 +27,6 @@ func TestSignCertInvalid(t *testing.T) { require.Error(t, err) } -// (AI GENERATED DESCRIPTION): Verifies that a key can self‑sign its own certificate, checking the certificate name, public‑key content, validity period, and Ed25519 signature correctness. func TestSignCertSelf(t *testing.T) { tu.SetT(t) @@ -76,7 +74,6 @@ func TestSignCertSelf(t *testing.T) { require.True(t, tu.NoErr(signer.ValidateData(cert, certSigCov, cert))) } -// (AI GENERATED DESCRIPTION): Verifies that Alice’s signer can correctly sign a root certificate, producing a certificate with the expected name, content, signature format, validity period, and that the signature verifies against Alice’s own certificate. func TestSignCertOther(t *testing.T) { tu.SetT(t) diff --git a/std/security/trust_config.go b/std/security/trust_config.go index faeae787..cb0de2ef 100644 --- a/std/security/trust_config.go +++ b/std/security/trust_config.go @@ -81,7 +81,6 @@ func NewTrustConfig(keyChain ndn.KeyChain, schema ndn.TrustSchema, roots []enc.N }, nil } -// (AI GENERATED DESCRIPTION): Returns the constant string `"trust-config"` for a `TrustConfig` value, enabling string formatting via the `fmt.Stringer` interface. func (tc *TrustConfig) String() string { return "trust-config" } @@ -94,8 +93,7 @@ func (tc *TrustConfig) Suggest(name enc.Name) ndn.Signer { return tc.schema.Suggest(name, tc.keychain) } -// InstallRevocationRecord stores a received revocation record in the keychain store. -// The wire must encode a Data packet whose name follows the REVOKE naming convention. +// InstallRevocationRecord stores a revocation record Data packet in the keychain store. func (tc *TrustConfig) InstallRevocationRecord(wire enc.Wire) error { if len(wire) == 0 { return fmt.Errorf("revocation record wire is empty") @@ -373,9 +371,7 @@ func (tc *TrustConfig) Validate(args TrustConfigValidateArgs) { } // Cert not found, attempt to fetch from network. - // TODO(revocation): the engine can express an Interest for the derived REVOKE - // record name when no local record is found. For now, only records installed - // via InstallRevocationRecord in keychain.Store() are checked. + // TODO(revocation): fetch REVOKE record by Interest when not in keychain.Store(). fetchCfg := &ndn.InterestConfig{ CanBePrefix: true, MustBeFresh: true, @@ -432,7 +428,6 @@ func (tc *TrustConfig) Validate(args TrustConfigValidateArgs) { args.Fetch(keyLocator, fetchCfg, cb) } -// (AI GENERATED DESCRIPTION): Validates the cross‑schema signed Data packet by parsing its embedded schema, checking its validity period, ensuring it authorizes the original certificate, and recursively validating the cross‑schema’s signature against the trust configuration. func (tc *TrustConfig) validateCrossSchema(args TrustConfigValidateArgs) { crossWire := args.Data.CrossSchema() if crossWire == nil { diff --git a/std/security/trust_config_test.go b/std/security/trust_config_test.go index 87742c3d..70352ceb 100644 --- a/std/security/trust_config_test.go +++ b/std/security/trust_config_test.go @@ -67,7 +67,6 @@ type SignCertOptions struct { NotAfter time.Time } -// (AI GENERATED DESCRIPTION): Creates a signed certificate for the supplied Data packet using the provided signer, and returns the certificate wire, its content, and the signature‑covered portion. func signCert(signer ndn.Signer, wire enc.Wire, opts SignCertOptions) (enc.Wire, ndn.Data, enc.Wire) { data, _, _ := spec.Spec{}.ReadData(enc.NewWireView(wire)) cert, _ := sec.SignCert(sec.SignCertArgs{ @@ -495,8 +494,6 @@ func testTrustConfigIntra(t *testing.T, schema ndn.TrustSchema) { })) require.Equal(t, 4, tcTestFetchCount) // (same as root 1, except no mallory root fetch) - // ======================================================================== - // Test with cross schema validation // Alice signs a cross schema for bob to allow bob to publish in alice's namespace abInvite, err := trust_schema.SignCrossSchema(trust_schema.SignCrossSchemaArgs{ @@ -1022,7 +1019,6 @@ func testTrustConfigInter(t *testing.T, schema ndn.TrustSchema) { } } -// (AI GENERATED DESCRIPTION): Initializes an in‑memory store and key chain, loads an LVS trust schema, and runs trust configuration tests. func TestTrustConfigLvsIntra(t *testing.T) { tu.SetT(t) @@ -1110,14 +1106,14 @@ func TestTrustConfigRevocation(t *testing.T) { signer: aliceSigner, })) - // Trust anchors are not subject to revocation checks. + // Trust anchors skip revocation checks. rootRecordWire := makeRevocationRecordWire(t, rootCertData, rootSigner) require.NoError(t, trust.InstallRevocationRecord(rootRecordWire)) _, rootSigCov, err := spec.Spec{}.ReadData(enc.NewWireView(rootCertWire)) require.NoError(t, err) require.True(t, validateCerts(rootCertData, rootSigCov, false)) - // Cached certificate path. + // Cached cert path. tcTestFetchCount = 0 require.True(t, validateSync(ValidateSyncOptions{ name: "/test/bob/data1", From f322b0a202a5173282fcc6e40bc4de6fbf961ce0 Mon Sep 17 00:00:00 2001 From: Harsh Kashyap Date: Sat, 4 Jul 2026 12:50:13 +0530 Subject: [PATCH 06/10] Address PR review: stateless RevokeCert and TLV codegen --- std/security/certificate.go | 185 +++------------ std/security/certificate_test.go | 69 +----- std/security/revocation_tlv/definitions.go | 18 ++ std/security/revocation_tlv/zz_generated.go | 248 ++++++++++++++++++++ std/security/trust_config.go | 32 +-- std/security/trust_config_test.go | 23 +- 6 files changed, 318 insertions(+), 257 deletions(-) create mode 100644 std/security/revocation_tlv/definitions.go create mode 100644 std/security/revocation_tlv/zz_generated.go diff --git a/std/security/certificate.go b/std/security/certificate.go index 7dc59d69..5f7f4b24 100644 --- a/std/security/certificate.go +++ b/std/security/certificate.go @@ -9,7 +9,7 @@ import ( enc "github.com/named-data/ndnd/std/encoding" "github.com/named-data/ndnd/std/ndn" spec "github.com/named-data/ndnd/std/ndn/spec_2022" - "github.com/named-data/ndnd/std/object/storage" + revocationtlv "github.com/named-data/ndnd/std/security/revocation_tlv" sig "github.com/named-data/ndnd/std/security/signer" "github.com/named-data/ndnd/std/types/optional" ) @@ -18,23 +18,21 @@ type revocationReason uint64 const revocationReasonUnspecified revocationReason = 0 -const ( - tlvRevocationTimestamp enc.TLNum = 201 - tlvPublicKeyHash enc.TLNum = 202 - tlvRevocationReason enc.TLNum = 203 - tlvRevocationNotBefore enc.TLNum = 205 -) - const defaultRevocationFreshness = 8760 * time.Hour -var revocationStore = storage.NewMemoryStore() - -type MakeRevocationRecordArgs struct { - Cert ndn.Data - Signer ndn.Signer - Reason revocationReason +// RevokeCertArgs are the arguments to RevokeCert. +type RevokeCertArgs struct { + // Cert is the certificate being revoked. + Cert ndn.Data + // Signer signs the revocation record Data packet. May be nil for unsigned records. + Signer ndn.Signer + // Reason is the revocation reason code. + Reason revocationReason + // Timestamp is the revocation timestamp. Defaults to now. Timestamp optional.Optional[time.Time] + // NotBefore marks data produced before this timestamp as still valid. NotBefore optional.Optional[time.Time] + // Freshness is the FreshnessPeriod of the record Data packet. Freshness optional.Optional[time.Duration] } @@ -59,6 +57,7 @@ type SignCertArgs struct { // SignCert signs a new NDN certificate with the given signer. // Data must have either a Key or Secret in the Content. func SignCert(args SignCertArgs) (enc.Wire, error) { + // Check all parameters (strict for certs) if args.Signer == nil || args.Data == nil || args.IssuerId.Typ == 0 { return nil, ndn.ErrInvalidValue{Item: "SignCertArgs", Value: args} } @@ -71,17 +70,20 @@ func SignCert(args SignCertArgs) (enc.Wire, error) { return nil, ndn.ErrInvalidValue{Item: "Expiry", Value: args.NotAfter} } + // Get public key bits and key name pk, keyName, err := getPubKey(args.Data) if err != nil { return nil, err } + // Get certificate name certName, err := MakeCertName(keyName, args.IssuerId, uint64(time.Now().UnixMilli())) if err != nil { return nil, err } // TODO: set description + // Create certificate data cfg := &ndn.DataConfig{ ContentType: optional.Some(ndn.ContentTypeKey), Freshness: optional.Some(time.Hour), @@ -137,23 +139,8 @@ func CertIsExpired(cert ndn.Data) bool { return false } -// Revoke records a certificate as revoked by storing a revocation record Data packet. -func Revoke(cert ndn.Data) { - wire, err := MakeRevocationRecord(MakeRevocationRecordArgs{Cert: cert}) - if err != nil { - return - } - - data, _, err := spec.Spec{}.ReadData(enc.NewWireView(wire)) - if err != nil { - return - } - - _ = revocationStore.Put(data.Name(), wire.Join()) -} - -// MakeRevocationRecord builds a revocation record Data packet for a certificate. -func MakeRevocationRecord(args MakeRevocationRecordArgs) (enc.Wire, error) { +// RevokeCert builds a signed revocation record Data packet for a certificate. +func RevokeCert(args RevokeCertArgs) (enc.Wire, error) { if args.Cert == nil { return nil, fmt.Errorf("certificate is nil") } @@ -165,14 +152,21 @@ func MakeRevocationRecord(args MakeRevocationRecordArgs) (enc.Wire, error) { timestamp := args.Timestamp.GetOr(time.Now()) hash := sha256.Sum256(args.Cert.Content().Join()) - content := encodeRevocationRecordContent(hash, args.Reason, timestamp, args.NotBefore) + record := revocationtlv.RevocationRecord{ + Timestamp: uint64(timestamp.UnixMilli()), + Reason: uint64(args.Reason), + PublicKeyHash: hash[:], + } + if nb, ok := args.NotBefore.Get(); ok { + record.NotBefore = optional.Some(uint64(nb.UnixMilli())) + } cfg := &ndn.DataConfig{ ContentType: optional.Some(ndn.ContentTypeKey), Freshness: optional.Some(args.Freshness.GetOr(defaultRevocationFreshness)), } - encoded, err := spec.Spec{}.MakeData(recordName, cfg, content, args.Signer) + encoded, err := spec.Spec{}.MakeData(recordName, cfg, record.Encode(), args.Signer) if err != nil { return nil, err } @@ -180,128 +174,6 @@ func MakeRevocationRecord(args MakeRevocationRecordArgs) (enc.Wire, error) { return encoded.Wire, nil } -// RevocationRecordName derives the REVOKE-style record name for a certificate. -func RevocationRecordName(cert ndn.Data) (enc.Name, bool) { - return revocationRecordName(cert) -} - -// IsRevoked reports whether a revocation record Data packet is installed for the certificate. -func IsRevoked(cert ndn.Data) bool { - recordName, ok := revocationRecordName(cert) - if !ok { - return false - } - - wire, _ := revocationStore.Get(recordName, false) - return len(wire) > 0 -} - -func encodeRevocationRecordContent( - hash [sha256.Size]byte, - reason revocationReason, - timestamp time.Time, - notBefore optional.Optional[time.Time], -) enc.Wire { - var inner []byte - inner = appendTlvNat(inner, tlvRevocationTimestamp, uint64(timestamp.UnixMilli())) - inner = appendTlvNat(inner, tlvRevocationReason, uint64(reason)) - inner = appendTlv(inner, tlvPublicKeyHash, hash[:]) - if nb, ok := notBefore.Get(); ok { - inner = appendTlvNat(inner, tlvRevocationNotBefore, uint64(nb.UnixMilli())) - } - - return enc.Wire{appendTlv(nil, enc.TLNum(0x15), inner)} -} - -func appendTlv(dst []byte, typ enc.TLNum, value []byte) []byte { - typeLen := typ.EncodingLength() - lengthLen := enc.TLNum(len(value)).EncodingLength() - out := make([]byte, len(dst)+typeLen+lengthLen+len(value)) - copy(out, dst) - pos := len(dst) - pos += typ.EncodeInto(out[pos:]) - pos += enc.TLNum(len(value)).EncodeInto(out[pos:]) - copy(out[pos:], value) - return out -} - -func appendTlvNat(dst []byte, typ enc.TLNum, n uint64) []byte { - nat := enc.Nat(n) - return appendTlv(dst, typ, nat.Bytes()) -} - -func validateRevocationRecordData(data ndn.Data) error { - if data == nil { - return fmt.Errorf("revocation record is nil") - } - if !isRevocationRecordName(data.Name()) { - return fmt.Errorf("not a revocation record name: %s", data.Name()) - } - _, err := parseRevocationRecordContent(data.Content()) - return err -} - -func parseRevocationRecordContent(content enc.Wire) (map[enc.TLNum][]byte, error) { - if len(content) == 0 { - return nil, fmt.Errorf("revocation record content is empty") - } - - reader := enc.NewWireView(content) - typ, err := reader.ReadTLNum() - if err != nil { - return nil, err - } - if typ != enc.TLNum(0x15) { - return nil, fmt.Errorf("revocation record content is not a Content TLV") - } - - l, err := reader.ReadTLNum() - if err != nil { - return nil, err - } - - body := reader.Delegate(int(l)) - if body.Length() != int(l) { - return nil, io.ErrUnexpectedEOF - } - - fields := map[enc.TLNum][]byte{} - for !body.IsEOF() { - fieldType, err := body.ReadTLNum() - if err != nil { - return nil, err - } - fieldLen, err := body.ReadTLNum() - if err != nil { - return nil, err - } - fieldView := body.Delegate(int(fieldLen)) - if fieldView.Length() != int(fieldLen) { - return nil, io.ErrUnexpectedEOF - } - buf, err := fieldView.ReadBuf(fieldView.Length()) - if err != nil { - return nil, err - } - fields[fieldType] = buf - } - - if _, ok := fields[tlvRevocationTimestamp]; !ok { - return nil, fmt.Errorf("revocation record missing timestamp") - } - if _, ok := fields[tlvRevocationReason]; !ok { - return nil, fmt.Errorf("revocation record missing reason") - } - if _, ok := fields[tlvPublicKeyHash]; !ok { - return nil, fmt.Errorf("revocation record missing public key hash") - } - if len(fields[tlvPublicKeyHash]) != sha256.Size { - return nil, fmt.Errorf("revocation record public key hash has invalid length") - } - - return fields, nil -} - func revocationRecordName(cert ndn.Data) (enc.Name, bool) { if cert == nil { return nil, false @@ -343,6 +215,7 @@ func getPubKey(data ndn.Data) ([]byte, enc.Name, error) { switch contentType { case ndn.ContentTypeKey: + // Content is public key, return directly pub := data.Content().Join() keyName, err := GetKeyNameFromCertName(data.Name()) if err != nil { @@ -350,6 +223,7 @@ func getPubKey(data ndn.Data) ([]byte, enc.Name, error) { } return pub, keyName, nil case ndn.ContentTypeSigningKey: + // Content is private key, parse the signer signer, err := sig.UnmarshalSecret(data) if err != nil { return nil, nil, err @@ -360,6 +234,7 @@ func getPubKey(data ndn.Data) ([]byte, enc.Name, error) { } return pub, signer.KeyName(), nil default: + // Invalid content type return nil, nil, ndn.ErrInvalidValue{Item: "Data.ContentType", Value: contentType} } } diff --git a/std/security/certificate_test.go b/std/security/certificate_test.go index 77d4daae..1fe7929d 100644 --- a/std/security/certificate_test.go +++ b/std/security/certificate_test.go @@ -3,7 +3,6 @@ package security_test import ( "encoding/base64" "fmt" - "sync" "testing" "time" @@ -16,6 +15,7 @@ import ( "github.com/stretchr/testify/require" ) +// (AI GENERATED DESCRIPTION): Tests that the `SignCert` function correctly returns an error when invoked with a nil signer and nil data. func TestSignCertInvalid(t *testing.T) { tu.SetT(t) @@ -27,6 +27,7 @@ func TestSignCertInvalid(t *testing.T) { require.Error(t, err) } +// (AI GENERATED DESCRIPTION): Verifies that a key can self‑sign its own certificate, checking the certificate name, public‑key content, validity period, and Ed25519 signature correctness. func TestSignCertSelf(t *testing.T) { tu.SetT(t) @@ -74,6 +75,7 @@ func TestSignCertSelf(t *testing.T) { require.True(t, tu.NoErr(signer.ValidateData(cert, certSigCov, cert))) } +// (AI GENERATED DESCRIPTION): Verifies that Alice’s signer can correctly sign a root certificate, producing a certificate with the expected name, content, signature format, validity period, and that the signature verifies against Alice’s own certificate. func TestSignCertOther(t *testing.T) { tu.SetT(t) @@ -191,76 +193,23 @@ func TestCertificateRevocationRecord(t *testing.T) { aliceCert, _, err := spec_2022.Spec{}.ReadData(enc.NewWireView(aliceCertWire)) require.NoError(t, err) - otherCertWire := tu.NoErr(sec.SignCert(sec.SignCertArgs{ - Signer: aliceSigner, - Data: aliceKeyData, - IssuerId: revocationTestIssuer(t), - NotBefore: T1, - NotAfter: T2, - })) - otherCert, _, err := spec_2022.Spec{}.ReadData(enc.NewWireView(otherCertWire)) - require.NoError(t, err) + _, err = sec.RevokeCert(sec.RevokeCertArgs{Cert: nil}) + require.Error(t, err) - require.False(t, sec.IsRevoked(nil)) - require.NotPanics(t, func() { - sec.Revoke(nil) + recordWire, err := sec.RevokeCert(sec.RevokeCertArgs{ + Cert: aliceCert, + Signer: aliceSigner, }) - - require.False(t, sec.IsRevoked(aliceCert)) - require.False(t, sec.IsRevoked(otherCert)) - - sec.Revoke(aliceCert) - - require.True(t, sec.IsRevoked(aliceCert)) - require.False(t, sec.IsRevoked(otherCert)) - - recordWire, err := sec.MakeRevocationRecord(sec.MakeRevocationRecordArgs{Cert: aliceCert}) require.NoError(t, err) recordData, _, err := spec_2022.Spec{}.ReadData(enc.NewWireView(recordWire)) require.NoError(t, err) - recordName, ok := sec.RevocationRecordName(aliceCert) - require.True(t, ok) - require.True(t, recordName.Equal(recordData.Name())) + require.Contains(t, recordData.Name().String(), "/REVOKE/") contentType, ok := recordData.ContentType().Get() require.True(t, ok) require.Equal(t, ndn.ContentTypeKey, contentType) require.NotEmpty(t, recordData.Content()) } -func TestCertificateRevocationRecordConcurrentAccess(t *testing.T) { - tu.SetT(t) - - aliceKey, _ := base64.StdEncoding.DecodeString(KEY_ALICE) - aliceKeyData, _, _ := spec_2022.Spec{}.ReadData(enc.NewBufferView(aliceKey)) - aliceSigner := tu.NoErr(signer.UnmarshalSecret(aliceKeyData)) - - aliceCertWire := tu.NoErr(sec.SignCert(sec.SignCertArgs{ - Signer: aliceSigner, - Data: aliceKeyData, - IssuerId: revocationTestIssuer(t), - NotBefore: T1, - NotAfter: T2, - })) - aliceCert, _, err := spec_2022.Spec{}.ReadData(enc.NewWireView(aliceCertWire)) - require.NoError(t, err) - - var wg sync.WaitGroup - for range 50 { - wg.Add(2) - go func() { - defer wg.Done() - sec.Revoke(aliceCert) - }() - go func() { - defer wg.Done() - _ = sec.IsRevoked(aliceCert) - }() - } - wg.Wait() - - require.True(t, sec.IsRevoked(aliceCert)) -} - func revocationTestIssuer(t *testing.T) enc.Component { t.Helper() return enc.NewGenericComponent(fmt.Sprintf("%s-%d", t.Name(), time.Now().UnixNano())) diff --git a/std/security/revocation_tlv/definitions.go b/std/security/revocation_tlv/definitions.go new file mode 100644 index 00000000..66e87b17 --- /dev/null +++ b/std/security/revocation_tlv/definitions.go @@ -0,0 +1,18 @@ +//go:generate gondn_tlv_gen +package revocationtlv + +import "github.com/named-data/ndnd/std/types/optional" + +// RevocationRecord content TLVs per UCLA-IRL/ndnrevoke. +// +// +tlv-model:ordered +type RevocationRecord struct { + //+field:natural + Timestamp uint64 `tlv:"0xC9"` + //+field:natural + Reason uint64 `tlv:"0xCB"` + //+field:binary + PublicKeyHash []byte `tlv:"0xCA"` + //+field:natural:optional + NotBefore optional.Optional[uint64] `tlv:"0xCD"` +} diff --git a/std/security/revocation_tlv/zz_generated.go b/std/security/revocation_tlv/zz_generated.go new file mode 100644 index 00000000..c10de755 --- /dev/null +++ b/std/security/revocation_tlv/zz_generated.go @@ -0,0 +1,248 @@ +// Code generated by ndn tlv codegen DO NOT EDIT. +package revocationtlv + +import ( + "io" + + enc "github.com/named-data/ndnd/std/encoding" +) + +type RevocationRecordEncoder struct { + Length uint +} + +type RevocationRecordParsingContext struct { +} + +func (encoder *RevocationRecordEncoder) Init(value *RevocationRecord) { + + l := uint(0) + l += 1 + l += uint(1 + enc.Nat(value.Timestamp).EncodingLength()) + l += 1 + l += uint(1 + enc.Nat(value.Reason).EncodingLength()) + if value.PublicKeyHash != nil { + l += 1 + l += uint(enc.TLNum(len(value.PublicKeyHash)).EncodingLength()) + l += uint(len(value.PublicKeyHash)) + } + if optval, ok := value.NotBefore.Get(); ok { + l += 1 + l += uint(1 + enc.Nat(optval).EncodingLength()) + } + encoder.Length = l + +} + +func (context *RevocationRecordParsingContext) Init() { + +} + +func (encoder *RevocationRecordEncoder) EncodeInto(value *RevocationRecord, buf []byte) { + + pos := uint(0) + + buf[pos] = byte(201) + pos += 1 + + buf[pos] = byte(enc.Nat(value.Timestamp).EncodeInto(buf[pos+1:])) + pos += uint(1 + buf[pos]) + buf[pos] = byte(203) + pos += 1 + + buf[pos] = byte(enc.Nat(value.Reason).EncodeInto(buf[pos+1:])) + pos += uint(1 + buf[pos]) + if value.PublicKeyHash != nil { + buf[pos] = byte(202) + pos += 1 + pos += uint(enc.TLNum(len(value.PublicKeyHash)).EncodeInto(buf[pos:])) + copy(buf[pos:], value.PublicKeyHash) + pos += uint(len(value.PublicKeyHash)) + } + if optval, ok := value.NotBefore.Get(); ok { + buf[pos] = byte(205) + pos += 1 + + buf[pos] = byte(enc.Nat(optval).EncodeInto(buf[pos+1:])) + pos += uint(1 + buf[pos]) + + } +} + +func (encoder *RevocationRecordEncoder) Encode(value *RevocationRecord) enc.Wire { + + wire := make(enc.Wire, 1) + wire[0] = make([]byte, encoder.Length) + buf := wire[0] + encoder.EncodeInto(value, buf) + + return wire +} + +func (context *RevocationRecordParsingContext) Parse(reader enc.WireView, ignoreCritical bool) (*RevocationRecord, error) { + + var handled_Timestamp bool = false + var handled_Reason bool = false + var handled_PublicKeyHash bool = false + var handled_NotBefore bool = false + + progress := -1 + _ = progress + + value := &RevocationRecord{} + var err error + var startPos int + for { + startPos = reader.Pos() + if startPos >= reader.Length() { + break + } + typ := enc.TLNum(0) + l := enc.TLNum(0) + typ, err = reader.ReadTLNum() + if err != nil { + return nil, enc.ErrFailToParse{TypeNum: 0, Err: err} + } + l, err = reader.ReadTLNum() + if err != nil { + return nil, enc.ErrFailToParse{TypeNum: 0, Err: err} + } + + err = nil + for handled := false; !handled && progress < 4; progress++ { + switch typ { + case 201: + if progress+1 == 0 { + handled = true + handled_Timestamp = true + value.Timestamp = uint64(0) + { + for i := 0; i < int(l); i++ { + x := byte(0) + x, err = reader.ReadByte() + if err != nil { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + break + } + value.Timestamp = uint64(value.Timestamp<<8) | uint64(x) + } + } + } + case 203: + if progress+1 == 1 { + handled = true + handled_Reason = true + value.Reason = uint64(0) + { + for i := 0; i < int(l); i++ { + x := byte(0) + x, err = reader.ReadByte() + if err != nil { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + break + } + value.Reason = uint64(value.Reason<<8) | uint64(x) + } + } + } + case 202: + if progress+1 == 2 { + handled = true + handled_PublicKeyHash = true + value.PublicKeyHash = make([]byte, l) + _, err = reader.ReadFull(value.PublicKeyHash) + } + case 205: + if progress+1 == 3 { + handled = true + handled_NotBefore = true + { + optval := uint64(0) + optval = uint64(0) + { + for i := 0; i < int(l); i++ { + x := byte(0) + x, err = reader.ReadByte() + if err != nil { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + break + } + optval = uint64(optval<<8) | uint64(x) + } + } + value.NotBefore.Set(optval) + } + } + default: + if !ignoreCritical && ((typ <= 31) || ((typ & 1) == 1)) { + return nil, enc.ErrUnrecognizedField{TypeNum: typ} + } + handled = true + err = reader.Skip(int(l)) + } + if err == nil && !handled { + switch progress { + case 0 - 1: + handled_Timestamp = true + err = enc.ErrSkipRequired{Name: "Timestamp", TypeNum: 201} + case 1 - 1: + handled_Reason = true + err = enc.ErrSkipRequired{Name: "Reason", TypeNum: 203} + case 2 - 1: + handled_PublicKeyHash = true + value.PublicKeyHash = nil + case 3 - 1: + handled_NotBefore = true + value.NotBefore.Unset() + } + } + if err != nil { + return nil, enc.ErrFailToParse{TypeNum: typ, Err: err} + } + } + } + + startPos = reader.Pos() + err = nil + + if !handled_Timestamp && err == nil { + err = enc.ErrSkipRequired{Name: "Timestamp", TypeNum: 201} + } + if !handled_Reason && err == nil { + err = enc.ErrSkipRequired{Name: "Reason", TypeNum: 203} + } + if !handled_PublicKeyHash && err == nil { + value.PublicKeyHash = nil + } + if !handled_NotBefore && err == nil { + value.NotBefore.Unset() + } + + if err != nil { + return nil, err + } + + return value, nil +} + +func (value *RevocationRecord) Encode() enc.Wire { + encoder := RevocationRecordEncoder{} + encoder.Init(value) + return encoder.Encode(value) +} + +func (value *RevocationRecord) Bytes() []byte { + return value.Encode().Join() +} + +func ParseRevocationRecord(reader enc.WireView, ignoreCritical bool) (*RevocationRecord, error) { + context := RevocationRecordParsingContext{} + context.Init() + return context.Parse(reader, ignoreCritical) +} diff --git a/std/security/trust_config.go b/std/security/trust_config.go index cb0de2ef..0da51cd7 100644 --- a/std/security/trust_config.go +++ b/std/security/trust_config.go @@ -93,8 +93,8 @@ func (tc *TrustConfig) Suggest(name enc.Name) ndn.Signer { return tc.schema.Suggest(name, tc.keychain) } -// InstallRevocationRecord stores a revocation record Data packet in the keychain store. -func (tc *TrustConfig) InstallRevocationRecord(wire enc.Wire) error { +// InsertRevoke stores a revocation record Data packet. +func (tc *TrustConfig) InsertRevoke(wire enc.Wire) error { if len(wire) == 0 { return fmt.Errorf("revocation record wire is empty") } @@ -106,12 +106,10 @@ func (tc *TrustConfig) InstallRevocationRecord(wire enc.Wire) error { if !isRevocationRecordName(data.Name()) { return fmt.Errorf("not a revocation record name: %s", data.Name()) } - if err := validateRevocationRecordData(data); err != nil { - return err - } tc.mutex.Lock() defer tc.mutex.Unlock() + // HACK: revocation records live in keychain.Store() until we have a dedicated store. return tc.keychain.Store().Put(data.Name(), wire.Join()) } @@ -543,38 +541,18 @@ func (tc *TrustConfig) PromoteAnchor(cert ndn.Data, raw enc.Wire) { tc.roots = append(tc.roots, name) } -func (tc *TrustConfig) isTrustedAnchorCert(cert ndn.Data) bool { +func (tc *TrustConfig) rejectIfRevokedCert(cert ndn.Data, callback func(bool, error)) bool { if cert == nil { return false } - name := stripImplicitDigest(cert.Name()) - tc.mutex.RLock() - defer tc.mutex.RUnlock() - for _, root := range tc.roots { - if root.Equal(name) { - return true - } - } - return false -} - -func (tc *TrustConfig) certIsRevoked(cert ndn.Data) bool { - if cert == nil || tc.isTrustedAnchorCert(cert) { - return false - } - recordName, ok := revocationRecordName(cert) if !ok { return false } wire, _ := tc.keychain.Store().Get(recordName, false) - return len(wire) > 0 -} - -func (tc *TrustConfig) rejectIfRevokedCert(cert ndn.Data, callback func(bool, error)) bool { - if tc.certIsRevoked(cert) { + if len(wire) > 0 { callback(false, fmt.Errorf("certificate is revoked: %s", cert.Name())) return true } diff --git a/std/security/trust_config_test.go b/std/security/trust_config_test.go index 70352ceb..7dc61472 100644 --- a/std/security/trust_config_test.go +++ b/std/security/trust_config_test.go @@ -1044,7 +1044,7 @@ func TestTrustConfigLvsInter(t *testing.T) { func makeRevocationRecordWire(t *testing.T, cert ndn.Data, signer ndn.Signer) enc.Wire { t.Helper() - wire, err := sec.MakeRevocationRecord(sec.MakeRevocationRecordArgs{ + wire, err := sec.RevokeCert(sec.RevokeCertArgs{ Cert: cert, Signer: signer, }) @@ -1089,15 +1089,15 @@ func TestTrustConfigRevocation(t *testing.T) { require.NoError(t, err) tcTestTrustConfig = trust - require.Error(t, trust.InstallRevocationRecord(nil)) - require.Error(t, trust.InstallRevocationRecord(aliceCertWire)) + require.Error(t, trust.InsertRevoke(nil)) + require.Error(t, trust.InsertRevoke(aliceCertWire)) recordWire := makeRevocationRecordWire(t, aliceCertData, rootSigner) - require.NoError(t, trust.InstallRevocationRecord(recordWire)) + require.NoError(t, trust.InsertRevoke(recordWire)) - recordName, ok := sec.RevocationRecordName(aliceCertData) - require.True(t, ok) - if buf, _ := keychain.Store().Get(recordName, false); buf == nil { + recordData, _, err := spec.Spec{}.ReadData(enc.NewWireView(recordWire)) + require.NoError(t, err) + if buf, _ := keychain.Store().Get(recordData.Name(), false); buf == nil { t.Fatal("revocation record not in store") } @@ -1106,13 +1106,6 @@ func TestTrustConfigRevocation(t *testing.T) { signer: aliceSigner, })) - // Trust anchors skip revocation checks. - rootRecordWire := makeRevocationRecordWire(t, rootCertData, rootSigner) - require.NoError(t, trust.InstallRevocationRecord(rootRecordWire)) - _, rootSigCov, err := spec.Spec{}.ReadData(enc.NewWireView(rootCertWire)) - require.NoError(t, err) - require.True(t, validateCerts(rootCertData, rootSigCov, false)) - // Cached cert path. tcTestFetchCount = 0 require.True(t, validateSync(ValidateSyncOptions{ @@ -1122,7 +1115,7 @@ func TestTrustConfigRevocation(t *testing.T) { require.Equal(t, 1, tcTestFetchCount) bobRecordWire := makeRevocationRecordWire(t, bobCertData, rootSigner) - require.NoError(t, trust.InstallRevocationRecord(bobRecordWire)) + require.NoError(t, trust.InsertRevoke(bobRecordWire)) require.False(t, validateSync(ValidateSyncOptions{ name: "/test/bob/data2", signer: bobSigner, From 8e83d318a639b3a558e382a72fa5b4e033ee40e1 Mon Sep 17 00:00:00 2001 From: Harsh Kashyap Date: Sat, 4 Jul 2026 13:24:38 +0530 Subject: [PATCH 07/10] Remove AI-generated test comment labels --- std/security/certificate_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/std/security/certificate_test.go b/std/security/certificate_test.go index 1fe7929d..9002e244 100644 --- a/std/security/certificate_test.go +++ b/std/security/certificate_test.go @@ -15,7 +15,6 @@ import ( "github.com/stretchr/testify/require" ) -// (AI GENERATED DESCRIPTION): Tests that the `SignCert` function correctly returns an error when invoked with a nil signer and nil data. func TestSignCertInvalid(t *testing.T) { tu.SetT(t) @@ -27,7 +26,6 @@ func TestSignCertInvalid(t *testing.T) { require.Error(t, err) } -// (AI GENERATED DESCRIPTION): Verifies that a key can self‑sign its own certificate, checking the certificate name, public‑key content, validity period, and Ed25519 signature correctness. func TestSignCertSelf(t *testing.T) { tu.SetT(t) @@ -75,7 +73,6 @@ func TestSignCertSelf(t *testing.T) { require.True(t, tu.NoErr(signer.ValidateData(cert, certSigCov, cert))) } -// (AI GENERATED DESCRIPTION): Verifies that Alice’s signer can correctly sign a root certificate, producing a certificate with the expected name, content, signature format, validity period, and that the signature verifies against Alice’s own certificate. func TestSignCertOther(t *testing.T) { tu.SetT(t) From af39a3b5d55a9b4ee4c640446217273a4bce44c6 Mon Sep 17 00:00:00 2001 From: Harsh Kashyap Date: Sat, 4 Jul 2026 13:25:48 +0530 Subject: [PATCH 08/10] Strip AI prefix from test comments, keep descriptions --- std/security/certificate_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/std/security/certificate_test.go b/std/security/certificate_test.go index 9002e244..042052e2 100644 --- a/std/security/certificate_test.go +++ b/std/security/certificate_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/require" ) +// Tests that SignCert returns an error when invoked with a nil signer and nil data. func TestSignCertInvalid(t *testing.T) { tu.SetT(t) @@ -26,6 +27,7 @@ func TestSignCertInvalid(t *testing.T) { require.Error(t, err) } +// Verifies that a key can self-sign its own certificate, checking name, content, validity, and signature. func TestSignCertSelf(t *testing.T) { tu.SetT(t) @@ -73,6 +75,7 @@ func TestSignCertSelf(t *testing.T) { require.True(t, tu.NoErr(signer.ValidateData(cert, certSigCov, cert))) } +// Verifies that Alice's signer can sign a root certificate and that the signature verifies. func TestSignCertOther(t *testing.T) { tu.SetT(t) From 9317f3412f65ceeafd98cd4ed6e37b64d34683ec Mon Sep 17 00:00:00 2001 From: Harsh Kashyap Date: Sun, 5 Jul 2026 00:46:19 +0530 Subject: [PATCH 09/10] Restore test comments dropped in cleanup --- std/security/certificate.go | 1 + std/security/certificate_test.go | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/std/security/certificate.go b/std/security/certificate.go index 5f7f4b24..69746e94 100644 --- a/std/security/certificate.go +++ b/std/security/certificate.go @@ -161,6 +161,7 @@ func RevokeCert(args RevokeCertArgs) (enc.Wire, error) { record.NotBefore = optional.Some(uint64(nb.UnixMilli())) } + // Create revocation record data cfg := &ndn.DataConfig{ ContentType: optional.Some(ndn.ContentTypeKey), Freshness: optional.Some(args.Freshness.GetOr(defaultRevocationFreshness)), diff --git a/std/security/certificate_test.go b/std/security/certificate_test.go index 042052e2..1fe7929d 100644 --- a/std/security/certificate_test.go +++ b/std/security/certificate_test.go @@ -15,7 +15,7 @@ import ( "github.com/stretchr/testify/require" ) -// Tests that SignCert returns an error when invoked with a nil signer and nil data. +// (AI GENERATED DESCRIPTION): Tests that the `SignCert` function correctly returns an error when invoked with a nil signer and nil data. func TestSignCertInvalid(t *testing.T) { tu.SetT(t) @@ -27,7 +27,7 @@ func TestSignCertInvalid(t *testing.T) { require.Error(t, err) } -// Verifies that a key can self-sign its own certificate, checking name, content, validity, and signature. +// (AI GENERATED DESCRIPTION): Verifies that a key can self‑sign its own certificate, checking the certificate name, public‑key content, validity period, and Ed25519 signature correctness. func TestSignCertSelf(t *testing.T) { tu.SetT(t) @@ -75,7 +75,7 @@ func TestSignCertSelf(t *testing.T) { require.True(t, tu.NoErr(signer.ValidateData(cert, certSigCov, cert))) } -// Verifies that Alice's signer can sign a root certificate and that the signature verifies. +// (AI GENERATED DESCRIPTION): Verifies that Alice’s signer can correctly sign a root certificate, producing a certificate with the expected name, content, signature format, validity period, and that the signature verifies against Alice’s own certificate. func TestSignCertOther(t *testing.T) { tu.SetT(t) From 7a5136b1010d3eef2d055c3ee617d50d008d72f0 Mon Sep 17 00:00:00 2001 From: Harsh Kashyap Date: Mon, 6 Jul 2026 23:57:06 +0530 Subject: [PATCH 10/10] address PR review: revocation reason enum, notBefore bypass, sigtime --- std/ndn/spec.go | 3 ++ std/ndn/spec_2022/spec.go | 9 +++- std/security/certificate.go | 29 +++++++++-- std/security/certificate_test.go | 3 -- std/security/trust_config.go | 42 ++++++++++++++-- std/security/trust_config_test.go | 82 +++++++++++++++++++++++++++++++ 6 files changed, 156 insertions(+), 12 deletions(-) diff --git a/std/ndn/spec.go b/std/ndn/spec.go index 59e32a46..80eac647 100644 --- a/std/ndn/spec.go +++ b/std/ndn/spec.go @@ -97,6 +97,9 @@ type DataConfig struct { SigNotBefore optional.Optional[time.Time] SigNotAfter optional.Optional[time.Time] + // Signed Data signature timestamp (added with #186). + SigTime optional.Optional[time.Duration] + // Cross Schema attachment CrossSchema enc.Wire } diff --git a/std/ndn/spec_2022/spec.go b/std/ndn/spec_2022/spec.go index b245705f..a985a4fc 100644 --- a/std/ndn/spec_2022/spec.go +++ b/std/ndn/spec_2022/spec.go @@ -51,8 +51,11 @@ func (d *Data) SigNonce() []byte { return nil } -// (AI GENERATED DESCRIPTION): Returns the signature timestamp of the Data packet (currently unimplemented and returns nil). +// SigTime returns the signature timestamp of the Data packet, or nil if not set. func (d *Data) SigTime() *time.Time { + if d.SignatureInfo != nil && d.SignatureInfo.SignatureTime.IsSet() { + return utils.IdPtr(time.UnixMilli(d.SignatureInfo.SignatureTime.Unwrap().Milliseconds())) + } return nil } @@ -295,6 +298,10 @@ func (Spec) MakeData(name enc.Name, config *ndn.DataConfig, content enc.Wire, si data.SignatureInfo.KeyLocator = &KeyLocator{Name: key} } + if config.SigTime.IsSet() { + data.SignatureInfo.SignatureTime = config.SigTime + } + if config.SigNotBefore.IsSet() && config.SigNotAfter.IsSet() { data.SignatureInfo.ValidityPeriod = &ValidityPeriod{ NotBefore: config.SigNotBefore.Unwrap().UTC().Format(TimeFmt), diff --git a/std/security/certificate.go b/std/security/certificate.go index 69746e94..1aa56830 100644 --- a/std/security/certificate.go +++ b/std/security/certificate.go @@ -14,9 +14,23 @@ import ( "github.com/named-data/ndnd/std/types/optional" ) -type revocationReason uint64 - -const revocationReasonUnspecified revocationReason = 0 +// RevocationReason is the reason code for a revocation record, +// per RFC 5280 / UCLA-IRL/ndnrevoke. +type RevocationReason uint64 + +// Revocation reason codes from RFC 5280 / UCLA-IRL/ndnrevoke. +// RFC 5280 slots 7 (removeFromCRL) and 8 are intentionally unused. +const ( + RevocationReasonUnspecified RevocationReason = 0 + RevocationReasonKeyCompromise RevocationReason = 1 + RevocationReasonCACompromise RevocationReason = 2 + RevocationReasonAffiliationChanged RevocationReason = 3 + RevocationReasonSuperseded RevocationReason = 4 + RevocationReasonCessationOfOperation RevocationReason = 5 + RevocationReasonCertificateHold RevocationReason = 6 + RevocationReasonPrivilegeWithdrawn RevocationReason = 9 + RevocationReasonAACompromise RevocationReason = 10 +) const defaultRevocationFreshness = 8760 * time.Hour @@ -27,7 +41,7 @@ type RevokeCertArgs struct { // Signer signs the revocation record Data packet. May be nil for unsigned records. Signer ndn.Signer // Reason is the revocation reason code. - Reason revocationReason + Reason RevocationReason // Timestamp is the revocation timestamp. Defaults to now. Timestamp optional.Optional[time.Time] // NotBefore marks data produced before this timestamp as still valid. @@ -52,6 +66,9 @@ type SignCertArgs struct { Description map[string]string // CrossSchema to attach to the certificate. CrossSchema enc.Wire + // SigTime overrides the signature timestamp applied to the certificate. + // When unset, the certificate is signed at the current time. + SigTime optional.Optional[time.Time] } // SignCert signs a new NDN certificate with the given signer. @@ -91,6 +108,10 @@ func SignCert(args SignCertArgs) (enc.Wire, error) { SigNotAfter: optional.Some(args.NotAfter), CrossSchema: args.CrossSchema, } + if args.SigTime.IsSet() { + t := args.SigTime.Unwrap() + cfg.SigTime = optional.Some(time.Duration(t.UnixMilli()) * time.Millisecond) + } signer := sig.AsContextSigner(args.Signer) cert, err := spec.Spec{}.MakeData(certName, cfg, enc.Wire{pk}, signer) diff --git a/std/security/certificate_test.go b/std/security/certificate_test.go index 1fe7929d..9002e244 100644 --- a/std/security/certificate_test.go +++ b/std/security/certificate_test.go @@ -15,7 +15,6 @@ import ( "github.com/stretchr/testify/require" ) -// (AI GENERATED DESCRIPTION): Tests that the `SignCert` function correctly returns an error when invoked with a nil signer and nil data. func TestSignCertInvalid(t *testing.T) { tu.SetT(t) @@ -27,7 +26,6 @@ func TestSignCertInvalid(t *testing.T) { require.Error(t, err) } -// (AI GENERATED DESCRIPTION): Verifies that a key can self‑sign its own certificate, checking the certificate name, public‑key content, validity period, and Ed25519 signature correctness. func TestSignCertSelf(t *testing.T) { tu.SetT(t) @@ -75,7 +73,6 @@ func TestSignCertSelf(t *testing.T) { require.True(t, tu.NoErr(signer.ValidateData(cert, certSigCov, cert))) } -// (AI GENERATED DESCRIPTION): Verifies that Alice’s signer can correctly sign a root certificate, producing a certificate with the expected name, content, signature format, validity period, and that the signature verifies against Alice’s own certificate. func TestSignCertOther(t *testing.T) { tu.SetT(t) diff --git a/std/security/trust_config.go b/std/security/trust_config.go index 0da51cd7..dfaa22f1 100644 --- a/std/security/trust_config.go +++ b/std/security/trust_config.go @@ -8,6 +8,7 @@ import ( "github.com/named-data/ndnd/std/log" "github.com/named-data/ndnd/std/ndn" spec "github.com/named-data/ndnd/std/ndn/spec_2022" + revocationtlv "github.com/named-data/ndnd/std/security/revocation_tlv" "github.com/named-data/ndnd/std/security/signer" "github.com/named-data/ndnd/std/security/trust_schema" "github.com/named-data/ndnd/std/types/optional" @@ -552,11 +553,44 @@ func (tc *TrustConfig) rejectIfRevokedCert(cert ndn.Data, callback func(bool, er } wire, _ := tc.keychain.Store().Get(recordName, false) - if len(wire) > 0 { - callback(false, fmt.Errorf("certificate is revoked: %s", cert.Name())) - return true + if len(wire) == 0 { + return false } - return false + // A cert signed before the record's notBefore was issued before the + // revocation took effect, so we accept it as still valid. + if preRevocationCert(cert, enc.Wire{wire}) { + return false + } + callback(false, fmt.Errorf("certificate is revoked: %s", cert.Name())) + return true +} + +// preRevocationCert returns true if the cert was signed before the +// installed record's NotBefore timestamp, in which case the cert predates +// the revocation and is still valid. +func preRevocationCert(cert ndn.Data, recordWire enc.Wire) bool { + sig := cert.Signature() + if sig == nil { + return false + } + sigTime := sig.SigTime() + if sigTime == nil { + return false + } + + data, _, err := spec.Spec{}.ReadData(enc.NewWireView(recordWire)) + if err != nil { + return false + } + record, err := revocationtlv.ParseRevocationRecord(enc.NewWireView(data.Content()), false) + if err != nil || record == nil { + return false + } + nb, ok := record.NotBefore.Get() + if !ok { + return false + } + return sigTime.UnixMilli() < int64(nb) } func (tc *TrustConfig) isTrustedAnchorKey(keyLocator enc.Name) bool { diff --git a/std/security/trust_config_test.go b/std/security/trust_config_test.go index 7dc61472..25044542 100644 --- a/std/security/trust_config_test.go +++ b/std/security/trust_config_test.go @@ -14,6 +14,7 @@ import ( "github.com/named-data/ndnd/std/object/storage" sec "github.com/named-data/ndnd/std/security" "github.com/named-data/ndnd/std/security/keychain" + revocationtlv "github.com/named-data/ndnd/std/security/revocation_tlv" "github.com/named-data/ndnd/std/security/signer" "github.com/named-data/ndnd/std/security/trust_schema" "github.com/named-data/ndnd/std/types/optional" @@ -1122,3 +1123,84 @@ func TestTrustConfigRevocation(t *testing.T) { })) require.Equal(t, 1, tcTestFetchCount) } + +func TestTrustConfigRevocationNotBefore(t *testing.T) { + tu.SetT(t) + + clear(tcTestNetwork) + tcTestT = t + store := storage.NewMemoryStore() + keychainLocal := keychain.NewKeyChainMem(store) + tcTestKeyChain = keychainLocal + + opts := SignCertOptions{ + NotBefore: time.Now(), + NotAfter: time.Now().Add(2 * time.Hour), + } + + rootSigner, _ := signer.KeygenEd25519(sec.MakeKeyName(sname("/test"))) + rootCertWire, rootCertData, _ := signCert(rootSigner, tu.NoErr(signer.MarshalSecret(rootSigner)), opts) + require.NoError(t, keychainLocal.InsertCert(rootCertWire.Join())) + + daveSigner, _ := signer.KeygenEd25519(sec.MakeKeyName(sname("/test/dave"))) + daveSec, _ := tu.NoErr(signer.MarshalSecret(daveSigner)), error(nil) + daveCertWire, daveCertData, _ := signCertWithSigTime(rootSigner, daveSec, opts, + time.Now().Add(-2*time.Hour)) + require.NoError(t, keychainLocal.InsertCert(daveCertWire.Join())) + require.NoError(t, keychainLocal.InsertKey(daveSigner)) + + schema, err := trust_schema.NewLvsSchema(TRUST_CONFIG_INTRA_LVS) + require.NoError(t, err) + + trust, err := sec.NewTrustConfig(keychainLocal, schema, []enc.Name{rootCertData.Name()}) + require.NoError(t, err) + tcTestTrustConfig = trust + + // Issue a revocation record with NotBefore set to the future. A cert whose + // signature timestamp is older than NotBefore must still validate. + recordWire := tu.NoErr(sec.RevokeCert(sec.RevokeCertArgs{ + Cert: daveCertData, + Signer: rootSigner, + Reason: sec.RevocationReasonKeyCompromise, + NotBefore: optional.Some(time.Now().Add(time.Hour)), + })) + require.NoError(t, trust.InsertRevoke(recordWire)) + + // Sanity-check: record wire has the chosen reason. + recordData, _, err := spec.Spec{}.ReadData(enc.NewWireView(recordWire)) + require.NoError(t, err) + record, err := revocationtlv.ParseRevocationRecord(enc.NewWireView(recordData.Content()), false) + require.NoError(t, err) + require.Equal(t, uint64(1), record.Reason) + + // Validation passes because the cert's signature timestamp pre-dates + // the record's NotBefore. + network := tcTestNetwork + network[daveCertData.Name().String()] = daveCertWire + require.True(t, validateSync(ValidateSyncOptions{ + name: "/test/dave/data1", + signer: daveSigner, + })) +} + +// signCertWithSigTime mirrors signCert but pins the new cert's signature +// timestamp via sec.SignCert's SigTime option. +func signCertWithSigTime(s ndn.Signer, wire enc.Wire, opts SignCertOptions, sigTime time.Time) (enc.Wire, ndn.Data, enc.Wire) { + data, _, _ := spec.Spec{}.ReadData(enc.NewWireView(wire)) + cert, err := sec.SignCert(sec.SignCertArgs{ + Signer: s, + Data: data, + IssuerId: enc.NewGenericComponent("ndn"), + NotBefore: opts.NotBefore, + NotAfter: opts.NotAfter, + SigTime: optional.Some(sigTime), + }) + if err != nil { + panic(err) + } + certData, sigCov, err := spec.Spec{}.ReadData(enc.NewWireView(cert)) + if err != nil { + panic(err) + } + return cert, certData, sigCov +}