Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions cmd/fabric-ca-server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
"regexp"
"strings"

"github.com/cloudflare/cfssl/log"
Expand Down Expand Up @@ -160,6 +161,7 @@ registry:
identities:
- name: <<<ADMIN>>>
pass: <<<ADMINPW>>>
passFile: <<<ADMINPASSFILE>>>
type: client
affiliation: ""
attrs:
Expand Down Expand Up @@ -619,7 +621,7 @@ func (s *ServerCmd) configInit() (err error) {
}

func (s *ServerCmd) createDefaultConfigFile() error {
var user, pass string
var user, pass, upFile string
// If LDAP is enabled, authentication of enrollment requests are performed
// by using LDAP authentication; therefore, no bootstrap username and password
// are required.
Expand All @@ -631,6 +633,7 @@ func (s *ServerCmd) createDefaultConfigFile() error {
// bootstrap administrator. Other identities can be dynamically registered.
// Create the default config, but only if they provided this bootstrap
// username and password.
upFile = s.myViper.GetString("bootfile")
up := s.myViper.GetString("boot")
if up == "" {
return errors.New("The '-b user:pass' option is required")
Expand All @@ -644,11 +647,12 @@ func (s *ServerCmd) createDefaultConfigFile() error {
}
user = ups[0]
pass = ups[1]

if len(user) >= 1024 {
return errors.Errorf("The identity name must be less than 1024 characters: '%s'", user)
}
if len(pass) == 0 {
return errors.New("An empty password in the '-b user:pass' option is not permitted")
if len(pass) == 0 && upFile == "" {
return errors.New("An empty password in the '-b user:pass' and '-f passfile' option is not permitted")
}
}

Expand All @@ -662,6 +666,13 @@ func (s *ServerCmd) createDefaultConfigFile() error {
// Do string subtitution to get the default config
cfg := strings.Replace(defaultCfgTemplate, "<<<VERSION>>>", metadata.Version, 1)
cfg = strings.Replace(cfg, "<<<ADMIN>>>", user, 1)
// When not provided, remove passfile from template
if upFile != "" && !util.FileExists(upFile) {
re := regexp.MustCompile("(?m)[\r\n]+^.*ADMINPASSFILE.*$")
cfg = re.ReplaceAllString(cfg, "")
} else {
cfg = strings.Replace(cfg, "<<<ADMINPASSFILE>>>", upFile, 1)
}
cfg = strings.Replace(cfg, "<<<ADMINPW>>>", pass, 1)
cfg = strings.Replace(cfg, "<<<MYHOST>>>", myhost, 1)
purl := s.myViper.GetString("intermediate.parentserver.url")
Expand Down
89 changes: 50 additions & 39 deletions cmd/fabric-ca-server/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"os"
"path"
"path/filepath"
"regexp"
"testing"

"github.com/cloudflare/cfssl/log"
Expand All @@ -27,25 +26,15 @@ import (
)

const (
initYaml = "i.yaml"
startYaml = "s.yaml"
ldapTestDir = "ldapTestDir"
)

var (
longUserName = util.RandomString(1025)
)

var (
longFileName = util.RandomString(261)
)

// Create a config element in unexpected format
var badSyntaxYaml = "bad.yaml"

// Unsupported file type
var unsupportedFileType = "config.txt"

type TestData struct {
input []string // input
expected string // expected result
Expand All @@ -69,19 +58,6 @@ func checkTest(in *TestData, t *testing.T) {
}
}

// errorTest validates error cases
func errorTest(in *TestData, t *testing.T) {
err := RunMain(in.input)
if err != nil {
matched, _ := regexp.MatchString(in.expected, err.Error())
if !matched {
t.Errorf("FAILED:\n \tin: %v;\n \tout: %v;\n \texpected: %v\n", in.input, err.Error(), in.expected)
}
} else {
t.Errorf("FAILED:\n \tin: %v;\n \tout: <nil>\n \texpected: %v\n", in.input, in.expected)
}
}

func TestMain(m *testing.M) {
os.Setenv("FABRIC_CA_SERVER_OPERATIONS_LISTENADDRESS", "localhost:0")
defer os.Unsetenv("FABRIC_CA_SERVER_OPERATIONS_LISTENADDRESS")
Expand All @@ -99,9 +75,22 @@ func TestNoArguments(t *testing.T) {

func TestErrors(t *testing.T) {
os.Unsetenv(homeEnvVar)
initYaml := "i.yaml"
// Create a config element in unexpected format
badSyntaxYaml := "bad.yaml"
// Unsupported file type
unsupportedFileType := "config.txt"
_ = ioutil.WriteFile(badSyntaxYaml, []byte("signing: true\n"), 0644)

errorCases := []TestData{
defer func() {
os.Remove(badSyntaxYaml)
os.Remove(unsupportedFileType)
os.Remove(startYaml)
}()

tests := []struct {
cmd []string
expected string
}{
{[]string{cmdName, "init", "-c", initYaml}, "option is required"},
{[]string{cmdName, "init", "-c", initYaml, "-n", "acme.com", "-b", "user::"}, "Failed to read"},
{[]string{cmdName, "init", "-b", "user:pass", "-n", "acme.com", "ca.key"}, "Unrecognized arguments found"},
Expand All @@ -118,20 +107,45 @@ func TestErrors(t *testing.T) {
{[]string{cmdName, "start", "-c", startYaml, "-b", "user:pass", "ca.key"}, "Unrecognized arguments found"},
}

for _, e := range errorCases {
errorTest(&e, t)
_ = os.Remove(initYaml)
for _, tt := range tests {
tt := tt
t.Run(tt.expected, func(t *testing.T) {
err := RunMain(tt.cmd)
assert.Error(t, err, tt.expected)
os.Remove(initYaml)
})
}
}

func TestOneTimePass(t *testing.T) {
testDir := "oneTimePass"
os.RemoveAll(testDir)
defer os.RemoveAll(testDir)
// Test with "-b" option
err := RunMain([]string{cmdName, "init", "-b", "admin:adminpw", "--registry.maxenrollments", "1", "-H", testDir})
if err != nil {
t.Fatalf("Failed to init server with one time passwords: %s", err)
tests := []struct {
testName string
cmd []string
}{
{
testName: "When identity has user and pass",
cmd: []string{cmdName, "init", "-b", "admin:adminpw", "--registry.maxenrollments", "1", "-H", os.TempDir()},
},
{
testName: "When identity has user and passfile",
cmd: []string{cmdName, "init", "-b", "admin:adminpw", "-f", "pwFile", "--registry.maxenrollments", "1", "-H", os.TempDir()},
},
{
testName: "When identity has user, pass, and passfile",
cmd: []string{cmdName, "init", "-b", "admin:adminpw", "-f", "pwFile", "--registry.maxenrollments", "1", "-H", os.TempDir()},
},
}

for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
defer os.Remove(filepath.Join(os.TempDir(), "pwFile"))

err := ioutil.WriteFile(filepath.Join(os.TempDir(), "pwFile"), []byte("mypassword\n"), 0666)
assert.NoError(t, err, "Failed to create passfile")

err = RunMain(tt.cmd)
assert.NoError(t, err, "Failed to init server with one time passwords")
})
}
}

Expand Down Expand Up @@ -426,11 +440,8 @@ func checkConfigAndDBLoc(t *testing.T, args TestData, cfgFile string, dsFile str
func TestClean(t *testing.T) {
defYaml := util.GetDefaultConfigFile(cmdName)
os.Remove(defYaml)
os.Remove(initYaml)
os.Remove(startYaml)
os.Remove(badSyntaxYaml)
os.Remove(fmt.Sprintf("/tmp/%s.yaml", longFileName))
os.Remove(unsupportedFileType)
os.Remove("ca-key.pem")
os.Remove("ca-cert.pem")
os.Remove("IssuerSecretKey")
Expand Down
2 changes: 2 additions & 0 deletions cmd/fabric-ca-server/servercmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ func (s *ServerCmd) registerFlags() {
pflags.StringVarP(&s.homeDirectory, "home", "H", "", fmt.Sprintf("Server's home directory (default \"%s\")", filepath.Dir(cfg)))
util.FlagString(s.myViper, pflags, "boot", "b", "",
"The user:pass for bootstrap admin which is required to build default config file")
util.FlagString(s.myViper, pflags, "bootfile", "f", "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage question here: when I try to specify a username and passfile, it complains because I haven't specified a password after a colon separator:
fabric-ca-server init -b admin -f pass.file
Error: Failed to create default configuration file: The value 'admin' on the command line is missing a colon separator

While it isn't too difficult to add the : after admin, it feels like we shouldn't require that when -f is used. Any reason that shouldn't be the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, this was a design choice (trying to change as little as possible). The init and start commands still require both -b admin:pass with the -f pass.file flag specified. The pass from -b user:pass will be used as a backup when a password cannot be read from pass.file when starting server.

Another option as you suggested is to allow only -b admin when -f pass.file is provided then

  1. Allow the server to error on startup if the pass.file cannot be read.
  2. Error when creating the default config file if the pass.file doesn't exist, cannot be read, etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sykesm Any thoughts / opinions here?

"Password file for bootstrap admin which is required to build the default config file")

// Register flags for all tagged and exported fields in the config
s.cfg = &lib.ServerConfig{}
Expand Down
1 change: 1 addition & 0 deletions docs/source/servercli.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Fabric-CA Server's CLI
Flags:
--address string Listening address of fabric-ca-server (default "0.0.0.0")
-b, --boot string The user:pass for bootstrap admin which is required to build default config file
-f, --bootfile string Password file for bootstrap admin which is required to build the default config file
--ca.certfile string PEM-encoded CA certificate file (default "ca-cert.pem")
--ca.chainfile string PEM-encoded CA chain file (default "ca-chain.pem")
--ca.keyfile string PEM-encoded CA key file
Expand Down
1 change: 1 addition & 0 deletions docs/source/serverconfig.rst
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ Fabric-CA Server's Configuration File
identities:
- name: <<<adminUserName>>>
pass: <<<adminPassword>>>
passFile:
type: client
affiliation: ""
attrs:
Expand Down
12 changes: 12 additions & 0 deletions lib/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"path"
"path/filepath"
"strconv"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -825,6 +826,17 @@ func (ca *CA) addIdentity(id *CAConfigIdentity, errIfFound bool) error {
return err
}

// A password file takes precedence over password
if id.PassFile != "" {
passBytes, err := util.ReadFile(id.PassFile)
if err != nil {
return errors.WithMessage(err, fmt.Sprintf("Failed to read password from '%s'", id.PassFile))
}
// Remove the newline from reading file
id.Pass = string(passBytes[:])
id.Pass = strings.TrimSuffix(id.Pass, "\n")
}

rec := cadbuser.Info{
Name: id.Name,
Pass: id.Pass,
Expand Down
88 changes: 88 additions & 0 deletions lib/ca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package lib

import (
"crypto/x509"
"errors"
"fmt"
"io/ioutil"
"os"
Expand Down Expand Up @@ -351,6 +352,93 @@ func TestCAgetUserAttrValue(t *testing.T) {
CAclean(ca, t)
}

func TestCAAddIdentity(t *testing.T) {
tests := []struct {
testName string
id *CAConfigIdentity
}{
{
testName: "When identity has user and pass",
id: &CAConfigIdentity{
Name: "admin1",
Pass: "adminpw",
},
},
{
testName: "When identity has user and passfile",
id: &CAConfigIdentity{
Name: "admin2",
PassFile: filepath.Join(os.TempDir(), "adminpwfile"),
},
},
{
testName: "When identity has user, pass, and passfile",
id: &CAConfigIdentity{
Name: "admin3",
Pass: "adminpw",
PassFile: filepath.Join(os.TempDir(), "adminpwfile"),
},
},
}

for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
testDirClean(t)

if tt.id.PassFile != "" {
err := ioutil.WriteFile(tt.id.PassFile, []byte("mypassword\n"), 0666)
assert.NoError(t, err, "Failed to create passfile")
defer os.Remove(tt.id.PassFile)
}

cfg = CAConfig{}
cfg.Registry = CAConfigRegistry{MaxEnrollments: 10}
ca, err := newCA(configFile, &cfg, &srv, false)
assert.NoError(t, err, "Failed creating newCA")

err = ca.addIdentity(tt.id, true)
assert.NoError(t, err, "Failed to add new identity")

CAclean(ca, t)
})
}
}

func TestCAAddIdentityFailure(t *testing.T) {
tests := []struct {
testName string
newID *CAConfigIdentity
expectedErr error
}{
{
testName: "When passfile does not exist",
newID: &CAConfigIdentity{
Name: "admin",
Pass: "adminpw",
PassFile: "fakepassfile",
},
expectedErr: errors.New("Failed to read password from 'fakepassfile'"),
},
}

for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
testDirClean(t)

cfg = CAConfig{}
cfg.Registry = CAConfigRegistry{
MaxEnrollments: 10,
}
ca, err := newCA(configFile, &cfg, &srv, false)
assert.NoError(t, err, "Failed creating newCA")

err = ca.addIdentity(tt.newID, true)
assert.Error(t, err, tt.expectedErr)
CAclean(ca, t)
})
}
}

func TestCAaddIdentity(t *testing.T) {
testDirClean(t)
id := &CAConfigIdentity{
Expand Down
1 change: 1 addition & 0 deletions lib/caconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ type CAConfigRegistry struct {
type CAConfigIdentity struct {
Name string `mask:"username"`
Pass string `mask:"password"`
PassFile string
Type string
Affiliation string
MaxEnrollments int
Expand Down