Opened 3 months ago

Closed 3 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 3 months ago by nickm

  • Owner set to nickm
  • Status changed from new to accepted

comment:2 Changed 3 months ago by nickm

  • Actual Points set to .3
  • Status changed from accepted to needs_review

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

comment:3 Changed 3 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 3 months ago by ahf

  • Status changed from needs_review to needs_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 3 months ago by ahf

  • Cc ahf added
  • Reviewer set to ahf

comment:6 Changed 3 months ago by nickm

  • Resolution set to implemented
  • Status changed from needs_revision to closed

fixed and merged; thanks!

Note: See TracTickets for help on using tickets.