Opened 7 weeks ago

Closed 7 weeks ago

#23071 closed defect (fixed)

test_hs_ntor.sh fails with recent pysha3

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: python tests hs
Cc: Actual Points: .1
Parent ID: Points: .1
Reviewer: asn Sponsor: SponsorR

Description

Right now the recommended sha3 in python is pysha3, which emulates the same sha3 API as is added in python 3.6. But hs_ntor_ref.py doesn't work there.

Child Tickets

Change History (6)

comment:1 Changed 7 weeks ago by nickm

Actual Points: .1
Keywords: python tests hs added
Status: newneeds_review

Fix in my branch bug23071_031; please review?

comment:2 Changed 7 weeks ago by nickm

(I have tested this with pysha3 and with sha3.)

comment:3 Changed 7 weeks ago by asn

Reviewer: asn

comment:4 Changed 7 weeks ago by asn

Hmm, the code is now:

try:
    import sha3
except ImportError:
    # In python 3.6, the sha3 functions are in hashlib whether we
    # import sha3 or not.
    pass

try:
    # Pull the sha3 functions in.
    from hashlib import sha3_256, shake_256
    shake_squeeze = shake_256.digest
except ImportError:
    if hasattr(sha3, "SHA3256"):

I think there is a problem here: If the sha3 import fails, and the from hashlib... import fails, then the if hasattr(sha3, "SHA3256"): will also fail since sha3 is undefined. That's when we should return 77, right?

How about the following patch on top of your patch?

-    if hasattr(sha3, "SHA3256"):
+    if "sha3" in sys.modules and hasattr(sha3, "SHA3256"):

comment:5 Changed 7 weeks ago by asn

Status: needs_reviewneeds_revision

comment:6 Changed 7 weeks ago by nickm

Resolution: fixed
Status: needs_revisionclosed

Looks good, but maybe simpler to just do "sha3 = None" if the sha3 import fails. I've taken that approach and merged. Thanks!

Note: See TracTickets for help on using tickets.