Repository navigation
Add Go NDNCERT CA server and enhance status reporting - #235
Closed
markverick wants to merge 11 commits into
Closed
markverick wants to merge 11 commits into
markverick wants to merge 11 commits into
Conversation
Add NDNd status report and fix outbound face byte accounting
Add Go NDNCERT CA server and include system CA roots
Add Go NDNCERT CA server and validate CA roots
Collaborator
Author
|
Draft until I tested on challenges |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved authorization, malformed-input, replay, and synchronization issues compromise CA security and reliability.
7 open issues
1 file not reviewed
std/security/ndncert/tlv/zz_generated.go: Generated file
What changed in this PR
Adds an NDNCERT certificate authority to NDNd and expands forwarder status reporting.
Changes:
- Implements CA issuance, DNS/email challenges, configuration, and tests.
- Adds XML status reports and an HTTP HTML dashboard.
- Corrects outbound-byte metrics and includes container TLS trust certificates.
| File | Description |
|---|---|
| tools/sec/cmd_sec.go | Registers the CA command. |
| tools/sec/cert_server.go | Configures and runs the CA. |
| tools/nfdc/nfdc.go | Registers status commands. |
| tools/nfdc/nfdc_status_server.go | Serves the HTML dashboard. |
| tools/nfdc/nfdc_status_server_test.go | Tests stylesheet delivery. |
| tools/nfdc/nfdc_status_report.go | Collects and encodes status. |
| tools/nfdc/nfdc_status_report_test.go | Tests mappings and rendering. |
| tools/nfdc/nfdc_dataset.go | Adds single-packet status fetching. |
| tools/nfdc/ndnd-status.xsl | Styles XML reports. |
| tools/nfdc/ndnd-status.html | Defines the HTML dashboard. |
| std/security/signer/validate_test.go | Tests Interest signature validation. |
| std/security/signer/validate_data.go | Shares Data/Interest signature validation. |
| std/security/ndncert/tlv/zz_generated.go | Encodes the optional suffix limit. |
| std/security/ndncert/tlv/definitions.go | Defines the profile suffix limit. |
| std/security/ndncert/server/storage.go | Stores requests and removes expired entries. |
| std/security/ndncert/server/state.go | Tracks issuance and challenge state. |
| std/security/ndncert/server/config.go | Loads CA and SMTP configuration. |
| std/security/ndncert/server/config_test.go | Tests configuration and naming helpers. |
| std/security/ndncert/server/challenge_email.go | Implements email verification and SMTP delivery. |
| std/security/ndncert/server/challenge_email_test.go | Tests email challenge behavior. |
| std/security/ndncert/server/challenge_dns.go | Implements DNS TXT verification. |
| std/security/ndncert/server/challenge_dns_test.go | Exercises DNS challenge paths. |
| std/security/ndncert/server/ca.go | Implements CA protocol and issuance. |
| std/security/ndncert/server/ca_policy_test.go | Tests request policy and certificate storage. |
| std/examples/ndncert/ca.conf.sample | Provides sample CA configuration. |
| fw/mgmt/face.go | Corrects outbound-byte reporting. |
| docs/fw-control.md | Documents status commands. |
| Dockerfile | Includes TLS trust certificates. |
Review effort: Balanced
Comment on lines
+578
to
+580
| if state.Status == StatusSuccess { | ||
| fmt.Printf("INFO: challenge completed successfully, issuing cert\n") | ||
| cert, err := ca.issueCertificate(state) |
Comment on lines
+740
to
+741
| var aeadMsg ndncert.AeadMessage | ||
| aeadMsg.FromTLV(cipherMsg) |
Comment on lines
+743
to
+745
| plaintext, err := ndncert.AeadDecrypt(state.AesKey, aeadMsg, state.RequestID) | ||
| if err != nil { | ||
| return "", nil, fmt.Errorf("failed to decrypt: %w", err) |
| "ca-prefix": "/example", | ||
| "ca-info": "An example NDNCERT CA", | ||
| "max-validity-period": "1296000", | ||
| "supported-challenges": [ |
| } | ||
|
|
||
| chPrefix := base.Append(enc.NewGenericComponent("CHALLENGE")) | ||
| if err := ca.engine.AttachHandler(chPrefix, ca.onChallenge); err != nil { |
Comment on lines
+75
to
+86
| if err != nil { | ||
| return retry, "wrong-record", nil | ||
| } | ||
|
|
||
| for _, r := range records { | ||
| if strings.TrimSpace(strings.Trim(r, "\"")) == expected { | ||
| state.Status = StatusSuccess | ||
| return nil, "", nil | ||
| } | ||
| } | ||
|
|
||
| return retry, "wrong-record", nil |
| state.SetChallengeStateValue(emailCodeHashKey, hashEmailCode(salt, code)) | ||
| state.Status = StatusChallenge | ||
| state.UpdatedAt = time.Now() | ||
| state.ExpiresAt = state.UpdatedAt.Add(emailCodeLifetime) |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Go Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.
| Benchmark suite | Current: 2cb9dff | Previous: d37de74 | Ratio |
|---|---|---|---|
BenchmarkDataEncodeMedium |
652.7 ns/op 744 B/op 9 allocs/op |
326.3 ns/op 744 B/op 9 allocs/op |
2.00 |
BenchmarkDataEncodeMedium - ns/op |
652.7 ns/op |
326.3 ns/op |
2.00 |
BenchmarkDataEncodeMediumLongName |
1026 ns/op 1032 B/op 9 allocs/op |
426.8 ns/op 1032 B/op 9 allocs/op |
2.40 |
BenchmarkDataEncodeMediumLongName - ns/op |
1026 ns/op |
426.8 ns/op |
2.40 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @pulsejet
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a comprehensive implementation of NDNCERT server-side challenges for certificate issuance, including DNS-01 and email-based verification, along with extensive unit tests and configuration examples. It also corrects a metrics bug and updates documentation to reflect new features.
NDNCERT Challenge Implementation:
ChallengeDnsHandler) that verifies domain ownership by requiring a TXT record with a random token, including support for mock/testing and robust error handling.ChallengeEmailHandler) and SMTP sender, supporting secure code generation, delivery, and validation, with replay/attempt limits and expiration.ca.conf.sample).Testing and Quality Assurance:
Documentation:
ndnd fw status-reportandstatus-servercommands, clarifying their output and behavior.Bug Fix:
NOutByteswas incorrectly set to the value ofNInBytesinstead ofNOutBytes.Build and Security: