From 4b1d9cfd7d1ebab37b05487057b8da7effc02e29 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Tue, 6 Dec 2016 00:11:32 -0800
Subject: [PATCH] communicator/ssh: don't share rand object to guarantee unique
 values

Fixes #10463

I'm really surprised this flew under the radar for years...

By having unique PRNGs, the SSH communicator could and would
generate identical ScriptPaths and two provisioners running in parallel
could overwrite each other and execute the same script. This would
happen because they're both seeded by the current time which could
potentially be identical if done in parallel...

Instead, we share the rand now so that the sequence is guaranteed
unique. As an extra measure of robustness, we also multiple by the PID
so that we're also protected against two processes at the same time.
---
 .../remote-exec/resource_provisioner.go       |  1 -
 communicator/ssh/communicator.go              | 29 ++++++++++++++++---
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go
index 2f784a4ea..a199b95c4 100644
--- a/builtin/provisioners/remote-exec/resource_provisioner.go
+++ b/builtin/provisioners/remote-exec/resource_provisioner.go
@@ -178,7 +178,6 @@ func (p *ResourceProvisioner) runScripts(
 
 		remotePath := comm.ScriptPath()
 		err = retryFunc(comm.Timeout(), func() error {
-
 			if err := comm.UploadScript(remotePath, script); err != nil {
 				return fmt.Errorf("Failed to upload script: %v", err)
 			}
diff --git a/communicator/ssh/communicator.go b/communicator/ssh/communicator.go
index 0b3a222b3..32a881c04 100644
--- a/communicator/ssh/communicator.go
+++ b/communicator/ssh/communicator.go
@@ -14,6 +14,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/hashicorp/terraform/communicator/remote"
@@ -27,6 +28,13 @@ const (
 	DefaultShebang = "#!/bin/sh\n"
 )
 
+// randShared is a global random generator object that is shared.
+// This must be shared since it is seeded by the current time and
+// creating multiple can result in the same values. By using a shared
+// RNG we assure different numbers per call.
+var randLock sync.Mutex
+var randShared *rand.Rand
+
 // Communicator represents the SSH communicator
 type Communicator struct {
 	connInfo *connectionInfo
@@ -34,7 +42,6 @@ type Communicator struct {
 	config   *sshConfig
 	conn     net.Conn
 	address  string
-	rand     *rand.Rand
 }
 
 type sshConfig struct {
@@ -66,11 +73,22 @@ func New(s *terraform.InstanceState) (*Communicator, error) {
 		return nil, err
 	}
 
+	// Setup the random number generator once. The seed value is the
+	// time multiplied by the PID. This can overflow the int64 but that
+	// is okay. We multiply by the PID in case we have multiple processes
+	// grabbing this at the same time. This is possible with Terraform and
+	// if we communicate to the same host at the same instance, we could
+	// overwrite the same files. Multiplying by the PID prevents this.
+	randLock.Lock()
+	defer randLock.Unlock()
+	if randShared == nil {
+		randShared = rand.New(rand.NewSource(
+			time.Now().UnixNano() * int64(os.Getpid())))
+	}
+
 	comm := &Communicator{
 		connInfo: connInfo,
 		config:   config,
-		// Seed our own rand source so that script paths are not deterministic
-		rand: rand.New(rand.NewSource(time.Now().UnixNano())),
 	}
 
 	return comm, nil
@@ -186,9 +204,12 @@ func (c *Communicator) Timeout() time.Duration {
 
 // ScriptPath implementation of communicator.Communicator interface
 func (c *Communicator) ScriptPath() string {
+	randLock.Lock()
+	defer randLock.Unlock()
+
 	return strings.Replace(
 		c.connInfo.ScriptPath, "%RAND%",
-		strconv.FormatInt(int64(c.rand.Int31()), 10), -1)
+		strconv.FormatInt(int64(randShared.Int31()), 10), -1)
 }
 
 // Start implementation of communicator.Communicator interface