Add a script for formatting the VerifySignedData() test data.

net/data/verify_signed_data_unittest/annotate_test_data.py

+#!/usr/bin/python
+# Copyright (c) 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""This script is called without any arguments to re-format all of the *.pem
+files in the script's parent directory.
+
+The main formatting change is to run "openssl asn1parse" for each of the PEM
+block sections (except for DATA), and add that output to the comment.
+
+Refer to the README file for more information.
+"""
+
+import glob
+import os
+import re
+import base64
+import subprocess
+
+
+def Transform(file_data):
+  """Returns a transformed (formatted) version of file_data"""
+
+  result = ''
+
+  # Get the file's description (all the text before the first PEM block)
+  file_description = GetTextUntilNextPemBlock(file_data)
+
+  result += file_description + '\n'
+
+  for block in GetPemBlocks(file_data):
+    result += '\n\n\n'
+
+    result += MakePemBlockString(block.name, block.data)
+
+    # If there was a user comment (non-script-generated comment) associated
+    # with the block, output it immediately after the block.
+    user_comment = GetUserComment(block.comment)
+    if len(user_comment) != 0:

[mattm, 2015/07/28 00:51:15]

"if user_comment:"

[eroman, 2015/07/28 01:41:23]

Done.

+      result += '\n' + user_comment + '\n'
+
+    # For every block except for DATA, try to pretty print the parsed ASN.1.
+    # DATA blocks likely would be DER in practice, but for the purposes of
+    # these tests seeing its structure doesn't clarify
+    # anything and is just a distraction.
+    if block.name != 'DATA':
+      generated_comment = GenerateCommentForBlock(block.name, block.data)
+      result += '\n' + generated_comment + '\n'

[mattm, 2015/07/28 00:51:15]

So this is putting the text formatted version of the data after the pem-encoded
version? The normal openssl output puts the formatted version before the pem
data (eg, look net/data/ssl/certificates/*.pem)

[eroman, 2015/07/28 01:41:24]

Correct. Examples at https://codereview.chromium.org/1209283004/
I initially had the comments on top to match, but I ran into some ambiguity
issues since the file level description appears as a comment for the first block
(and I wanted to offset them differently). I solved this by simply putting block
comments after the PEM.

I am sure I can make the comments above approach work too (and update this code
to match), however I think it is simpler to read and parse in the bottom manner.
What do you think?

[mattm, 2015/07/28 04:22:14]

I guess it's not a big deal either way. Having it above would be nice for
consistency, but this is good enough, no need to spend more time on it.

+
+  return result
+
+
+def GenerateCommentForBlock(block_name, block_data):
+  """Returns a string describing the ASN.1 structure of block_data"""
+
+  p = subprocess.Popen(['openssl', 'asn1parse', '-i', '-inform', 'DER'],
+                       stdout=subprocess.PIPE, stdin=subprocess.PIPE,
+                       stderr=subprocess.PIPE)
+  stdout_data, stderr_data = p.communicate(input=block_data)
+  generated_comment = '$ openssl asn1parse -i < [%s]\n%s' % (block_name,
+                                                             stdout_data)
+  return generated_comment.strip('\n')
+
+
+def GetTextUntilNextPemBlock(text):
+  result = ''
+  end = text.find('-----BEGIN ')

[mattm, 2015/07/28 00:51:15]

slightly more pythonic: text.split('-----BEGIN ')[0].strip('\n') 

[eroman, 2015/07/28 01:41:23]

Done.

+  if end != -1:
+    result = text[0:end]
+  return result.strip('\n')
+
+
+def GetUserComment(comment):
+  """Removes any script-generated lines (everything after the $ openssl line)"""
+
+  # Consider everything after "$ openssl" to be a generated comment.
+  pos = comment.find('$ openssl asn1parse -i')

[mattm, 2015/07/28 00:51:15]

same as previous comment

[eroman, 2015/07/28 01:41:23]

Done.

+  if pos != -1:
+    comment = comment[0:pos]
+
+  comment = comment.strip('\n')
+  if IsEntirelyWhiteSpace(comment):
+    comment = ''
+
+  return comment
+
+
+def MakePemBlockString(name, data):
+  return ('-----BEGIN %s-----\n'
+          '%s'
+          '-----END %s-----\n') % (name, EncodeDataForPem(data), name)
+
+
+def GetFilePaths():

[mattm, 2015/07/28 00:51:15]

GetPemFilePaths

[eroman, 2015/07/28 01:41:24]

Done.

+  """Returns an iterable for all the paths to the PEM test files"""
+
+  base_dir = os.path.dirname(os.path.realpath(__file__))
+  for filename in glob.iglob('*.pem'):

[mattm, 2015/07/28 00:51:15]

glob includes the path in the results, so you could just do:
return glob.iglob(os.path.join(base_dir, '*.pem'))

[eroman, 2015/07/28 01:41:24]

Done, much better!

In fact I don't think my previous code made sense -- I was still depending on
the CWD even though I meant not to ;)

+    yield os.path.join(base_dir, filename)
+
+
+def ReadFileToString(path):
+  with open (path, 'r') as f:

[mattm, 2015/07/28 00:51:14]

no space before ()

[eroman, 2015/07/28 01:41:23]

Done.

+    return f.read()
+
+
+def WrapTextToLineWidth(text, column_width):
+  result = ''
+  pos = 0
+  while pos < len(text):
+    result += text[pos : pos + column_width] + '\n'
+    pos += column_width
+  return result
+
+
+def EncodeDataForPem(data):
+  result = base64.b64encode(data)
+  return WrapTextToLineWidth(result, 75)
+
+
+class PemBlock(object):
+  def __init__(self):
+    self.name = None
+    self.data = None
+    self.comment = None
+
+
+def StripAllWhitespace(text):
+  pattern = re.compile(r'\s+')
+  return re.sub(pattern, '', text)
+
+
+def IsEntirelyWhiteSpace(text):
+  return len(StripAllWhitespace(text)) == 0
+
+
+def DecodePemBlockData(text):
+  text = StripAllWhitespace(text)
+  return base64.b64decode(text)
+
+
+def GetPemBlocks(data):
+  """Returns an iterable of PemBlock"""
+
+  # Position in |data|
+  pos = 0
+
+  while pos < len(data):
+    block = PemBlock()
+
+    # Find the block header
+    header_pattern = re.compile('-----BEGIN ([\w ]+)-----')

[mattm, 2015/07/28 00:51:15]

pretty nitpicky for a little helper script, but.. calling re.compile inside a
loop is a bit odd.. I'd just use re.search or call re.compile outside the
function definition (with a leading underscore in the variable name).

[eroman, 2015/07/28 01:41:24]

Done, thanks!

+    m = header_pattern.search(data, pos)
+    if m is None:
+      return
+
+    block.name = m.group(1)
+    header_start = m.start()
+    header_end = m.end() + 1
+
+    # Find the footer
+    footer = '-----END %s-----' % (block.name)
+    footer_start = data.find(footer, header_end)
+    if footer_start == -1:
+      raise "Missing PEM footer for %s" % (block.name)
+    footer_end = footer_start + len(footer)

[mattm, 2015/07/28 00:51:15]

you could do something fancier and make this whole parsing loop with re.finditer
and re.DOTALL. Or not. Not that important for a little helper script like this.

[eroman, 2015/07/28 01:41:23]

Thanks I will look that up.

Will probably keep it as is for now.

[eroman, 2015/07/28 18:24:52]

Done - switched to re.finditer()

+
+    block.data = DecodePemBlockData(data[header_end:footer_start])
+
+    # Keep track of any non-PEM text between blocks
+    block.comment = GetTextUntilNextPemBlock(data[footer_end:])
+
+    pos = footer_end
+    yield block
+
+
+def WriteStringToFile(data, path):
+  with open(path, "w") as f:
+    f.write(data)
+
+
+def main():
+  for path in GetFilePaths():
+    print "Processing %s ..." % (path)
+    original_data = ReadFileToString(path)
+    transformed_data = Transform(original_data)
+    if original_data != transformed_data:
+      WriteStringToFile(transformed_data, path)
+      print "Rewrote %s" % (path)
+
+
+if __name__ == "__main__":
+  main()