Limit the size of the response back from the external CA

Signed-off-by: Ying Li <ying.li@docker.com>

ca/external.go

@@ -8,6 +8,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"encoding/pem"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"sync"
@@ -24,8 +25,18 @@ import (
 	"golang.org/x/net/context/ctxhttp"
 )
 
-// ExternalCrossSignProfile is the profile that we will be sending cross-signing CSR sign requests with
-const ExternalCrossSignProfile = "CA"
+const (
+	// ExternalCrossSignProfile is the profile that we will be sending cross-signing CSR sign requests with
+	ExternalCrossSignProfile = "CA"
+
+	// CertificateMaxSize is the maximum expected size of a certificate.
+	// While there is no specced upper limit to the size of an x509 certificate in PEM format,
+	// one with a ridiculous RSA key size (16384) and 26 256-character DNS SAN fields is about 14k.
+	// While there is no upper limit on the length of certificate chains, long chains are impractical.
+	// To be conservative, and to also account for external CA certificate responses in JSON format
+	// from CFSSL, we'll set the max to be 256KiB.
+	CertificateMaxSize int64 = 256 << 10
+)
 
 // ErrNoExternalCAURLs is an error used it indicate that an ExternalCA is
 // configured with no URLs to which it can proxy certificate signing requests.
@@ -191,7 +202,8 @@ func makeExternalSignRequest(ctx context.Context, client *http.Client, url strin
 	}
 	defer resp.Body.Close()
 
-	body, err := ioutil.ReadAll(resp.Body)
+	b := io.LimitReader(resp.Body, CertificateMaxSize)
+	body, err := ioutil.ReadAll(b)
 	if err != nil {
 		return nil, recoverableErr{err: errors.Wrap(err, "unable to read CSR response body")}
 	}

ca/external_test.go

@@ -172,3 +172,77 @@ func TestExternalCACopy(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, cert)
 }
+
+// The ExternalCA object will stop reading the response from the server past a
+// a certain size
+func TestExternalCASignRequestSizeLimit(t *testing.T) {
+	t.Parallel()
+
+	if testutils.External {
+		return // this does not require the external CA in any way
+	}
+
+	ctx := log.WithLogger(context.Background(), log.L.WithFields(logrus.Fields{
+		"testname":          t.Name(),
+		"testHasExternalCA": false,
+	}))
+
+	rootCA, err := ca.CreateRootCA("rootCN")
+	require.NoError(t, err)
+
+	signDone, allDone, writeDone := make(chan error), make(chan struct{}), make(chan int64)
+	defer close(signDone)
+	mux := http.NewServeMux()
+
+	mux.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) {
+		var written int64
+		garbage := []byte("abcdefghijklmnopqrstuvwxyz")
+		// keep writing until done
+		for {
+			select {
+			case <-allDone:
+				return
+			default:
+				wrote, err := w.Write(garbage)
+				if err != nil {
+					writeDone <- written
+					return
+				}
+				written += int64(wrote)
+			}
+		}
+	})
+
+	server := httptest.NewServer(mux)
+	defer server.Close()
+	defer server.CloseClientConnections()
+	defer close(allDone)
+
+	csr, _, err := ca.GenerateNewCSR()
+	require.NoError(t, err)
+
+	externalCA := ca.NewExternalCA(&rootCA, nil, server.URL)
+	externalCA.ExternalRequestTimeout = time.Second
+	go func() {
+		_, err := externalCA.Sign(ctx, ca.PrepareCSR(csr, "cn", "ou", "org"))
+		select {
+		case <-allDone:
+		case signDone <- err:
+		}
+	}()
+
+	select {
+	case err = <-signDone:
+		require.Error(t, err)
+	case <-time.After(3 * time.Second):
+		require.FailNow(t, "call to external CA signing should have failed by now")
+	}
+
+	select {
+	case written := <-writeDone:
+		// due to buffering/client disconnecting, it may be a little over the max size, so add a fudge factor
+		require.True(t, written <= 2*ca.CertificateMaxSize)
+	case <-time.After(time.Second):
+		require.FailNow(t, "the client connection to the server should have been closed by now")
+	}
+}