Skip to content

Commit

Permalink
[FAB-5035] Limit searchKeystoreForSKI to 64k files
Browse files Browse the repository at this point in the history
Software keystore is trying to be helpful, and allow arbitrary names
for the keys (this was so that one could, for example, create pem
files with openssl, and point bccsp at the directory).

Though at the time helpful (cryptogen did not exist), this is is a
dangerous behaviour, bccsp/sw/fileks.searchKeystoreForSKI()
will open every file in the directory and run PEM decoder on the
file.

If we indend to keep searchKeystoreForSKI() behaviour, lets add a
file size check so that we dont end up reading in huge files
(like a core dump..) and trying to see if its a PEM file.

Change-Id: Ic7611eb2a3d4a1ab7d87dfbafb1634db770a4e24
Signed-off-by: Volodymyr Paprotski <vpaprots@ca.ibm.com>
  • Loading branch information
Volodymyr Paprotski committed Feb 8, 2018
1 parent 466e6ac commit 39fba9e
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
7 changes: 6 additions & 1 deletion bccsp/sw/fileks.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@ func (ks *fileBasedKeyStore) searchKeystoreForSKI(ski []byte) (k bccsp.Key, err
if f.IsDir() {
continue
}

if f.Size() > (1 << 16) { //64k, somewhat arbitrary limit, considering even large RSA keys
continue
}

raw, err := ioutil.ReadFile(filepath.Join(ks.path, f.Name()))
if err != nil {
continue
Expand All @@ -254,7 +259,7 @@ func (ks *fileBasedKeyStore) searchKeystoreForSKI(ski []byte) (k bccsp.Key, err

return k, nil
}
return nil, errors.New("Key type not recognized")
return nil, fmt.Errorf("Key with SKI %s not found in %s", hex.EncodeToString(ski), ks.path)
}

func (ks *fileBasedKeyStore) getSuffix(alias string) string {
Expand Down
45 changes: 45 additions & 0 deletions bccsp/sw/fileks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,19 @@ limitations under the License.
package sw

import (
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
"encoding/hex"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"

"github.com/hyperledger/fabric/bccsp/utils"
)

func TestInvalidStoreKey(t *testing.T) {
Expand Down Expand Up @@ -73,3 +79,42 @@ func TestInvalidStoreKey(t *testing.T) {
t.Fatal("Error should be different from nil in this case")
}
}

func TestBigKeyFile(t *testing.T) {
ksPath, err := ioutil.TempDir("", "bccspks")
assert.NoError(t, err)
defer os.RemoveAll(ksPath)

ks, err := NewFileBasedKeyStore(nil, ksPath, false)
assert.NoError(t, err)

// Generate a key for keystore to find
privKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
assert.NoError(t, err)

cspKey := &ecdsaPrivateKey{privKey}
ski := cspKey.SKI()
rawKey, err := utils.PrivateKeyToPEM(privKey, nil)
assert.NoError(t, err)

// Large padding array, of some values PEM parser will NOOP
bigBuff := make([]byte, (1 << 17))
for i := range bigBuff {
bigBuff[i] = '\n'
}
copy(bigBuff, rawKey)

//>64k, so that total file size will be too big
ioutil.WriteFile(filepath.Join(ksPath, "bigfile.pem"), bigBuff, 0666)

_, err = ks.GetKey(ski)
assert.Error(t, err)
expected := fmt.Sprintf("Key with SKI %s not found in %s", hex.EncodeToString(ski), ksPath)
assert.EqualError(t, err, expected)

// 1k, so that the key would be found
ioutil.WriteFile(filepath.Join(ksPath, "smallerfile.pem"), bigBuff[0:1<<10], 0666)

_, err = ks.GetKey(ski)
assert.NoError(t, err)
}

0 comments on commit 39fba9e

Please sign in to comment.