Skip to content

Commit

Permalink
[FAB-4826] Token-based auth issue to int server
Browse files Browse the repository at this point in the history
There are two 1 line changes in lib/ca.go in this change set as follows:
1) Change "for" to "if" on this line: "if len(rest) > 0 {"
   This is the main fix to prevent an infinite loop in token-based authentication
   against an intermediate server with a cert chain that has multiple certs.
2) ca.Config.CA.Chainfile = chainPath
   This line is added to set the default value of the chainpath file.
   This is needed for test cases which don't use the config file, or if someone
   removes the default value for "ca.chainfile" from the config file.
This also includes appropriate test case changes.

Change-Id: I21497091ccedb6f7ba78e2a14a8ba4d9b9d238a2
Signed-off-by: Keith Smith <bksmith@us.ibm.com>
  • Loading branch information
Keith Smith committed Jun 16, 2017
1 parent 5180ce7 commit 313d945
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 7 deletions.
3 changes: 2 additions & 1 deletion lib/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ func (ca *CA) getCACert() (cert []byte, err error) {
if err != nil {
return nil, fmt.Errorf("Failed to create intermediate chain file path: %s", err)
}
ca.Config.CA.Chainfile = chainPath
}
chain := ca.concatChain(resp.ServerInfo.CAChain, cert)
err = os.MkdirAll(path.Dir(chainPath), 0755)
Expand Down Expand Up @@ -448,7 +449,7 @@ func (ca *CA) getVerifyOptions() (*x509.VerifyOptions, error) {
rootPool := x509.NewCertPool()
rootPool.AddCert(rootCert)
var intPool *x509.CertPool
for len(rest) > 0 {
if len(rest) > 0 {
intPool = x509.NewCertPool()
if !intPool.AppendCertsFromPEM(rest) {
return nil, errors.New("Failed to add intermediate PEM certificates")
Expand Down
11 changes: 7 additions & 4 deletions lib/client_whitebox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,14 @@ func TestTLSClientAuth(t *testing.T) {
if err != nil {
t.Fatalf("Failed to start server: %s", err)
}
defer server.Stop()
// Enroll over HTTP
client := &Client{
Config: &ClientConfig{URL: fmt.Sprintf("http://localhost:%d", whitePort)},
HomeDir: path.Join(testTLSClientAuthDir, "client"),
}
eresp, err := client.Enroll(&api.EnrollmentRequest{Name: user, Secret: pass})
if err != nil {
server.Stop()
t.Fatalf("Failed to enroll admin: %s", err)
}
id := eresp.Identity
Expand All @@ -112,20 +112,22 @@ func TestTLSClientAuth(t *testing.T) {
// Try to reenroll over HTTP and it should fail because server is listening on HTTPS
_, err = id.Reenroll(&api.ReenrollmentRequest{})
if err == nil {
t.Fatal("Client HTTP should have failed to reenroll with server HTTPS")
t.Error("Client HTTP should have failed to reenroll with server HTTPS")
}
// Reenroll over HTTPS
client.Config.URL = fmt.Sprintf("https://localhost:%d", whitePort)
client.Config.TLS.Enabled = true
client.Config.TLS.CertFiles = []string{"../server/ca-cert.pem"}
resp, err := id.Reenroll(&api.ReenrollmentRequest{})
if err != nil {
server.Stop()
t.Fatalf("Failed to reenroll over HTTPS: %s", err)
}
id = resp.Identity
// Store identity persistently
err = id.Store()
if err != nil {
server.Stop()
t.Fatalf("Failed to store identity: %s", err)
}
// Stop server
Expand All @@ -146,13 +148,14 @@ func TestTLSClientAuth(t *testing.T) {
// Try to reenroll and it should fail because client has no client cert
_, err = id.Reenroll(&api.ReenrollmentRequest{})
if err == nil {
t.Fatal("Client reenroll without client cert should have failed")
t.Error("Client reenroll without client cert should have failed")
}
// Reenroll over HTTPS with client auth
client.Config.TLS.Client.CertFile = path.Join("msp", "signcerts", "cert.pem")
_, err = id.Reenroll(&api.ReenrollmentRequest{})
if err != nil {
t.Fatalf("Client reenroll with client auth failed: %s", err)
server.Stop()
t.Errorf("Client reenroll with client auth failed: %s", err)
}
// Stop server
err = server.Stop()
Expand Down
11 changes: 11 additions & 0 deletions lib/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,17 @@ func testIntermediateServer(idx int, t *testing.T) {
if err != nil {
t.Fatalf("Intermediate server start failed: %s", err)
}
// Test enroll against intermediate (covering basic auth)
c := getTestClient(intermediateServer.Config.Port)
resp, err := c.Enroll(&api.EnrollmentRequest{Name: "admin", Secret: "adminpw"})
if err != nil {
t.Fatalf("Failed to enroll with intermediate server: %s", err)
}
// Test reenroll against intermediate (covering token auth)
_, err = resp.Identity.Reenroll(&api.ReenrollmentRequest{})
if err != nil {
t.Fatalf("Failed to reenroll with intermediate server: %s", err)
}
// Stop it
intermediateServer.Stop()
}
Expand Down
4 changes: 2 additions & 2 deletions scripts/fvt/intermediateca_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ mkdir -p $TDIR/root
cd $TDIR/root
fabric-ca-server start -b admin:adminpw -d > server.log 2>&1&
cd ../..
sleep 1
sleep 3

mkdir -p $TDIR/int1
cd $TDIR/int1
fabric-ca-server start -b admin:adminpw -u http://admin:adminpw@localhost:7054 -p 7055 -d > server.log 2>&1&
cd ../..
sleep 1
sleep 3

fabric-ca-client getcacert -u http://admin:adminpw@localhost:7055
test $? -ne 0 && ErrorExit "Failed to talk to intermediate CA1"
Expand Down

0 comments on commit 313d945

Please sign in to comment.