Improvements to revoke client side command

Revocation logic improved to check that revoker has
the necessary affiliation to revoke a user.

Check added to make sure that either the enrollment ID
is provided or both serial and AKI are provided, all three
can't be provided at the same time.

Storing both serial and AKI in database in hex format.
This improves user experience as openssl returns both serial
and AKI in hex format. User's can now directly take the values
and pass into the revoke command without having to do any
manipulation.

Test cases added to test various use cases for revocation

https://jira.hyperledger.org/browse/FAB-2517

Change-Id: Ice68cfcc7b134046a18d2124ec3c3b0b1ae8be68
Signed-off-by: Saad Karim <[email protected]>
Signed-off-by: Keith Smith <[email protected]>

cmd/fabric-ca-client/config.go

@@ -190,7 +190,7 @@ func configInit(command string) error {
 	// Unmarshal the config into 'clientCfg'
 	err = viper.Unmarshal(clientCfg)
 	if err != nil {
-		util.Fatal("Failed to unmarshall client config: %s", err)
+		util.Fatal("Could not parse '%s': %s", cfgFileName, err)
 	}
 
 	purl, err := url.Parse(clientCfg.URL)

cmd/fabric-ca-client/main_test.go

@@ -17,6 +17,11 @@ limitations under the License.
 package main
 
 import (
+	"crypto/x509"
+	"encoding/hex"
+	"encoding/pem"
+	"errors"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path"
@@ -26,6 +31,7 @@ import (
 
 	"github.com/cloudflare/cfssl/csr"
 	"github.com/hyperledger/fabric-ca/lib"
+	"github.com/hyperledger/fabric-ca/lib/dbutil"
 	"github.com/hyperledger/fabric-ca/util"
 )
 
@@ -49,6 +55,7 @@ const (
 var (
 	defYaml    string
 	fabricCADB = path.Join(tdDir, db)
+	srv        *lib.Server
 )
 
 // TestCreateDefaultConfigFile test to make sure default config file gets generated correctly
@@ -85,22 +92,22 @@ func TestCreateDefaultConfigFile(t *testing.T) {
 func TestClientCommandsNoTLS(t *testing.T) {
 	os.Remove(fabricCADB)
 
-	srv := getServer()
+	srv = getServer()
 	srv.HomeDir = tdDir
 	srv.Config.Debug = true
 
-	err := srv.RegisterBootstrapUser("admin", "adminpw", "bank1")
+	err := srv.RegisterBootstrapUser("admin", "adminpw", "banks.bank_a.Dep1")
 	if err != nil {
 		t.Errorf("Failed to register bootstrap user: %s", err)
 	}
 
-	err = srv.RegisterBootstrapUser("admin2", "adminpw2", "bank1")
+	err = srv.RegisterBootstrapUser("admin2", "adminpw2", "banks.bank_a")
 	if err != nil {
 		t.Errorf("Failed to register bootstrap user: %s", err)
 	}
 
 	aff := make(map[string]interface{})
-	aff["banks"] = "bank_a"
+	aff["banks"] = []string{"bank_a", "bank_b", "bank_c"}
 
 	srv.Config.Affiliations = aff
 
@@ -122,27 +129,6 @@ func TestClientCommandsNoTLS(t *testing.T) {
 	}
 }
 
-func getServer() *lib.Server {
-	return &lib.Server{
-		HomeDir: ".",
-		Config:  getServerConfig(),
-	}
-}
-
-func getServerConfig() *lib.ServerConfig {
-	return &lib.ServerConfig{
-		Debug: true,
-		Port:  7054,
-		CA: lib.ServerConfigCA{
-			Keyfile:  keyfile,
-			Certfile: certfile,
-		},
-		CSR: csr.CertificateRequest{
-			CN: "TestCN",
-		},
-	}
-}
-
 // TestEnroll tests fabric-ca-client enroll
 func testEnroll(t *testing.T) {
 	t.Log("Testing Enroll CMD")
@@ -208,7 +194,7 @@ func testRegisterEnvVar(t *testing.T) {
 	defYaml = util.GetDefaultConfigFile("fabric-ca-client")
 
 	os.Setenv("FABRIC_CA_CLIENT_ID_NAME", "testRegister2")
-	os.Setenv("FABRIC_CA_CLIENT_ID_AFFILIATION", "banks.bank_a")
+	os.Setenv("FABRIC_CA_CLIENT_ID_AFFILIATION", "banks.bank_b")
 	os.Setenv("FABRIC_CA_CLIENT_ID_TYPE", "client")
 
 	err := RunMain([]string{cmdName, "register"})
@@ -255,11 +241,55 @@ func testRevoke(t *testing.T) {
 		t.Errorf("No enrollment ID or serial/aki provided, should have failed")
 	}
 
-	err = RunMain([]string{cmdName, "revoke", "-u", "http://localhost:7054", "-e", "admin"})
+	serial, aki, err := getSerialAKIByID("admin")
+	if err != nil {
+		t.Error(err)
+	}
+
+	aki = strings.ToUpper(aki)
+
+	// Revoker's affiliation: banks.bank_a
+	err = RunMain([]string{cmdName, "revoke", "-u", "http://localhost:7054", "-e", "nonexistinguser"})
+	if err == nil {
+		t.Errorf("Non existing user being revoked, should have failed")
+	}
+
+	err = RunMain([]string{cmdName, "revoke", "-u", "http://localhost:7054", "-e", "", "-s", serial})
+	if err == nil {
+		t.Errorf("Only serial specified, should have failed")
+	}
+
+	err = RunMain([]string{cmdName, "revoke", "-u", "http://localhost:7054", "-e", "", "-s", "", "-a", aki})
+	if err == nil {
+		t.Errorf("Only aki specified, should have failed")
+	}
+
+	err = RunMain([]string{cmdName, "revoke", "-u", "http://localhost:7054", "-s", serial, "-a", aki})
+	if err != nil {
+		t.Errorf("client revoke -u -s -a failed: %s", err)
+	}
+
+	serial, aki, err = getSerialAKIByID("testRegister")
+	if err != nil {
+		t.Error(err)
+	}
+
+	// Revoked user's affiliation: banks.bank_c
+	err = RunMain([]string{cmdName, "revoke", "-u", "http://localhost:7054", "-s", serial, "-a", aki})
+	if err != nil {
+		t.Errorf("Revoker does not have the correct affiliation to revoke, should have failed")
+	}
+
+	err = RunMain([]string{cmdName, "revoke", "-u", "http://localhost:7054", "-e", "testRegister3", "-s", "", "-a", ""})
 	if err != nil {
 		t.Errorf("client revoke -u -e failed: %s", err)
 	}
 
+	err = RunMain([]string{cmdName, "revoke", "-u", "http://localhost:7054", "-e", "testRegister2", "-s", "", "-a", ""})
+	if err == nil {
+		t.Errorf("Revoker does not have the correct affiliation to revoke, should have failed")
+	}
+
 	os.Remove(defYaml) // Delete default config file
 
 	err = RunMain([]string{cmdName, "revoke", "-u", "http://localhost:7055"})
@@ -282,7 +312,7 @@ func testBogus(t *testing.T) {
 func TestClientCommandsUsingConfigFile(t *testing.T) {
 	os.Remove(fabricCADB)
 
-	srv := getServer()
+	srv = getServer()
 	srv.Config.Debug = true
 
 	err := srv.RegisterBootstrapUser("admin", "adminpw", "bank1")
@@ -314,7 +344,7 @@ func TestClientCommandsUsingConfigFile(t *testing.T) {
 func TestClientCommandsTLSEnvVar(t *testing.T) {
 	os.Remove(fabricCADB)
 
-	srv := getServer()
+	srv = getServer()
 	srv.Config.Debug = true
 
 	err := srv.RegisterBootstrapUser("admin", "adminpw", "bank1")
@@ -359,7 +389,7 @@ func TestClientCommandsTLSEnvVar(t *testing.T) {
 func TestClientCommandsTLS(t *testing.T) {
 	os.Remove(fabricCADB)
 
-	srv := getServer()
+	srv = getServer()
 	srv.Config.Debug = true
 
 	err := srv.RegisterBootstrapUser("admin", "adminpw", "bank1")
@@ -406,3 +436,45 @@ func TestRegisterWithoutEnroll(t *testing.T) {
 		t.Errorf("Should have failed, as no enrollment information should exist. Enroll commands needs to be the first command to be executed")
 	}
 }
+
+func getServer() *lib.Server {
+	return &lib.Server{
+		HomeDir: ".",
+		Config:  getServerConfig(),
+	}
+}
+
+func getServerConfig() *lib.ServerConfig {
+	return &lib.ServerConfig{
+		Debug: true,
+		Port:  7054,
+		CA: lib.ServerConfigCA{
+			Keyfile:  keyfile,
+			Certfile: certfile,
+		},
+		CSR: csr.CertificateRequest{
+			CN: "TestCN",
+		},
+	}
+}
+
+func getSerialAKIByID(id string) (serial, aki string, err error) {
+	testdb, _, _ := dbutil.NewUserRegistrySQLLite3(srv.Config.DB.Datasource)
+	acc := lib.NewCertDBAccessor(testdb)
+
+	certs, _ := acc.GetCertificatesByID("admin")
+
+	block, _ := pem.Decode([]byte(certs[0].PEM))
+	if block == nil {
+		return "", "", errors.New("Failed to PEM decode certificate")
+	}
+	x509Cert, err := x509.ParseCertificate(block.Bytes)
+	if err != nil {
+		return "", "", fmt.Errorf("Error from x509.ParseCertificate: %s", err)
+	}
+
+	serial = util.GetSerialAsHex(x509Cert.SerialNumber)
+	aki = hex.EncodeToString(x509Cert.AuthorityKeyId)
+
+	return
+}

cmd/fabric-ca-client/revoke.go

@@ -17,7 +17,7 @@ limitations under the License.
 package main
 
 import (
-	"fmt"
+	"errors"
 	"path/filepath"
 
 	"github.com/cloudflare/cfssl/log"
@@ -28,6 +28,8 @@ import (
 	"github.com/spf13/viper"
 )
 
+var errInput = errors.New("Invalid usage; either --eid or both --serial and --aki are required")
+
 // initCmd represents the init command
 var revokeCmd = &cobra.Command{
 	Use:   "revoke",
@@ -72,7 +74,10 @@ func runRevoke() error {
 	log.Debug("Revoke Entered")
 
 	var err error
+
 	enrollmentID := viper.GetString("eid")
+	serial := viper.GetString("serial")
+	aki := viper.GetString("aki")
 
 	client := lib.Client{
 		HomeDir: filepath.Dir(cfgFileName),
@@ -84,18 +89,28 @@ func runRevoke() error {
 		return err
 	}
 
-	serial := viper.GetString("serial")
-	aki := viper.GetString("aki")
+	if enrollmentID == "" {
+		if serial == "" || aki == "" {
+			return errInput
+		}
+	} else {
+		if serial != "" || aki != "" {
+			return errInput
+		}
+	}
 
-	if enrollmentID == "" && serial == "" {
-		return fmt.Errorf("Invalid usage; either --eid or both --serial and --aki are required")
+	reasonInput := viper.GetString("reason")
+	var reason int
+	if reasonInput != "" {
+		reason = util.RevocationReasonCodes[reasonInput]
 	}
 
 	err = id.Revoke(
 		&api.RevocationRequest{
 			Name:   enrollmentID,
 			Serial: serial,
 			AKI:    aki,
+			Reason: reason,
 		})
 
 	if err == nil {

cmd/fabric-ca-server/config.go

@@ -144,13 +144,12 @@ registry:
 #  To run the fabric-ca-server in a cluster, you must choose "postgres"
 #  or "mysql".
 #############################################################################
-database:
+db:
   type: sqlite3
   datasource: fabric-ca-server.db
   tls:
       enabled: false
-      certfiles:
-        - db-server-cert.pem
+      certfiles: db-server-cert.pem			# Comma Separated (e.g. root.pem, root2.pem)
       client:
         certfile: db-client-cert.pem
         keyfile: db-client-key.pem
@@ -168,8 +167,7 @@ ldap:
    # The URL of the LDAP server
    url: ldap://<adminDN>:<adminPassword>@<host>:<port>/<base>
    tls:
-      certfiles:
-        - ldap-server-cert.pem
+      certfiles: ldap-server-cert.pem				# Comma Separated (e.g. root.pem, root2.pem)
       client:
          certfile: ldap-client-cert.pem
          keyfile: ldap-client-key.pem