Save the private keys used by generated verify_certificate_chain tests.

net/data/verify_certificate_chain_unittest/common.py

Index: net/data/verify_certificate_chain_unittest/common.py
diff --git a/net/data/verify_certificate_chain_unittest/common.py b/net/data/verify_certificate_chain_unittest/common.py
index a3d86420cfeb8aa3e91eb07606d25ca16310d108..bce072d2d3a5ca56b6f5e4937ba75ea65ab5f6b5 100755
--- a/net/data/verify_certificate_chain_unittest/common.py
+++ b/net/data/verify_certificate_chain_unittest/common.py
@@ -52,14 +52,16 @@ DEFAULT_KEY_PURPOSE = KEY_PURPOSE_SERVER_AUTH
 g_cur_path_id = {}
 
 # Output paths used:
-#   - g_out_dir: where any temporary files (keys, cert req, signing db etc) are
+#   - g_out_dir: where any temporary files (cert req, signing db etc) are
 #                saved to.
-#   - g_out_pem: the path to the final output (which is a .pem file)
+#   - g_script_name: the name of the invoking script. For instance if this is
+#                    being run by generate-foo.py then g_script_name will be
+#                    'foo'
 #
 # See init() for how these are assigned, based on the name of the calling
 # script.
 g_out_dir = None
-g_out_pem = None
+g_script_name = None
 
 # The default validity range of generated certificates. Can be modified with
 # set_default_validity_range().
@@ -75,6 +77,7 @@ def set_default_validity_range(start_date, end_date):
   g_default_start_date = start_date
   g_default_end_date = end_date
 
+
 def get_unique_path_id(name):
   """Returns a base filename that contains 'name', but is unique to the output
   directory"""
@@ -93,10 +96,6 @@ def get_path_in_output_dir(name, suffix):
   return os.path.join(g_out_dir, '%s%s' % (name, suffix))
 
 
-def get_unique_path_in_output_dir(name, suffix):
-  return get_path_in_output_dir(get_unique_path_id(name), suffix)
-
-
 class Key(object):
   """Describes a public + private key pair. It is a dumb wrapper around an
   on-disk key."""
@@ -110,38 +109,50 @@ class Key(object):
     return self.path
 
 
-def generate_rsa_key(size_bits, path=None):
-  """Generates an RSA private key and returns it as a Key object. If |path| is
-  specified the resulting key will be saved at that location."""
-  if path is None:
-    path = get_unique_path_in_output_dir('RsaKey', 'key')
+def get_or_generate_key(generation_arguments, path):
+  """Helper function to either retrieve a key from an existing file |path|, or
+  generate a new one using the command line |generation_arguments|."""
+
+  generation_arguments_str = ' '.join(generation_arguments)
 
-  # Ensure the path doesn't already exists (otherwise will be overwriting
-  # something).
-  assert not os.path.isfile(path)
+  # If the file doesn't already exist, generate a new key using the generation
+  # parameters.
+  if not os.path.isfile(path):
+    key_contents = subprocess.check_output(generation_arguments)
 
-  subprocess.check_call(
-      ['openssl', 'genrsa', '-out', path, str(size_bits)])
+    # Prepend the generation parameters to the key file.
+    write_string_to_file(generation_arguments_str + '\n' + key_contents,
+                         path)
+  else:
+    # If the path already exists, confirm that it is for the expected key type.
+    file_contents = read_file_to_string(path)
+    if not file_contents.startswith(generation_arguments_str):

[mattm, 2017/04/07 22:01:27]

This could just match part of the generation args in the file if it has a common
prefix. Should have generation_arguments_str include the newline, or equality
compare against the first line of the file (ex, using splitlines()[0]).
I guess the splitlines approach would be safe against different line endings.

[eroman, 2017/04/07 23:36:33]

Good point! Done.

+      sys.stderr.write(
+          ('The existing key file %s is not compatible with the '
+           'requested parameters:\n%s.\n') % (path, generation_arguments_str))

[mattm, 2017/04/07 22:01:27]

might be helpful to mention you can delete the key file to have a new one
generated

[eroman, 2017/04/07 23:36:33]

Done. I also print out the contradiction explicitly now.

+      sys.exit(1)
 
   return Key(path)
 
 
