From 39fba9e084dc6734426ba5e44dc5b6c8e31747ee Mon Sep 17 00:00:00 2001
From: Volodymyr Paprotski <vpaprots@ca.ibm.com>
Date: Wed, 7 Feb 2018 15:17:10 -0500
Subject: [PATCH] [FAB-5035] Limit searchKeystoreForSKI to 64k files

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>
---
 bccsp/sw/fileks.go      |  7 ++++++-
 bccsp/sw/fileks_test.go | 45 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/bccsp/sw/fileks.go b/bccsp/sw/fileks.go
index de84d1c5ff9..1ca6a7c6002 100644
--- a/bccsp/sw/fileks.go
+++ b/bccsp/sw/fileks.go
@@ -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
@@ -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 {
diff --git a/bccsp/sw/fileks_test.go b/bccsp/sw/fileks_test.go
index 7f9f6224af2..6adf9d436e5 100644
--- a/bccsp/sw/fileks_test.go
+++ b/bccsp/sw/fileks_test.go
@@ -16,6 +16,10 @@ limitations under the License.
 package sw
 
 import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"encoding/hex"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -23,6 +27,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"github.com/hyperledger/fabric/bccsp/utils"
 )
 
 func TestInvalidStoreKey(t *testing.T) {
@@ -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)
+}