Allow fingerprints to be explicitly specified. (#192)

Co-authored-by: Carl Johnson <carljohnson@improbable.io>

Makefile

@@ -14,13 +14,8 @@ vendorize:
 	# Checkout hook not available locally, so make a command hook instead
 	cp local-pipeline/plugins/perforce/hooks/checkout local-pipeline/plugins/perforce/hooks/command -f
 
-# p4d: export P4SSLDIR=sslkeys
 p4d: clean_p4d
 	unzip python/fixture/server.zip	-d python/fixture/server/
-	# mkdir python/fixture/server/sslkeys
-	# chmod 700 python/fixture/server/sslkeys
-	# p4d -r python/fixture/server -Gc
-	# p4d -r python/fixture/server -Gf
 	p4d -r python/fixture/server -p 1666 &
 
 clean_p4d:

README.md

@@ -44,6 +44,10 @@ Override configuration at the User Environment level. May be overridden by P4CON
 
 See [p4 set](https://www.perforce.com/manuals/cmdref/Content/CmdRef/p4_set.html?Highlight=precedence) for more on system variables and precedence.
 
+#### `fingerprint` (optional, string)
+
+Supply a trusted p4 server fingerprint to ensure the server the client connects to has not been MITM'd.
+
 #### `stream` (optional, string)
 
 Which p4 stream to sync, e.g. `//dev/minimal`. Can be overridden by `view`.
@@ -165,11 +169,11 @@ Run `dev/setup_env_osx.sh`
 
 Python [virtualenv](https://docs.python.org/3/tutorial/venv.html) `.dev-venv` for running tests will be created at repo root.
 
-Run the `test_fixture` unit test to check everything is setup correctly:
+Run the `test_server_fixture` unit test to check everything is setup correctly:
 
 ```bash
 source .dev-venv/bin/activate
-pytest python/test_perforce.py -k test_fixture
+pytest python/test_perforce.py -k test_server_fixture
 ```
 
 ### Linux/Windows
@@ -180,7 +184,7 @@ TBC, feedback welcome.
 
 Making changes to `python/`
 
-* Read implementation of `test_fixture` in `test_perforce.py`
+* Read implementation of `test_server_fixture` in `test_perforce.py`
 * Write unit test in `test_perforce.py`, optionally making changes to the test fixture if required
 * Implement new functionality
 * Iterate via unit test

hooks/pre-checkout

@@ -26,4 +26,4 @@ if [[ "${BUILDKITE_PLUGIN_PERFORCE_SHARE_WORKSPACE}" == true ]] ; then
     PERFORCE_CHECKOUT_PATH="${BUILDKITE_BUILD_CHECKOUT_PATH}/../../${SANITIZED_STREAM}"
     export BUILDKITE_BUILD_CHECKOUT_PATH="${PERFORCE_CHECKOUT_PATH}"
     echo "Changed BUILDKITE_BUILD_CHECKOUT_PATH to ${PERFORCE_CHECKOUT_PATH}"
-fi
+fi

plugin.yml

@@ -27,5 +27,7 @@ configuration:
       type: bool
     sync:
       type: array
+    fingerprint:
+      type: string
     view:
       type: string

python/buildkite.py

@@ -40,7 +40,7 @@ def list_from_env_array(var):
             break
         result.append(elem)
         i += 1
-    
+
     return result
 
 def get_config():
@@ -52,12 +52,12 @@ def get_config():
     conf['parallel'] = os.environ.get('BUILDKITE_PLUGIN_PERFORCE_PARALLEL') or 0
     conf['client_options'] = os.environ.get('BUILDKITE_PLUGIN_PERFORCE_CLIENT_OPTIONS')
     conf['client_type'] = os.environ.get('BUILDKITE_PLUGIN_PERFORCE_CLIENT_TYPE')
+    conf['fingerprint'] = list_from_env_array('BUILDKITE_PLUGIN_PERFORCE_FINGERPRINT')
 
     if 'BUILDKITE_PLUGIN_PERFORCE_ROOT' in os.environ and not __LOCAL_RUN__:
         raise Exception("Custom P4 root is for use in unit tests only")
     conf['root'] = os.environ.get('BUILDKITE_PLUGIN_PERFORCE_ROOT') or os.environ.get('BUILDKITE_BUILD_CHECKOUT_PATH')
 
-
     # Coerce view into pairs of [depot client] paths
     view_parts = conf['view'].split(' ')
     assert (len(view_parts) % 2) == 0, "Invalid view format"
@@ -109,7 +109,7 @@ def set_build_changelist(changelist):
     """Set a shelved change that should be used instead of the user-supplied one"""
     if set_metadata(__SHELVED_METADATA__, changelist) and should_backup_changelists():
         subprocess.call([
-            'buildkite-agent', 'annotate', 
+            'buildkite-agent', 'annotate',
             __SHELVED_ANNOTATION__.format(**{
                 'original': get_users_changelist(),
                 'copy': changelist,
@@ -133,7 +133,7 @@ def get_build_revision():
     if revision.startswith('@') or revision.startswith('#'):
         return revision
     # Unable to establish a concrete revision for the build
-    return None 
+    return None
 
 def set_build_revision(revision):
     """Set the p4 revision for following jobs in this build"""

python/fixture/insecure-ssl/certificate.txt

@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDUzCCAjugAwIBAwIBATANBgkqhkiG9w0BAQUFADBtMQswCQYDVQQGEwJVUzEL
+MAkGA1UECAwCQ0ExEDAOBgNVBAcMB0FsYW1lZGExHjAcBgNVBAoMFVBlcmZvcmNl
+IEF1dG9nZW4gQ2VydDEfMB0GA1UEAwwWUGV0ZXJzLU1CUC0yLmZyaXR6LmJveDAe
+Fw0yMDEwMjAxNTI1MTlaFw0yMjEwMjAxNTI1MTlaMG0xCzAJBgNVBAYTAlVTMQsw
+CQYDVQQIDAJDQTEQMA4GA1UEBwwHQWxhbWVkYTEeMBwGA1UECgwVUGVyZm9yY2Ug
+QXV0b2dlbiBDZXJ0MR8wHQYDVQQDDBZQZXRlcnMtTUJQLTIuZnJpdHouYm94MIIB
+IjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAzqwbTJTvARGzfseVU5qKrxUQ
+LZMs5PjoMUxhY+nqHXeWIN0BZlAuG2feO3qGnWHbVJf/2YFxrqvDfDTUWaYIrbsg
+U2NeZcW9xrKm+0BllLCYkf12zDtc58MUMfgUuQSNAiPaYN/xztQ+Ief1fj2VxpiN
+fkxCeVrk5R4ccbrfOV39/2Dz1Zx+g6F9o4c1xPmyS+DpYtULIduYq2ERnupOs7x/
+qQMwtPJZDQbVChwYdae5L1pqc+gHHgFwDs02FL6cxQjPSaUXl8hvx4/ke55wp9f6
+r4W7GljjFcUCzXYjefCMX1X9kUjI5AAFZ+yPgzxSRjnjjew4KtVWVRom29/tlQID
+AQABMA0GCSqGSIb3DQEBBQUAA4IBAQC2sujXI7RbaazWjbGXzSZIHN7PGaqxMQIn
+RP6AqlW3wvO0J29gbtSv4VmuH9z4EkgNISeSeAWUrY+YCdJYPQpj1kdiwZpxT3M5
+P8f6IrasIznUkgqmaOKjifoGTshhGQ7TtbQY2kFFyCNVI4749F/rniHcr9ELazZq
+p06cwKLjSDulFTWO8MxBrPyh6UnhTfMHWUYjt9pGGEhTMis5ilwO/qaBUvCxaQKa
+CIBvCQFnjkzdQAb8CJ9EwDpOI1VM0/Pf6FJPDJKaQ8RcF1b7IjT2RE27KXe3k0xQ
+Ebz3gwplFQ+wGJVWCT9wpS+HQGCsJBKQF2u9P7PEQWMvlNEBWk+y
+-----END CERTIFICATE-----

python/fixture/insecure-ssl/privatekey.txt

@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQDOrBtMlO8BEbN+
+x5VTmoqvFRAtkyzk+OgxTGFj6eodd5Yg3QFmUC4bZ947eoadYdtUl//ZgXGuq8N8
+NNRZpgituyBTY15lxb3Gsqb7QGWUsJiR/XbMO1znwxQx+BS5BI0CI9pg3/HO1D4h
+5/V+PZXGmI1+TEJ5WuTlHhxxut85Xf3/YPPVnH6DoX2jhzXE+bJL4Oli1Qsh25ir
+YRGe6k6zvH+pAzC08lkNBtUKHBh1p7kvWmpz6AceAXAOzTYUvpzFCM9JpReXyG/H
+j+R7nnCn1/qvhbsaWOMVxQLNdiN58IxfVf2RSMjkAAVn7I+DPFJGOeON7Dgq1VZV
+Gibb3+2VAgMBAAECggEAdf38d/Rvn4SjnbYEov6QPvUfj2V/NBqHNd4NnCVn6/ri
+U1DaA7ezGyJp1jtVr3S268z73QnyBW865CalNal9OvKiufj5Y9FJT6+fdcKGPCW2
+dWLn+CHMIOVXGlAwRJE8kAQ4ISa5vwOdlW4A0loGsKNX5MtVCEPEeqp+QtAVsYHY
+SeJyWw2QqlzMJ4i44zdxhHm3SpXmpIs8Rk0pee6Wta+EmS/kwExsRpgldz/L/DGw
+VGUDIPdHbGYf58Aic9N7adKjzP8NOiyqOv1WyWqzH5Sz1rkDMElLJEk4SMY9P2VY
+SwmTsdbtox2UDCgG7n1ZKryCDbcG/ppdIONR0Qx7AQKBgQD8GD54jjjp4E7rLBHG
+dJaXILg4ZSSMAIeSsYkVCya8vzg84EIF5lbCfuhybYOJcB1KapDZ2/WiNzg3V2ur
+GBxZtxvDGCPyI7PY+CVy0z39fKykvpe+O1Ai1FsUEutG6bX9/r4tIZTvgPzIXC5W
+j8n9mJCCNBmLNkdqhr024jLCqQKBgQDR37oEMMiZBHED9Zd6SfsTUIV/bO6dDdRR
+nEpG3C3c6gT783j8D65GyMPvA3anriSzzchBIXtXP0GOcoXfXNGrIDiZeCYo8rn4
+kitSP3ASw1Ydpi6QKCF32z8nCc9njdt7YZ7GO2lpcBKR+/ODnvWzCuSSYQprV9YJ
+nW0Q5X2TDQKBgAEUtYfczD+sd4oomTbpnw+s0z1iqaJ0CiDF5BmT/6mFhF82cvIF
+h8+zrZl4AL1hHq8H//D/MXFtnS8Xj92e79guoc7XVqgeIRJIFhkE8NoaY78dFhd5
+t6E+mdlfL2URcXdSVUxqPXI9clgFlSlH2ozcz5nPUWC4bdv2Ee+fTqppAoGAXyyu
+Fqhoz7uL5NfC0doq2h9x6s9jhiV3W2sc4/WFduFJUVigTO5vgfoZoJJZhMEcM83m
+OmMMpAwzln2o6BoXmxsJj89Evt0UKP1gV/QcxuV+cAOkqgsI4mmywelY/QT/u3wR
+nKPkscP5J+qyC8ZSdddCwH7xUqyKi+GwTDFGOSECgYBghf4sow5Q31o+fhI2BSCE
+/6AfQkaBSjzf1KZiZseU2Je8anRhnzUN5eo/dOgDuixp1jcHZolHJO1OxsyGbjMW
+VWk20wWHl7iDb9cVIqC1ev5dVGw9KhOTbpETxPoFEMfMvQ4CAG6hTBcadCuXjUDJ
+nee25LdY7l8t5qam/DNDLA==
+-----END PRIVATE KEY-----

python/fixture/insecure-ssl/readme.md

@@ -0,0 +1,14 @@
+# Insecure SSL
+
+Cert for use in unit tests.
+
+Do not use in production.
+
+Generated via:
+
+```bash
+mkdir -p "python/fixture/insecure-ssl"
+chmod 700 "python/fixture/insecure-ssl"
+P4SSLDIR="python/fixture/insecure-ssl" p4d -Gc
+P4SSLDIR="python/fixture/insecure-ssl" p4d -Gf
+```

python/perforce.py

@@ -16,7 +16,7 @@
 class P4Repo:
     """A class for manipulating perforce workspaces"""
     def __init__(self, root=None, view=None, stream=None, sync=None,
-                 client_options=None, client_type=None, parallel=0):
+                 client_options=None, client_type=None, parallel=0, fingerprint=None):
         """
         root: Directory in which to create the client workspace
         view: Client workspace mapping
@@ -25,6 +25,7 @@ def __init__(self, root=None, view=None, stream=None, sync=None,
         client_options: Additional options to add to client. (e.g. allwrite)
         client_type: Type of client (writeable, readonly, partitioned)
         parallel: How many threads to use for parallel sync.
+        fingerprint: Acceptable fingerprint for a p4 server to have.
         """
         self.root = os.path.abspath(root or '')
         self.stream = stream
@@ -34,6 +35,7 @@ def __init__(self, root=None, view=None, stream=None, sync=None,
         self.client_options = client_options or ''
         self.client_type = client_type or 'writeable'
         self.parallel = parallel
+        self.fingerprint = fingerprint or ''
 
         self.created_client = False
         self.patchfile = os.path.join(self.root, 'patched.json')
@@ -53,9 +55,18 @@ def __init__(self, root=None, view=None, stream=None, sync=None,
         logger.addHandler(handler)
         self.perforce.logger = logger
         self.perforce.connect()
+
         if self.perforce.port.startswith('ssl'):
-            # TODO: Remove this and enforce prior provisioning of trusted fingerprints
-            self.perforce.run_trust('-y')
+            if self.fingerprint:
+                self.perforce.run_trust(
+                    '-r',       # Install a replacement fingerprint - will replace primary if this matches the server
+                    '-i',       # Install the specified fingerprint
+                    self.fingerprint,
+                )
+            else:
+                # Trust fingerprint from first contact with server
+                # If fingerprint changes, MITM attack is reported
+                self.perforce.run_trust('-y')
 
     def __del__(self):
         self.perforce.disconnect()
@@ -195,7 +206,7 @@ def head_at_revision(self, revision):
                 revision = labelinfo.get('Revision') or revision
             except P4Exception:
                 # revision may be clientname, datespec or something else
-                # fallback to default behaviour 
+                # fallback to default behaviour
                 pass
 
         # Get last submitted change at revision spec
@@ -281,7 +292,7 @@ def p4print_unshelve(self, changelist):
 
         whereinfo = self.perforce.run_where(depotfiles)
         depot_to_local = {item['depotFile']: item['path'] for item in whereinfo}
-        
+
         # Flag these files as modified
         self._write_patched(list(depot_to_local.values()))
 
@@ -319,7 +330,7 @@ def __init__(self, logger):
         OutputHandler.__init__(self)
         self.logger = logger
         self.sync_count = 0
-    
+
     def outputStat(self, stat):
         if 'depotFile' in stat:
             self.sync_count  += 1