Skip to content

Commit

Permalink
communicator/{ssh,winrm}: seed random script paths
Browse files Browse the repository at this point in the history
Without a seed, the "random" script path locations for the remote-exec
provisioner were actually deterministic!

Every rand.Int31() would return the same pseudorandom chain starting w/
the numbers: 1298498081, 2019727887, 1427131847, 939984059, ...

So here we properly seed the communicators so the script paths are
actually random, and multiple runs on a single remote host have much
less chance of clobbering each other.

Fixes #4186

Kudos to @DustinChaloupka for the correct hunch leading to this fix!
  • Loading branch information
phinze committed Jun 29, 2016
1 parent ebb9b7a commit 97f83ec
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 2 deletions.
5 changes: 4 additions & 1 deletion communicator/ssh/communicator.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type Communicator struct {
config *sshConfig
conn net.Conn
address string
rand *rand.Rand
}

type sshConfig struct {
Expand Down Expand Up @@ -68,6 +69,8 @@ func New(s *terraform.InstanceState) (*Communicator, error) {
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
Expand Down Expand Up @@ -185,7 +188,7 @@ func (c *Communicator) Timeout() time.Duration {
func (c *Communicator) ScriptPath() string {
return strings.Replace(
c.connInfo.ScriptPath, "%RAND%",
strconv.FormatInt(int64(rand.Int31()), 10), -1)
strconv.FormatInt(int64(c.rand.Int31()), 10), -1)
}

// Start implementation of communicator.Communicator interface
Expand Down
14 changes: 14 additions & 0 deletions communicator/ssh/communicator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,20 @@ func TestScriptPath(t *testing.T) {
}
}

func TestScriptPath_randSeed(t *testing.T) {
// Pre GH-4186 fix, this value was the deterministic start the pseudorandom
// chain of unseeded math/rand values for Int31().
staticSeedPath := "/tmp/terraform_1298498081.sh"
c, err := New(&terraform.InstanceState{})
if err != nil {
t.Fatalf("err: %s", err)
}
path := c.ScriptPath()
if path == staticSeedPath {
t.Fatalf("rand not seeded! got: %s", path)
}
}

const testClientPrivateKey = `-----BEGIN RSA PRIVATE KEY-----
MIIEpQIBAAKCAQEAxOgNXOJ/jrRDxBZTSk2X9otNy9zcpUmJr5ifDi5sy7j2ZiQS
beBt1Wf+tLNWis8Cyq06ttEvjjRuM75yucyD6GrqDTXVCSm4PeOIQeDhPhw26wYZ
Expand Down
5 changes: 4 additions & 1 deletion communicator/winrm/communicator.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type Communicator struct {
connInfo *connectionInfo
client *winrm.Client
endpoint *winrm.Endpoint
rand *rand.Rand
}

// New creates a new communicator implementation over WinRM.
Expand All @@ -44,6 +45,8 @@ func New(s *terraform.InstanceState) (*Communicator, error) {
comm := &Communicator{
connInfo: connInfo,
endpoint: endpoint,
// Seed our own rand source so that script paths are not deterministic
rand: rand.New(rand.NewSource(time.Now().UnixNano())),
}

return comm, nil
Expand Down Expand Up @@ -121,7 +124,7 @@ func (c *Communicator) Timeout() time.Duration {
func (c *Communicator) ScriptPath() string {
return strings.Replace(
c.connInfo.ScriptPath, "%RAND%",
strconv.FormatInt(int64(rand.Int31()), 10), -1)
strconv.FormatInt(int64(c.rand.Int31()), 10), -1)
}

// Start implementation of communicator.Communicator interface
Expand Down
14 changes: 14 additions & 0 deletions communicator/winrm/communicator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,17 @@ func TestScriptPath(t *testing.T) {
}
}
}

func TestScriptPath_randSeed(t *testing.T) {
// Pre GH-4186 fix, this value was the deterministic start the pseudorandom
// chain of unseeded math/rand values for Int31().
staticSeedPath := "C:/Temp/terraform_1298498081.cmd"
c, err := New(&terraform.InstanceState{})
if err != nil {
t.Fatalf("err: %s", err)
}
path := c.ScriptPath()
if path == staticSeedPath {
t.Fatalf("rand not seeded! got: %s", path)
}
}

0 comments on commit 97f83ec

Please sign in to comment.