Skip to content

Commit

Permalink
Remove logging of args for failed iscsi cmds
Browse files Browse the repository at this point in the history
The iscsiadm functions provide a debug mode that currently logs the
iscsiadm command that was issued and the response.  This is handy for
the lib as it's in beta and we want to make it easy for users to pick up
and start using, BUT it also has a major problem in that it will also
log the arguments passed in for things like CHAP secrets.

This change just removes the debug logging of the iscsiadm cmd args, in
the future if we need/want some debug tools we can implement a more
robust and secure method that's safe and doesn't leak secrets to log
files.

closes kubernetes-csi#8
  • Loading branch information
j-griffith committed Apr 5, 2019
1 parent 696aa76 commit fca6aad
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 45 deletions.
1 change: 1 addition & 0 deletions iscsi/iscsi.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ func Connect(c Connector) (string, error) {
// create db entry
args := append(baseArgs, []string{"-I", iFace, "-o", "new"}...)
debug.Printf("create the new record: %s\n", args)
// Make sure we don't log the secrets
err := CreateDBEntry(c.TargetIqn, p, iFace, c.DiscoverySecrets, c.SessionSecrets)
if err != nil {
debug.Printf("Error creating db entry: %s\n", err.Error())
Expand Down
49 changes: 4 additions & 45 deletions iscsi/iscsiadm.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ package iscsi
import (
"bytes"
"fmt"
"os/exec"
"strings"
"syscall"
)

// Secrets provides optional iscsi security credentials (CHAP settings)
Expand Down Expand Up @@ -60,57 +58,18 @@ func iscsiCmd(args ...string) (string, error) {
}
}

iscsiadmDebug(args, string(stdout.Bytes()), iscsiadmError)
iscsiadmDebug(string(stdout.Bytes()), iscsiadmError)
return string(stdout.Bytes()), iscsiadmError
}

func iscsiadmDebug(args []string, output string, cmdError error) {
func iscsiadmDebug(output string, cmdError error) {
debugOutput := strings.Replace(output, "\n", "\\n", -1)
debug.Printf("Output of iscsiadm command: {{cmd: iscsiadm %s}, {output: %s}", args, debugOutput)
debug.Printf("Output of iscsiadm command: {output: %s}", debugOutput)
if cmdError != nil {
debug.Printf("Error message returned from issiadm command: %s", cmdError.Error())
debug.Printf("Error message returned from iscsiadm command: %s", cmdError.Error())
}
}

func iscsiCmdHide(args ...string) (string, error) {
debug.Printf("Execute iscsiadm %s", args)
var waitStatus syscall.WaitStatus
var iscsiHelper = "iscsiadm"
eStat := 0
out := &bytes.Buffer{}
cmdErr := &bytes.Buffer{}

c := execCommand(iscsiHelper, args...)
c.Stdout = out
c.Stderr = cmdErr
iscsiadmErr := CmdError{}

if err := c.Run(); err != nil {
if exitError, ok := err.(*exec.ExitError); ok {
waitStatus = exitError.Sys().(syscall.WaitStatus)
eStat = waitStatus.ExitStatus()
}
} else {
waitStatus = c.ProcessState.Sys().(syscall.WaitStatus)
eStat = waitStatus.ExitStatus()
}
if eStat != 0 || string(cmdErr.Bytes()) != "" {
iscsiadmErr = CmdError{
StdErr: string(cmdErr.Bytes()),
ExitCode: eStat,
CMD: iscsiHelper + " " + strings.Join(args, " "),
}
}
if &iscsiadmErr != nil {
debug.Printf("FUCK: %v\n", &iscsiadmErr)
}
debugOutput := strings.Replace(string(out.Bytes()), "\n", "\\n ", -1)
debugStderr := strings.Replace(string(cmdErr.Bytes()), "\n", "\\n ", -1)

debug.Printf("Response from iscsiadm, {{output: %s}, {stderr: %s}, {exit-code: %d}}", debugOutput, debugStderr, eStat)
return string(out.Bytes()), &iscsiadmErr
}

// ListInterfaces returns a list of all iscsi interfaces configured on the node
/// along with the raw output in Response.StdOut we add the convenience of
// returning a list of entries found
Expand Down

0 comments on commit fca6aad

Please sign in to comment.