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.

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)
 			}

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