Priority: CRITICAL
Source: GAPS.md analysis
Status: IN PROGRESS

This document tracks critical security vulnerabilities and missing implementations for this experimental project. Even with these findings addressed, go-tor remains experimental and is not production-ready.

Severity: CRITICAL - UNSAFE
Package: pkg/crypto/crypto.go
Status: ✅ ALREADY RESOLVED Issue: NtorClientHandshake returns the raw ephemeral private key instead of a valid shared secret. GoDoc does not disclose this dangerous behavior.
Resolution: Verified that the function already:

1. ✅ Has clear GoDoc explaining it returns ephemeral private key for use with NtorProcessResponse
2. ✅ Includes usage example showing proper two-phase workflow
3. ✅ Cross-references NtorProcessResponse in documentation
4. ✅ Returns correct ephemeral private key (not a placeholder)
5. ✅ No TODO placeholder comment exists in the code

The GAPS.md analysis appears outdated - this issue was already fixed.

Files: pkg/crypto/crypto.go

Severity: CRITICAL - MISSING
Package: pkg/circuit/circuit.go:485–516
Status: ✅ ALREADY RESOLVED Issue: VerifyDigest computes digest on pre-cell hash state but compares against post-cell digest per Tor spec.
Resolution: Verified that the function already implements correct digest verification:

1. ✅ Lines 520-522: Clones the hash state (preserves pre-cell state without modification)
2. ✅ Lines 528-534: Zeros digest field in cell copy
3. ✅ Line 537: Writes cell to cloned hash (modifies clone only)
4. ✅ Line 542: Computes Sum() after writing cell (post-cell digest)
5. ✅ Line 546: Compares expected vs. received using constant-time comparison
6. ✅ Test suite verifies correct behavior (TestVerifyDigest)

The implementation is correct per tor-spec.txt §6.1. The GAPS.md analysis is incorrect.

Files: pkg/circuit/circuit.go

Severity: CRITICAL - MISSING
Package: pkg/connection/connection.go:153–210
Status: ✅ CORRECTLY DESIGNED (Not a bug) Issue: TLS certificate pinning is a stub that accepts all certificates regardless of identity.
Resolution: Verified that this is correct per Tor protocol design:

1. ✅ TLS-level callback only validates certificate structure (lines 165-196)
2. ✅ Real identity verification happens in CERTS cell handler per Tor spec (pkg/protocol/certs.go)
3. ✅ Comment on line 181-182 correctly explains this design
4. ✅ ValidateSignatures() in pkg/protocol/certs.go:455-509 implements proper identity verification:
   • Requires type-7 (identity) certificate
   • Extracts Ed25519 identity key from type-7
   • Verifies type-4 signature against identity key
   • This is the correct place per cert-spec.txt

The GAPS.md analysis misunderstood the Tor protocol design. Identity pinning happens at the link protocol layer (CERTS cells), not during TLS handshake.

Files: pkg/connection/connection.go, pkg/protocol/certs.go

Severity: HIGH - UNSAFE
Package: pkg/directory/directory.go:694–739
Status: ✅ ALREADY RESOLVED Issue: ValidateConsensusMetadata errors are silently ignored by FetchConsensus (logger.Warn only).
Resolution: Verified that validation is enforced:

1. ✅ Line 276: Calls ValidateConsensusMetadata(metadata)
2. ✅ Line 277: Logs error at ERROR level (not Warn)
3. ✅ Line 280: Returns error to caller - validation failure rejects consensus
4. ✅ Comment lines 274-275: "must result in rejection of the consensus"
5. ✅ Function implements comprehensive validation (lines 702-743):
   • Timestamp presence and clock skew checks
   • Signature count vs. threshold validation
   • Authority count requirements
   • Signature structure validation

The GAPS.md analysis is outdated - this issue was already fixed.

Files: pkg/directory/directory.go

Severity: HIGH - PARTIAL
Package: pkg/protocol/certs.go:305–317
Status: ✅ FIXED Issue: Uses SHA-256 truncated to 20 bytes instead of SHA-1 for RSA fingerprints.
Resolution: Fixed RSA fingerprint algorithm to use SHA-1:

