[FAB-3026] OOM for very large CRLs

A certificate containing a CRL URI that points
to an extremely large file causes the server
to crash with an out of memory exception.

A config option (CRLSizeLimit) has been added
to check and make sure that the requested CRL
does not exceed the size specified by CRLSizeLimit.
The default size limit is 512KB.

This will prevent a malicious intent to crash
server by pointing to a CRL that is very large.

See [FAB-3026] for more information

Change-Id: Ibbb0506faecf29b9a9c0a361c2ff701c9945a973
Signed-off-by: Saad Karim <[email protected]>

cmd/fabric-ca-server/config.go

@@ -84,6 +84,9 @@ port: 7054
 # Enables debug logging (default: false)
 debug: false
 
+# Size limit of an acceptable CRL in bytes (default: 512000)
+crlsizelimit: 512000
+
 #############################################################################
 #  TLS section for the server's listening port
 #

docs/source/users-guide.rst

@@ -256,6 +256,7 @@ The following shows the Fabric CA server usage message.
       --cacount int                               Number of non-default CA instances
       --cafiles stringSlice                       A list of comma-separated CA configuration files
   -c, --config string                             Configuration file (default "fabric-ca-server-config.yaml")
+      --crlsizelimit int                          Size limit of an acceptable CRL in bytes (default 512000)
       --csr.cn string                             The common name field of the certificate signing request to a parent fabric-ca-server
       --csr.hosts stringSlice                     A list of comma-separated host names in a certificate signing request to a parent fabric-ca-server
       --db.datasource string                      Data source which is database specific (default "fabric-ca-server.db")
@@ -360,6 +361,9 @@ the server's home directory (see `Fabric CA Server <#server>`__ section more inf
     # Enables debug logging (default: false)
     debug: false
 
+    # Size limit of an acceptable CRL in bytes (default: 512000)
+    crlsizelimit: 512000
+
     #############################################################################
     #  TLS section for the server's listening port
     #

lib/caconfig.go

