Skip to content

Commit b6e1f91

Browse files
author
Volodymyr Paprotski
committed
[FAB-3454] BCCSP Factory Options error checks
- (Reported) factory now can 'nicely' fail on bad bccsp names - (Part of debugging this problem) had test cases modifying default options. Hide default options behind a getter which returns a new instance every time, so that it does not get clobbered - (Part of debugging this problem) SetupBCCSPKeystoreConfig should did not update bccsp config outside the function Change-Id: I397a9e12908f5550ffc54e5277d950bf6428b571 Signed-off-by: Volodymyr Paprotski <vpaprots@ca.ibm.com>
1 parent a959653 commit b6e1f91

File tree

11 files changed

+68
-29
lines changed

11 files changed

+68
-29
lines changed

‎bccsp/factory/factory.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ func GetDefault() bccsp.BCCSP {
6262
bootBCCSPInitOnce.Do(func() {
6363
var err error
6464
f := &SWFactory{}
65-
bootBCCSP, err = f.Get(&DefaultOpts)
65+
bootBCCSP, err = f.Get(GetDefaultOpts())
6666
if err != nil {
67-
panic("BCCSP Internal error, failed initialization with DefaultOpts!")
67+
panic("BCCSP Internal error, failed initialization with GetDefaultOpts!")
6868
}
6969
})
7070
return bootBCCSP
@@ -74,7 +74,11 @@ func GetDefault() bccsp.BCCSP {
7474

7575
// GetBCCSP returns a BCCSP created according to the options passed in input.
7676
func GetBCCSP(name string) (bccsp.BCCSP, error) {
77-
return bccspMap[name], nil
77+
csp, ok := bccspMap[name]
78+
if !ok {
79+
return nil, fmt.Errorf("Could not find BCCSP, no '%s' provider", name)
80+
}
81+
return csp, nil
7882
}
7983

8084
func initBCCSP(f BCCSPFactory, config *FactoryOpts) error {

‎bccsp/factory/factory_test.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
"github.com/hyperledger/fabric/bccsp/pkcs11"
2727
"github.com/spf13/viper"
28+
"github.com/stretchr/testify/assert"
2829
)
2930

3031
func TestMain(m *testing.M) {
@@ -115,10 +116,11 @@ func TestGetDefault(t *testing.T) {
115116

116117
func TestGetBCCSP(t *testing.T) {
117118
bccsp, err := GetBCCSP("SW")
118-
if err != nil {
119-
t.Fatalf("Failed getting default BCCSP [%s]", err)
120-
}
121-
if bccsp == nil {
122-
t.Fatal("Failed Software BCCSP. Nil instance.")
123-
}
119+
assert.NoError(t, err)
120+
assert.NotNil(t, bccsp)
121+
122+
bccsp, err = GetBCCSP("BadName")
123+
assert.Error(t, err)
124+
assert.Contains(t, err.Error(), "Could not find BCCSP, no 'BadName' provider")
125+
assert.Nil(t, bccsp)
124126
}

‎bccsp/factory/nopkcs11.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ func InitFactories(config *FactoryOpts) error {
3636
factoriesInitOnce.Do(func() {
3737
// Take some precautions on default opts
3838
if config == nil {
39-
config = &DefaultOpts
39+
config = GetDefaultOpts()
4040
}
4141

4242
if config.ProviderName == "" {
4343
config.ProviderName = "SW"
4444
}
4545

4646
if config.SwOpts == nil {
47-
config.SwOpts = DefaultOpts.SwOpts
47+
config.SwOpts = GetDefaultOpts().SwOpts
4848
}
4949

5050
// Initialize factories map
@@ -75,6 +75,8 @@ func GetBCCSPFromOpts(config *FactoryOpts) (bccsp.BCCSP, error) {
7575
switch config.ProviderName {
7676
case "SW":
7777
f = &SWFactory{}
78+
default:
79+
return nil, fmt.Errorf("Could not find BCCSP, no '%s' provider", config.ProviderName)
7880
}
7981

8082
csp, err := f.Get(config)

‎bccsp/factory/opts.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,18 @@ limitations under the License.
1515
*/
1616
package factory
1717

18-
// DefaultOpts offers a default implementation for Opts
19-
var DefaultOpts = FactoryOpts{
20-
ProviderName: "SW",
21-
SwOpts: &SwOpts{
22-
HashFamily: "SHA2",
23-
SecLevel: 256,
18+
// GetDefaultOpts offers a default implementation for Opts
19+
// returns a new instance every time
20+
func GetDefaultOpts() *FactoryOpts {
21+
return &FactoryOpts{
22+
ProviderName: "SW",
23+
SwOpts: &SwOpts{
24+
HashFamily: "SHA2",
25+
SecLevel: 256,
2426

25-
Ephemeral: true,
26-
},
27+
Ephemeral: true,
28+
},
29+
}
2730
}
2831

2932
// FactoryName returns the name of the provider

‎bccsp/factory/opts_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@ import (
2222
)
2323

2424
func TestFactoryOptsFactoryName(t *testing.T) {
25-
assert.Equal(t, DefaultOpts.FactoryName(), "SW")
25+
assert.Equal(t, GetDefaultOpts().FactoryName(), "SW")
2626
}

‎bccsp/factory/pkcs11.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,15 @@ func InitFactories(config *FactoryOpts) error {
4545
func setFactories(config *FactoryOpts) error {
4646
// Take some precautions on default opts
4747
if config == nil {
48-
config = &DefaultOpts
48+
config = GetDefaultOpts()
4949
}
5050

5151
if config.ProviderName == "" {
5252
config.ProviderName = "SW"
5353
}
5454

5555
if config.SwOpts == nil {
56-
config.SwOpts = DefaultOpts.SwOpts
56+
config.SwOpts = GetDefaultOpts().SwOpts
5757
}
5858

5959
// Initialize factories map
@@ -94,6 +94,8 @@ func GetBCCSPFromOpts(config *FactoryOpts) (bccsp.BCCSP, error) {
9494
f = &SWFactory{}
9595
case "PKCS11":
9696
f = &PKCS11Factory{}
97+
default:
98+
return nil, fmt.Errorf("Could not find BCCSP, no '%s' provider", config.ProviderName)
9799
}
98100

99101
csp, err := f.Get(config)

‎bccsp/factory/pkcs11_test.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ import (
2424
)
2525

2626
func TestInitFactories(t *testing.T) {
27+
// Reset errors from previous negative test runs
28+
factoriesInitError = nil
29+
2730
err := InitFactories(nil)
2831
assert.NoError(t, err)
2932
}
@@ -53,21 +56,32 @@ func TestSetFactoriesInvalidArgs(t *testing.T) {
5356
}
5457

5558
func TestGetBCCSPFromOpts(t *testing.T) {
56-
opts := &DefaultOpts
59+
opts := GetDefaultOpts()
5760
opts.SwOpts.FileKeystore = &FileKeystoreOpts{KeyStorePath: os.TempDir()}
5861
opts.SwOpts.Ephemeral = false
5962
csp, err := GetBCCSPFromOpts(opts)
6063
assert.NoError(t, err)
6164
assert.NotNil(t, csp)
6265

66+
lib, pin, label := pkcs11.FindPKCS11Lib()
6367
csp, err = GetBCCSPFromOpts(&FactoryOpts{
6468
ProviderName: "PKCS11",
6569
Pkcs11Opts: &pkcs11.PKCS11Opts{
6670
SecLevel: 256,
6771
HashFamily: "SHA2",
6872
Ephemeral: true,
73+
Library: lib,
74+
Pin: pin,
75+
Label: label,
6976
},
7077
})
7178
assert.NoError(t, err)
7279
assert.NotNil(t, csp)
80+
81+
csp, err = GetBCCSPFromOpts(&FactoryOpts{
82+
ProviderName: "BadName",
83+
})
84+
assert.Error(t, err)
85+
assert.Contains(t, err.Error(), "Could not find BCCSP, no 'BadName' provider")
86+
assert.Nil(t, csp)
7387
}

‎bccsp/factory/pkcs11factory_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,15 @@ func TestPKCS11FactoryGetInvalidArgs(t *testing.T) {
4646

4747
func TestPKCS11FactoryGet(t *testing.T) {
4848
f := &PKCS11Factory{}
49+
lib, pin, label := pkcs11.FindPKCS11Lib()
4950

5051
opts := &FactoryOpts{
5152
Pkcs11Opts: &pkcs11.PKCS11Opts{
5253
SecLevel: 256,
5354
HashFamily: "SHA2",
55+
Library: lib,
56+
Pin: pin,
57+
Label: label,
5458
},
5559
}
5660
csp, err := f.Get(opts)
@@ -62,6 +66,9 @@ func TestPKCS11FactoryGet(t *testing.T) {
6266
SecLevel: 256,
6367
HashFamily: "SHA2",
6468
FileKeystore: &pkcs11.FileKeystoreOpts{KeyStorePath: os.TempDir()},
69+
Library: lib,
70+
Pin: pin,
71+
Label: label,
6572
},
6673
}
6774
csp, err = f.Get(opts)
@@ -73,6 +80,9 @@ func TestPKCS11FactoryGet(t *testing.T) {
7380
SecLevel: 256,
7481
HashFamily: "SHA2",
7582
Ephemeral: true,
83+
Library: lib,
84+
Pin: pin,
85+
Label: label,
7686
},
7787
}
7888
csp, err = f.Get(opts)

‎core/chaincode/shim/chaincode.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func Start(cc Chaincode) error {
118118
return fmt.Errorf("Error chaincode id not provided")
119119
}
120120

121-
err := factory.InitFactories(&factory.DefaultOpts)
121+
err := factory.InitFactories(factory.GetDefaultOpts())
122122
if err != nil {
123123
return fmt.Errorf("Internal error, BCCSP could not be initialized with default options: %s", err)
124124
}

‎msp/configbuilder.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,14 @@ const (
109109
configfilename = "config.yaml"
110110
)
111111

112-
func SetupBCCSPKeystoreConfig(bccspConfig *factory.FactoryOpts, keystoreDir string) {
112+
func SetupBCCSPKeystoreConfig(bccspConfig *factory.FactoryOpts, keystoreDir string) *factory.FactoryOpts {
113113
if bccspConfig == nil {
114-
bccspConfig = &factory.DefaultOpts
114+
bccspConfig = factory.GetDefaultOpts()
115115
}
116116

117117
if bccspConfig.ProviderName == "SW" {
118118
if bccspConfig.SwOpts == nil {
119-
bccspConfig.SwOpts = factory.DefaultOpts.SwOpts
119+
bccspConfig.SwOpts = factory.GetDefaultOpts().SwOpts
120120
}
121121

122122
// Only override the KeyStorePath if it was left empty
@@ -126,12 +126,14 @@ func SetupBCCSPKeystoreConfig(bccspConfig *factory.FactoryOpts, keystoreDir stri
126126
bccspConfig.SwOpts.FileKeystore = &factory.FileKeystoreOpts{KeyStorePath: keystoreDir}
127127
}
128128
}
129+
130+
return bccspConfig
129131
}
130132

131133
func GetLocalMspConfig(dir string, bccspConfig *factory.FactoryOpts, ID string) (*msp.MSPConfig, error) {
132134
signcertDir := filepath.Join(dir, signcerts)
133135
keystoreDir := filepath.Join(dir, keystore)
134-
SetupBCCSPKeystoreConfig(bccspConfig, keystoreDir)
136+
bccspConfig = SetupBCCSPKeystoreConfig(bccspConfig, keystoreDir)
135137

136138
err := factory.InitFactories(bccspConfig)
137139
if err != nil {

0 commit comments

Comments
 (0)