Opened 8 months ago

Closed 8 months ago

#21841 closed enhancement (implemented)

Refactor: include openssl headers from fewer modules

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: ahf Actual Points: .3
Parent ID: Points: .3
Reviewer: ahf Sponsor:

Description

When possible, we should make it so that no part of our code other than crypto*.c and tortls*.c actually depend on openssl APIs.

Child Tickets

Change History (6)

comment:1 Changed 8 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:2 Changed 8 months ago by nickm

Actual Points: .3
Status: acceptedneeds_review

See branch 'isolate_openssl' in my public repo. I think it improves our code.

comment:3 Changed 8 months ago by nickm

(Note that there are still openssl includes in some of the donna headers, but that they are no longer actually used. Also note that tor-gencert and tor-checkkey in src/tool/ still use openssl APIs directly. I'm not changing those right now.)

comment:4 Changed 8 months ago by ahf

Status: needs_reviewneeds_revision

In commit 38fb651f0d740bef575ad7f95475cc504ea4cb8f:

+    crypto_digest_add_bytes(sha_ctx_, (const char *)(inp2), (len2));	\
+    crypto_digest_get_digest(sha_ctx_, (char *)out, 64);		\
+    crypto_digest_free(sha_ctx_);

and in the same commit:

+    crypto_digest_add_bytes(sha_ctx_, (const char *)(inp3), (len3));	\
+    crypto_digest_get_digest(sha_ctx_, (char *)out, 64);		\
+    crypto_digest_free(sha_ctx_);					\

The 64 should probably be replaced with DIGEST512_LEN.

Other than that it looks good - seems to simplify our code.

comment:5 Changed 8 months ago by ahf

Cc: ahf added
Reviewer: ahf

comment:6 Changed 8 months ago by nickm

Resolution: implemented
Status: needs_revisionclosed

fixed and merged; thanks!

Note: See TracTickets for help on using tickets.