@@ -117,7 +117,7 @@ type CAConfigIdentity struct {
 // the server to connect to
 type ParentServer struct {
 	URL    string `opt:"u" help:"URL of the parent fabric-ca-server (e.g. http://<username>:<password>@<address>:<port)"`
-	CAName string `help:"Name of the CA to connect to on fabric-ca-serve"`
+	CAName string `help:"Name of the CA to connect to on fabric-ca-server"`
 }
 
 // IntermediateCA contains parent server information, TLS configuration, and

lib/server.go

@@ -22,6 +22,7 @@ import (
 	"crypto/x509/pkix"
 	"errors"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net"
 	"net/http"
@@ -34,6 +35,7 @@ import (
 	"time"
 
 	"github.com/cloudflare/cfssl/log"
+	"github.com/cloudflare/cfssl/revoke"
 	stls "github.com/hyperledger/fabric-ca/lib/tls"
 	"github.com/hyperledger/fabric-ca/util"
 	"github.com/spf13/viper"
@@ -222,9 +224,9 @@ func (s *Server) initConfig() (err error) {
 	if err != nil {
 		return err
 	}
+	revoke.SetCRLFetcher(s.fetchCRL)
 	// Make file names absolute
 	s.makeFileNamesAbsolute()
-	// Create empty CA map
 	return nil
 }
 
@@ -605,6 +607,21 @@ func (s *Server) compareDN(existingCACertFile, newCACertFile string) error {
 	return nil
 }
 
+// Read the CRL from body of http response
+func (s *Server) fetchCRL(r io.Reader) ([]byte, error) {
+	crlSizeLimit := s.Config.CRLSizeLimit
+	log.Debugf("CRL size limit is %d bytes", crlSizeLimit)
+
+	crl := make([]byte, crlSizeLimit)
+
+	crl, err := util.Read(r, crl)
+	if err != nil {
+		return nil, fmt.Errorf("Error reading CRL with max buffer size of %d: %s", crlSizeLimit, err)
+	}
+
+	return crl, nil
+}
+
 func (s *Server) loadDNFromCertFile(certFile string) (*DN, error) {
 	log.Debugf("Loading DNs from certificate %s", certFile)
 	cert, err := util.GetX509CertificateFromPEMFile(certFile)

lib/serverauth.go

@@ -144,22 +144,15 @@ func (ah *fcaAuthHandler) serveHTTP(w http.ResponseWriter, r *http.Request) erro
 	case token:
 
 		ca := ah.server.caMap[req.CAName]
-
 		// verify token
 		cert, err2 := util.VerifyToken(ca.csp, authHdr, body)
 		if err2 != nil {
 			log.Debugf("Failed to verify token: %s", err2)
 			return authError
 		}
-		// Make sure the caller's cert was issued by this CA
-		err2 = ca.VerifyCertificate(cert)
-		if err2 != nil {
-			log.Debugf("Failed to verify certificate: %s", err2)
-			return authError
-		}
+
 		id := util.GetEnrollmentIDFromX509Certificate(cert)
 		log.Debugf("Checking for revocation/expiration of certificate owned by '%s'", id)
-
 		// VerifyCertificate ensures that the certificate passed in hasn't
 		// expired and checks the CRL for the server.
 		revokedOrExpired, checked := revoke.VerifyCertificate(cert)
@@ -172,6 +165,13 @@ func (ah *fcaAuthHandler) serveHTTP(w http.ResponseWriter, r *http.Request) erro
 			return authError
 		}
 
+		// Make sure the caller's cert was issued by this CA
+		err2 = ca.VerifyCertificate(cert)
+		if err2 != nil {
+			log.Debugf("Failed to verify certificate: %s", err2)
+			return authError
+		}
+
 		aki := hex.EncodeToString(cert.AuthorityKeyId)
 		serial := util.GetSerialAsHex(cert.SerialNumber)
 

lib/serverconfig.go

@@ -53,4 +53,6 @@ type ServerConfig struct {
 	// The number of non-default CAs, which is useful for a dev environment to
 	// quickly start any number of CAs in a single server
 	CAcount int `def:"0" help:"Number of non-default CA instances"`
+	// Size limit of an acceptable CRL in bytes
+	CRLSizeLimit int `def:"512000" help:"Size limit of an acceptable CRL in bytes"`
 }

scripts/fvt/cdp_exploit_test.sh

@@ -0,0 +1,58 @@
+#!/bin/bash
+checkMsg() {
+awk -v rc=-1 -v m="$1" '$0~m {rc=0} ; {print}; END {exit rc}'
+}
+rm -rf /tmp/oom/
+RC=0
+httpPort=3755
+export CA_CFG_PATH=/tmp/oom
+export FABRIC_CA_CLIENT_HOME=/tmp/oom/admin
+FABRIC_CA="$GOPATH/src/github.com/hyperledger/fabric-ca"
+SCRIPTDIR="$FABRIC_CA/scripts/fvt"
+. $SCRIPTDIR/fabric-ca_utils
+mkdir -p /FVT/crl/
+cp $GOPATH/src/github.com/hyperledger/fabric-ca/testdata/crl.pem /FVT/crl/crl.pem
+$SCRIPTDIR/utils/pki -f newcert -p admin -t ec -l 256 -n '/C=US/CN=admin/ST=North Carolina/O=Hyperledger/OU=Fabric/CN=admin/' -x <<EOF
+127.0.0.2
+
+admin.fabric.raleigh.ibm.com
+[email protected]
+Y
+EOF
+
+# Start the default server and check to see if CRL retrieval works if size limit is appropriate
+# However, register will fail because we used a 'hacked' enrollment certificate
+$SCRIPTDIR/fabric-ca_setup.sh -I -X -S -D
+enroll
+admin_keyfile="$(find /tmp/oom/admin/msp/keystore -type f)"
+admin_certfile="/tmp/oom/admin/msp/signcerts/cert.pem"
+hacker_admin_keyfile="/root/adminkey.pem"
+hacker_admin_certfile="/root/admincert.pem"
+cp $hacker_admin_keyfile $admin_keyfile
+cp $hacker_admin_certfile $admin_certfile
+cd /
+python -m SimpleHTTPServer $httpPort &
+pollServer httpserver 127.0.0.1 3755 10
+register admin user1 2>&1 | checkMsg "Error: Error response from server was: Authorization failure"
+test $? -ne 0 && ErrorMsg "Failed to return correct error"
+
+# Lower the CRL size limit on server and check to see that server does not continue to proceed with retrieving CRL list
+export FABRIC_CA_SERVER_CRLSIZELIMIT=10
+cd $GOPATH/src/github.com/hyperledger/fabric-ca
+$SCRIPTDIR/fabric-ca_setup.sh -K
+$SCRIPTDIR/fabric-ca_setup.sh -S -D -X > /tmp/log.txt 2>&1
+enroll admin2 adminpw2
+admin_keyfile="$(find /tmp/oom/admin/msp/keystore -type f)"
+admin_certfile="/tmp/oom/admin/msp/signcerts/cert.pem"
+hacker_admin_keyfile="/root/adminkey.pem"
+hacker_admin_certfile="/root/admincert.pem"
+cp $hacker_admin_keyfile $admin_keyfile
+cp $hacker_admin_certfile $admin_certfile
+register admin user2 client bank_a "" /tmp/oom/admin2 2>&1 | checkMsg "Error: Error response from server was: Authorization failure"
+test $? -ne 0 && ErrorMsg "Failed to return correct error"
+grep "failed to fetch CRL: Error reading CRL with max buffer size of 10: Size of requested data is too large" /tmp/log.txt &> /dev/null
+if [ $? != 0 ]; then
+   ErrorMsg "Client authentication failed for other reason than CRL size"
+fi
+CleanUp $RC
+exit $RC

scripts/fvt/fabric-ca_setup.sh

@@ -204,7 +204,7 @@ function startFabricCa() {
 
    inst=0
    $FABRIC_CA_SERVEREXEC start --address $server_addr $server_port --ca.certfile $DST_CERT \
-                     --ca.keyfile $DST_KEY --config $RUNCONFIG 2>&1 | sed 's/^/     /' &
+                     --ca.keyfile $DST_KEY --config $RUNCONFIG 2>&1 &
                     # --db.datasource $DATASRC --ca.keyfile $DST_KEY --config $RUNCONFIG 2>&1 | sed 's/^/     /' &
    until test "$started" = "$server_addr:${USER_CA_PORT-$CA_DEFAULT_PORT}" -o "$now" -gt "$timeout"; do
       started=$(ss -ltnp src $server_addr:${USER_CA_PORT-$CA_DEFAULT_PORT} | awk 'NR!=1 {print $4}')

testdata/crl.pem

@@ -0,0 +1,12 @@
+-----BEGIN X509 CRL-----
+MIIBxTCBrgIBATANBgkqhkiG9w0BAQsFADBrMRMwEQYKCZImiZPyLGQBGRYDb3Jn
+MRcwFQYKCZImiZPyLGQBGRYHY2lsb2dvbjELMAkGA1UEBhMCVVMxEDAOBgNVBAoT
+B0NJTG9nb24xHDAaBgNVBAMTE0NJTG9nb24gU2lsdmVyIENBIDEXDTE3MDcyMDA4
+NDcwNFoXDTE3MDgxOTA4NDcwNFqgDzANMAsGA1UdFAQEAgIWJTANBgkqhkiG9w0B
+AQsFAAOCAQEAqvzbH1bkrk/mfPAzZEIODN6RMTLe+xQvfZNMAyuY+1ZTXVLq7DjZ
+Ya+wYcw99R4Uw6238ET37CnYVBvSt5MFFM4sWF/fPtnLFjlHG4T8l2ie7GjTihzG
+ckaG1tfa6DxpdK2a7lQxu9ITobpZvxs5oUDw0YEl+D5bjYOcdZCAh+A2Kf1MJugL
+ylL9U7B4rC+MljcPar8Akf9FCQQardnhB/piMhnzqE164OEHhWLgwuL2BUJIyYTq
+c25kJ6a1aioX6ifFK53r6NqkVJFo0563zv+6UFA6FWL932kRYQI59kld/rHzc5ZC
+MkOd81zvNXrRP+zOPw0qmFhyuBS7qB1R8g==
+-----END X509 CRL-----

testdata/openssl.cnf.base

@@ -289,8 +289,8 @@ fullname=URI:http://localhost:$HTTP_PORT/$DOMAIN/crl/crl.der
 #onlysomereasons=$IDPREASON
 
 [ cdp_section ]
-fullname=URI:http://localhost:$HTTP_PORT/$DOMAIN/crl/crl.der
-# revocation reason, where reason is one of:
+fullname=URI:http://localhost:$HTTP_PORT/$DOMAIN/crl/crl.pem
+# revocation reason, where reason is one of: 
 #    unspecified
 #    keyCompromise
 #    CACompromise

util/util.go

@@ -27,6 +27,7 @@ import (
 	"encoding/pem"
 	"errors"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"math/big"
 	mrand "math/rand"
@@ -666,3 +667,24 @@ func CheckHostsInCert(certFile string, host string) error {
 
 	return nil
 }
+
+// Read reads from Reader into a byte array
+func Read(r io.Reader, data []byte) ([]byte, error) {
+	j := 0
+	for {
+		n, err := r.Read(data[j:])
+		j = j + n
+		if err != nil {
+			if err == io.EOF {
+				break
+			}
+			return nil, fmt.Errorf("Read failure: %s", err)
+		}
+
+		if (n == 0 && j == len(data)) || j > len(data) {
+			return nil, errors.New("Size of requested data is too large")
+		}
+	}
+
+	return data[:j], nil
+}

util/util_test.go

@@ -19,6 +19,7 @@ package util
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -567,6 +568,60 @@ func TestCertDuration(t *testing.T) {
 	assert.Error(t, err)
 }
 
+type MyReader struct {
+	buf                   []byte
+	maxPerRead, bytesRead int
+}
+
+func (r *MyReader) Read(data []byte) (int, error) {
+	if r.bytesRead >= len(r.buf) {
+		return 0, io.EOF
+	}
+	buf := r.buf[r.bytesRead:]
+	count := 0
+	for i, v := range buf {
+		if i >= len(data) || count > r.maxPerRead {
+			break
+		}
+		data[i] = v
+		count++
+	}
+	r.bytesRead = r.bytesRead + count
+	return count, nil
+}
+
+func TestRead(t *testing.T) {
+	myReader := MyReader{
+		buf:        []byte("123456789012345"),
+		maxPerRead: 6,
+	}
+
+	// Test with a buffer that is too small to fit data
+	buf := make([]byte, 10)
+	data, err := Read(&myReader, buf)
+	assert.Error(t, err, "Should have errored, the data passed is bigger than the buffer")
+
+	// Test with a buffer that is big enough to fit data
+	buf = make([]byte, 25)
+	myReader.bytesRead = 0
+	data, err = Read(&myReader, buf)
+	if assert.NoError(t, err, fmt.Sprintf("Error occured during read: %s", err)) {
+		if string(data) != string(myReader.buf) {
+			t.Error("The data returned does not match")
+		}
+	}
+
+	// Test with a buffer with exact size of data
+	buf = make([]byte, len(myReader.buf))
+	myReader.bytesRead = 0
+	data, err = Read(&myReader, buf)
+	if assert.NoError(t, err, fmt.Sprintf("Error occured during exact size read: %s", err)) {
+		if string(data) != string(myReader.buf) {
+			t.Error("The data returned does not match")
+		}
+	}
+}
+
 func getPEM(file string, t *testing.T) []byte {
 	buf, err := ioutil.ReadFile(file)
 	assert.NoError(t, err)