-def generate_ec_key(named_curve, path=None):
-  """Generates an EC private key for the certificate and returns it as a Key
-  object. |named_curve| can be something like secp384r1. If |path| is specified
-  the resulting key will be saved at that location."""
-  if path is None:
-    path = get_unique_path_in_output_dir('EcKey', 'key')
+def get_or_generate_rsa_key(size_bits, path):
+  """Retrieves an existing key from a file if the path exists. Otherwise
+  generates an RSA key with the specified bit size and saves it to the path."""
+  return get_or_generate_key(['openssl', 'genrsa', str(size_bits)], path)
 
-  # Ensure the path doesn't already exists (otherwise will be overwriting
-  # something).
-  assert not os.path.isfile(path)
 
-  subprocess.check_call(
-      ['openssl', 'ecparam', '-out', path,
-       '-name', named_curve, '-genkey'])
+def get_or_generate_ec_key(named_curve, path):
+  """Retrieves an existing key from a file if the path exists. Otherwise
+  generates an EC key with the specified named curve and saves it to the
+  path."""
+  return get_or_generate_key(['openssl', 'ecparam', '-name', named_curve,
+                              '-genkey'], path)
 
-  return Key(path)
+
+def create_key_path(base_name):
+  """Generates a name that contains |base_name| in it, and is relative to the
+  "keys/" directory"""
+  return get_unique_path_id('keys/' + g_script_name + '_' + base_name) + '.key'

[mattm, 2017/04/07 22:01:27]

This may work a bit less well for other scripts that use common.py to generate a
bunch of certs directly rather than using the write_test_file interface. (eg
net/data/cert_issuer_source_aia_unittest/generate-certs.py,
net/data/cert_issuer_source_static_unittest/generate-certs.py)

Looks like it does work, but you end up with keys like "certs_target.key",
"certs_target_1.key", "certs_target_2.key", etc which don't correspond to the
names the cert is actually saved as. And would probably get re-associated with a
different cert if you added/removed/re-ordered the cert creation.

Those scripts currently save a cert like:
common.write_string_to_file(target.get_cert_pem(), 'target_one_aia.pem')        
                                                                                
          

I guess we could have a helper method that takes the target file name, and uses
it both to generate the key(if it hasn't already had one manually set with
set_key) and output the cert file.

wdyt?

[eroman, 2017/04/07 23:36:33]

I made a few changes which hopefully address those concerns:

 (1) Changed to a directory grouping under keys/ to make the names less terrible
 (2) For the specific test you hilighted, I extracted 3 named keys
 (3) [bugfix] added code to automatically create the keys/ directory when
missing

 
 
 class Certificate(object):
@@ -255,7 +266,8 @@ class Certificate(object):
 
   def get_key(self):
     if self.key is None:
-      self.set_key_internal(generate_rsa_key(2048, path=self.get_path(".key")))
+      self.set_key_internal(
+          get_or_generate_rsa_key(2048, create_key_path(self.name)))
     return self.key
 
 
@@ -284,8 +296,7 @@ class Certificate(object):
     self.finalize()
 
     # Read the certificate data.
-    with open(self.get_cert_path(), 'r') as f:
-      return f.read()
+    return read_file_to_string(self.get_cert_path())
 
 
   def finalize(self):
@@ -480,7 +491,9 @@ def write_test_file(description, chain, trust_anchor, utc_time, key_purpose,
   if errors is not None:
     test_data += '\n' + text_data_to_pem('ERRORS', errors)
 
-  write_string_to_file(test_data, out_pem if out_pem else g_out_pem)
+  if not out_pem:
+    out_pem = g_script_name + '.pem'
+  write_string_to_file(test_data, out_pem)
 
 
 def write_string_to_file(data, path):
@@ -488,6 +501,11 @@ def write_string_to_file(data, path):
     f.write(data)
 
 
+def read_file_to_string(path):
+  with open(path, 'r') as f:
+    return f.read()
+
+
 def init(invoking_script_path):
   """Creates an output directory to contain all the temporary files that may be
   created, as well as determining the path for the final output. These paths
@@ -495,7 +513,7 @@ def init(invoking_script_path):
   """
 
   global g_out_dir
-  global g_out_pem
+  global g_script_name
 
   # Base the output name off of the invoking script's name.
   out_name = os.path.splitext(os.path.basename(invoking_script_path))[0]
@@ -512,7 +530,7 @@ def init(invoking_script_path):
   shutil.rmtree(g_out_dir, True)
   os.makedirs(g_out_dir)
 
-  g_out_pem = os.path.join('%s.pem' % (out_name))
+  g_script_name = out_name
 
 
 def create_self_signed_root_certificate(name):