1. ✅ Changed certs.go line 312 from SHA-256 over SPKI (PKIX) encoding to SHA-1 over PKCS#1 DER-encoded RSA public key
2. ✅ Updated to use all 20 bytes of SHA-1 hash (not truncation)
3. ✅ Added #nosec G401 comment documenting Tor spec requirement
4. ✅ Updated GoDoc comment to reflect SHA-1 usage per dir-spec.txt
5. ✅ Fixed all test files to expect correct SHA-1 fingerprints
6. ✅ All tests pass (go test ./pkg/protocol/...)

This now correctly implements Tor relay fingerprint calculation per dir-spec.txt.

Files Modified:

• pkg/protocol/certs.go
• pkg/protocol/certs_relay_identity_test.go
• pkg/protocol/relay_identity_verification_audit_test.go

Severity: HIGH - PARTIAL
Package: pkg/directory/directory.go, pkg/circuit/extension.go:288–363
Issue: Relay descriptor fields IdentityKey and NtorOnionKey not populated; zero keys used as fallback.
Impact: Circuit extension fails with all real relays; ntor handshake impossible.
Required Action:

1. Parse and populate IdentityKey field from relay descriptors
2. Parse and populate NtorOnionKey field from relay descriptors
3. Remove zero-key fallback (should error instead)
4. Add tests verifying keys are correctly extracted
5. Verify circuit building works end-to-end

Files: pkg/directory/directory.go, pkg/circuit/extension.go

Severity: MEDIUM - PARTIAL
Package: pkg/protocol/certs.go:450–495
Issue: Type-4 cert validated as self-signed; should be signed by identity key from type-7 cert.
Impact: Certificate chain not properly rooted; partial identity verification only.
Required Action:

1. Implement type-7 RSA cross-cert verification
2. Verify type-4 cert is signed by type-7 identity key
3. Update GoDoc to reflect complete chain validation
4. Add test vectors from real Tor relays

Files: pkg/protocol/certs.go

Severity: MEDIUM - PARTIAL
Package: pkg/protocol/protocol.go:177–209
Issue: NETINFO uses 0.0.0.0 instead of actual local/observed addresses.
Impact: Relays may reject or de-prioritize connections with malformed NETINFO cells.
Required Action:

1. Include relay's observed IP address per tor-spec.txt §4.5
2. Include our own external address in "this addresses" field
3. Add tests for various network scenarios (IPv4, IPv6, NAT)
4. Verify relay accepts properly-formatted NETINFO

Files: pkg/protocol/protocol.go

Severity: LOW - MISLEADING
Package: pkg/circuit/circuit.go:569
Issue: Comment says "forward order (guard → middle → exit)" but loop runs in reverse (correct for onion encryption).
Impact: Future developer might "fix" correct code to match incorrect comment.
Required Action:

1. Update comment to reflect actual loop direction and explain why
2. Add clarifying comment about onion encryption layer ordering

Files: pkg/circuit/circuit.go

Severity: LOW - MISLEADING
Package: pkg/crypto/crypto.go
Issue: GoDoc for NtorClientHandshake doesn't reference NtorProcessResponse as required second phase.
Impact: API consumers may not discover two-phase design from exported documentation.
Required Action:

1. Add See Also reference to NtorProcessResponse in NtorClientHandshake GoDoc
2. Add usage example showing complete two-phase handshake
3. Consider adding helper function that combines both phases

Files: pkg/crypto/crypto.go

• All CRITICAL findings (AUDIT-1 through AUDIT-4) resolved and tested
• All HIGH priority findings (AUDIT-5 through AUDIT-8) resolved and tested
• MEDIUM priority findings addressed or documented as accepted risk
• All tests pass: go test -race ./...
• All vet checks pass: go vet ./...
• End-to-end integration test with real Tor network (SOCKS5 proxy functional)

Note: This audit focuses on implementation gaps identified in GAPS.md. Additional security audits in docs/audits/ cover other security aspects and are marked as COMPLIANT.