Merge ~jbfzs/dkimpy:content_debugging into dkimpy:master

[Jonathan Bastien-Filiatrault]

[Scott Kitterman (kitterman)]

Please add an @param for class DomainSigner for the new debug parameter. Also, it'd be nice for you to include the ChangeLog entry.

Do dkimsign, dkimverify, etc. still work with this change? I believe they set the logger, so they may need updating. They aren't tested by the test suite, so passing tests isn't enough.

Thanks,

Scott K

review: Needs Fixing

[Jonathan Bastien-Filiatrault (jbfzs)]

dkimverify:
  dkim.DKIM(message, logger=logging)

dkimsign:
  dkim.DKIM(message,logger=logger, signature_algorithm=args.signalg)

Both specify arguments explicitly and work as expected. In my experience, appending a parameter with a default value is quite safe from an API breaking point of view.

I added a changelog entry and documented the parameter as requested.

review: Needs Resubmitting

[Scott Kitterman (kitterman)]

Merged. Thanks.

review: Approve

diff --git a/ChangeLog b/ChangeLog
index ffaa3dc..2176499 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,6 +4,9 @@ UNRELEASED Version 0.9.2
       Bastien-Filiatrault for the change)
     - Refactor header folding for more consistent results, including reduced
       stray whitespace (Also Jonathan Bastien-Filiatrault)
+    - Don't log message headers and body unless explicitely requested. This
+      should also reduce memory usage on large messages. (Jonathan
+      Bastien-Filiatrault).
 2018-12-09 Version 0.9.1
     - Fixed ARC verification to fail if h= tag is present in Arc-Seal and
       added tests
diff --git a/dkim/__init__.py b/dkim/__init__.py
index a8d3bbe..5a1708a 100644
--- a/dkim/__init__.py
+++ b/dkim/__init__.py
@@ -112,13 +112,15 @@ CV_None = b'none'
 
 
 class HashThrough(object):
-  def __init__(self, hasher):
+  def __init__(self, hasher, debug=False):
     self.data = []
     self.hasher = hasher
     self.name = hasher.name
+    self.debug = debug
 
   def update(self, data):
-    self.data.append(data)
+    if self.debug:
+        self.data.append(data)
     return self.hasher.update(data)
 
   def digest(self):
@@ -460,12 +462,14 @@ class DomainSigner(object):
   #: (with either \\n or \\r\\n line endings)
   #: @param logger: a logger to which debug info will be written (default None)
   #: @param signature_algorithm: the signing algorithm to use when signing
+  #: @param debug_content: log headers and body after canonicalization (default False)
   def __init__(self,message=None,logger=None,signature_algorithm=b'rsa-sha256',
-        minkey=1024):
+        minkey=1024, debug_content=False):
     self.set_message(message)
     if logger is None:
         logger = get_default_logger()
     self.logger = logger
+    self.debug_content = debug_content and logger.isEnabledFor(logging.DEBUG)
     if signature_algorithm not in HASH_ALGORITHMS:
         raise ParameterError(
             "Unsupported signature algorithm: "+signature_algorithm)
@@ -611,13 +615,14 @@ class DomainSigner(object):
       header_value = fold(header_value, namelen=len(header_name))
     header_value = RE_BTAG.sub(b'\\1',header_value)
     header = (header_name, b' ' + header_value)
-    h = HashThrough(self.hasher())
+    h = HashThrough(self.hasher(), self.debug_content)
     sig = dict(fields)
 
     headers = canon_policy.canonicalize_headers(self.headers)
     self.signed_headers = hash_headers(
         h, canon_policy, headers, include_headers, header, sig)
-    self.logger.debug("sign %s headers: %r" % (header_name, h.hashed()))
+    if self.debug_content:
+        self.logger.debug("sign %s headers: %r" % (header_name, h.hashed()))
 
     if self.signature_algorithm == b'rsa-sha256' or self.signature_algorithm == b'rsa-sha1':
         try:
@@ -663,13 +668,14 @@ class DomainSigner(object):
 
     # validate body if present
     if b'bh' in sig:
-      h = HashThrough(hasher())
+      h = HashThrough(hasher(), self.debug_content)
 
       body = canon_policy.canonicalize_body(self.body)
       if b'l' in sig:
         body = body[:int(sig[b'l'])]
       h.update(body)
-      self.logger.debug("body hashed: %r" % h.hashed())
+      if self.debug_content:
+          self.logger.debug("body hashed: %r" % h.hashed())
       bodyhash = h.digest()
 
       self.logger.debug("bh: %s" % base64.b64encode(bodyhash))
@@ -688,12 +694,13 @@ class DomainSigner(object):
     # generalized to check for extras of other singleton headers.
     if b'from' in include_headers:
       include_headers.append(b'from')
-    h = HashThrough(hasher())
+    h = HashThrough(hasher(), self.debug_content)
 
     headers = canon_policy.canonicalize_headers(self.headers)
     self.signed_headers = hash_headers(
         h, canon_policy, headers, include_headers, sig_header, sig)
-    self.logger.debug("signed for %s: %r" % (sig_header[0], h.hashed()))
+    if self.debug_content:
+        self.logger.debug("signed for %s: %r" % (sig_header[0], h.hashed()))
     signature = base64.b64decode(re.sub(br"\s+", b"", sig[b'b']))
     if ktag == b'rsa':
         try:
@@ -1009,9 +1016,10 @@ class ARC(DomainSigner):
     canon_policy = CanonicalizationPolicy.from_c_value(b'relaxed/relaxed')
 
     self.hasher = HASH_ALGORITHMS[self.signature_algorithm]
-    h = HashThrough(self.hasher())
+    h = HashThrough(self.hasher(), self.debug_content)
     h.update(canon_policy.canonicalize_body(self.body))
-    self.logger.debug("sign ams body hashed: %r" % h.hashed())
+    if self.debug_content:
+        self.logger.debug("sign ams body hashed: %r" % h.hashed())
     bodyhash = base64.b64encode(h.digest())
 
     # Compute ARC-Message-Signature