Max out S2K parameters
There were issues in trying to do CPU tuning (see #279 and #157 for examples). The simpler approach, given the capacity of modern hardware and the antiquity of OpenPGP's S2K parameters, is just to always use the maximum S2K settings. This also saves us some CPU from running calibration steps. This commit also drops the test for #157 entirely, because that test embeds assumptions about the way that calibration is done that are no longer true. If this is merged, we should close #279 without merging it. Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
This commit is contained in:
@@ -342,7 +342,7 @@ class HashAlgorithm(IntEnum):
|
||||
|
||||
def __init__(self, *args):
|
||||
super(self.__class__, self).__init__()
|
||||
self._tuned_count = 0
|
||||
self._tuned_count = 255
|
||||
|
||||
@property
|
||||
def hasher(self):
|
||||
@@ -354,41 +354,12 @@ class HashAlgorithm(IntEnum):
|
||||
|
||||
@property
|
||||
def tuned_count(self):
|
||||
if self._tuned_count == 0:
|
||||
self.tune_count()
|
||||
|
||||
return self._tuned_count
|
||||
|
||||
@property
|
||||
def is_supported(self):
|
||||
return True
|
||||
|
||||
def tune_count(self):
|
||||
start = end = 0
|
||||
htd = _hashtunedata[:]
|
||||
|
||||
while start == end:
|
||||
# potentially do this multiple times in case the resolution of time.time is low enough that
|
||||
# hashing 100 KiB isn't enough time to produce a measurable difference
|
||||
# (e.g. if the timer for time.time doesn't have enough precision)
|
||||
htd = htd + htd
|
||||
h = self.hasher
|
||||
|
||||
start = time.time()
|
||||
h.update(htd)
|
||||
end = time.time()
|
||||
|
||||
# now calculate how many bytes need to be hashed to reach our expected time period
|
||||
# GnuPG tunes for about 100ms, so we'll do that as well
|
||||
_TIME = 0.100
|
||||
ct = int(len(htd) * (_TIME / (end - start)))
|
||||
c1 = ((ct >> (ct.bit_length() - 5)) - 16)
|
||||
c2 = (ct.bit_length() - 11)
|
||||
c = ((c2 << 4) + c1)
|
||||
|
||||
# constrain self._tuned_count to be between 0 and 255
|
||||
self._tuned_count = max(min(c, 255), 0)
|
||||
|
||||
|
||||
class RevocationReason(IntEnum):
|
||||
#: No reason was specified. This is the default reason.
|
||||
|
||||
@@ -164,41 +164,6 @@ def test_reg_bug_56():
|
||||
assert vres
|
||||
|
||||
|
||||
@pytest.mark.regression(issue=157)
|
||||
def test_reg_bug_157(monkeypatch):
|
||||
# local imports for this
|
||||
import pgpy.constants
|
||||
from pgpy.packet.fields import String2Key
|
||||
from time import time as rtime
|
||||
|
||||
# to more easily replicate this bug, hash only 8 bytes instead of 100 KiB
|
||||
monkeypatch.setattr('pgpy.constants._hashtunedata', bytearray([10, 11, 12, 13, 14, 15, 16, 17]))
|
||||
# also monkeypatch time.time to return fewer significant digits
|
||||
monkeypatch.setattr('time.time', lambda: round(rtime(), 3))
|
||||
assert len(pgpy.constants._hashtunedata) == 8
|
||||
|
||||
pgpy.constants.HashAlgorithm.SHA256.tune_count()
|
||||
assert pgpy.constants.HashAlgorithm.SHA256.tuned_count > 0
|
||||
|
||||
# now let's try it out and ensure that the count actually worked
|
||||
s2k = String2Key()
|
||||
s2k.encalg = pgpy.constants.SymmetricKeyAlgorithm.AES256
|
||||
s2k.specifier = pgpy.constants.String2KeyType.Iterated
|
||||
s2k.halg = pgpy.constants.HashAlgorithm.SHA256
|
||||
s2k.count = pgpy.constants.HashAlgorithm.SHA256.tuned_count
|
||||
|
||||
start = rtime()
|
||||
sk = s2k.derive_key('sooper_sekret_passphrase')
|
||||
elapsed = rtime() - start
|
||||
|
||||
# check that we're actually close to our target
|
||||
assert len(sk) == 32
|
||||
try:
|
||||
assert 0.1 <= round(elapsed, 1) <= 0.2
|
||||
|
||||
except AssertionError:
|
||||
warnings.warn("tuned_count: {}; elapsed time: {:.5f}".format(pgpy.constants.HashAlgorithm.SHA256.tuned_count, elapsed))
|
||||
|
||||
|
||||
# load mixed keys separately so they do not overwrite "single algo" keys in the _seckeys mapping
|
||||
_seckeys = {sk.key_algorithm.name: sk for sk in (PGPKey.from_file(f)[0] for f in sorted(glob.glob('tests/testdata/keys/*.sec.asc')) if 'keys/mixed' not in f)}
|
||||
|
||||
Reference in New Issue
Block a user