#26818 closed defect (implemented)

Use NSS for RSA

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 035-roadmap-subticket, 035-triaged-in-20180711
Cc: nickm, gk, catalyst, ahf Actual Points:
Parent ID: #26631 Points:
Reviewer: asn Sponsor: Sponsor8-can

Description

RSA will be the last major cryptographic algorithm for us to use NSS for. As with DH, we'll need a way to convert to and from OpenSSL's RSA keys, until we have our TLS layer converted.

Child Tickets

Change History (15)

comment:1 Changed 12 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:2 Changed 12 months ago by nickm

Status: acceptedneeds_review

See branch nss_rsa; it's based on the nss_dh_squashed_merged branch of #26817 . PR at https://github.com/torproject/tor/pull/259 , but most of it also in the earlier branch.

comment:3 Changed 12 months ago by asn

Reviewer: catalyst

comment:4 Changed 11 months ago by nickm

I just rebased this branch onto the latest nss_ds_squashed_merged, to make it more testable.

comment:5 Changed 11 months ago by nickm

Status: needs_reviewneeds_revision

comment:6 Changed 11 months ago by nickm

Status: needs_revisionneeds_review

Okay, rebased again. The tests now pass for me now with openssl and nss.

comment:7 Changed 11 months ago by dgoulet

Reviewer: catalystasn

comment:8 Changed 11 months ago by asn

Status: needs_reviewneeds_revision

Did initial review here. Lots of comments are nits and can be ignored (e.g. the code dup ones).

Marking as needs_rev because some stuff could be improved.

comment:9 Changed 11 months ago by nickm

I agree with your suggestions. If it's okay with you, I'd like to fix these post-merge however, opening new tickets for them. Is that okay with you?

comment:10 Changed 11 months ago by nickm

(i've started working on these issues as a branch 'nss_tls_fixes' on top of 'nss_tls')

comment:11 Changed 11 months ago by nickm

Okay, I've made changes in nss_tls_fixes, and tried to answer on the review

comment:12 Changed 11 months ago by asn

Seems like only minor stuff remain on GH review wrt crypto_pk_check_key() and family. After these are resolved/discussed we can merge_ready this.

comment:13 Changed 11 months ago by nickm

I've added a couple more commits in nss_tls_fixes -- better now?

comment:14 Changed 11 months ago by asn

Status: needs_revisionmerge_ready

Yep. LGTM on my part!

comment:15 Changed 11 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

merged

Note: See TracTickets for help on using tickets.