Skip to content

Commit

Permalink
[FAB-4507] Token-based authentication issue
Browse files Browse the repository at this point in the history
This change set fixes a security hole in the token-based
authentication mechanism of fabric-ca.  See [FAB-4507]
for details.  The fix is to call x509.Certificate.Verify
with the CA's trusted root and optionally intermediate
CA certs.

The new test case fails without this fix but passes
with this fix.

Change-Id: I2cf058094c3688ebe5424b11cfbfc0820024df6a
Signed-off-by: Keith Smith <bksmith@us.ibm.com>
  • Loading branch information
Keith Smith committed Jun 9, 2017
1 parent 7555a9b commit ef110bc
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 2 deletions.
51 changes: 51 additions & 0 deletions lib/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"crypto/ecdsa"
"crypto/rsa"
"crypto/x509"
"encoding/pem"
"errors"
"fmt"
"io/ioutil"
Expand Down Expand Up @@ -83,6 +84,8 @@ type CA struct {
registry spi.UserRegistry
// The signer used for enrollment
enrollSigner signer.Signer
// The options to use in verifying a signature in token-based authentication
verifyOptions *x509.VerifyOptions
// The tcert manager for this CA
tcertMgr *tcert.Mgr
// The key tree
Expand Down Expand Up @@ -411,6 +414,54 @@ func (ca *CA) initConfig() (err error) {
return nil
}

// VerifyCertificate verifies that 'cert' was issued by this CA
// Return nil if successful; otherwise, return an error.
func (ca *CA) VerifyCertificate(cert *x509.Certificate) error {
opts, err := ca.getVerifyOptions()
if err != nil {
return fmt.Errorf("Failed to get verify options: %s", err)
}
_, err = cert.Verify(*opts)
if err != nil {
return fmt.Errorf("Failed to verify certificate: %s", err)
}
return nil
}

// Get the options to verify
func (ca *CA) getVerifyOptions() (*x509.VerifyOptions, error) {
if ca.verifyOptions != nil {
return ca.verifyOptions, nil
}
chain, err := ca.getCAChain()
if err != nil {
return nil, err
}
block, rest := pem.Decode(chain)
if block == nil {
return nil, errors.New("No root certificate was found")
}
rootCert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
return nil, fmt.Errorf("Failed to parse root certificate: %s", err)
}
rootPool := x509.NewCertPool()
rootPool.AddCert(rootCert)
var intPool *x509.CertPool
for len(rest) > 0 {
intPool = x509.NewCertPool()
if !intPool.AppendCertsFromPEM(rest) {
return nil, errors.New("Failed to add intermediate PEM certificates")
}
}
ca.verifyOptions = &x509.VerifyOptions{
Roots: rootPool,
Intermediates: intPool,
KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny},
}
return ca.verifyOptions, nil
}

// Initialize the database for the CA
func (ca *CA) initDB() error {
db := &ca.Config.DB
Expand Down
49 changes: 49 additions & 0 deletions lib/client_whitebox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ limitations under the License.
package lib

import (
"crypto/rand"
"crypto/x509"
"encoding/pem"
"fmt"
"os"
"path"
Expand All @@ -24,6 +27,9 @@ import (
"github.com/cloudflare/cfssl/signer"
"github.com/hyperledger/fabric-ca/api"
"github.com/hyperledger/fabric-ca/util"
"github.com/hyperledger/fabric/bccsp"
cspsigner "github.com/hyperledger/fabric/bccsp/signer"
"github.com/hyperledger/fabric/bccsp/utils"
)

const (
Expand Down Expand Up @@ -84,6 +90,7 @@ func TestTLSClientAuth(t *testing.T) {
t.Fatalf("Failed to enroll admin: %s", err)
}
id := eresp.Identity
testImpersonation(id, t)
// Stop server
err = server.Stop()
if err != nil {
Expand Down Expand Up @@ -196,6 +203,48 @@ func enrollAndCheck(t *testing.T, c *Client, body []byte, authHeader string) {
}
}

// Try to impersonate 'id' identity by creating a self-signed certificate
// with the same serial and AKI as this identity.
func testImpersonation(id *Identity, t *testing.T) {
// test as a fake user trying to impersonate admin give only the cert
cert, err := BytesToX509Cert(id.GetECert().Cert())
if err != nil {
t.Fatalf("Failed to convert admin's cert: %s", err)
}
csp := util.GetDefaultBCCSP()
privateKey, err := csp.KeyGen(&bccsp.ECDSAKeyGenOpts{Temporary: false})
if err != nil {
t.Fatalf("Failed generating ECDSA key [%s]", err)
}
cspSigner, err := cspsigner.New(csp, privateKey)
if err != nil {
t.Fatalf("Failed initializing signer: %s", err)
}
// Export the public key
publicKey, err := privateKey.PublicKey()
if err != nil {
t.Fatalf("Failed getting ECDSA public key: %s", err)
}
pkRaw, err := publicKey.Bytes()
if err != nil {
t.Fatalf("Failed getting ECDSA raw public key [%s]", err)
}
pub, err := utils.DERToPublicKey(pkRaw)
if err != nil {
t.Fatalf("Failed converting raw to ECDSA.PublicKey [%s]", err)
}
fakeCertBytes, err := x509.CreateCertificate(rand.Reader, cert, cert, pub, cspSigner)
if err != nil {
t.Fatalf("Failed to create self-signed fake cert: %s", err)
}
fakeCert := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: fakeCertBytes})
fakeID := newIdentity(id.GetClient(), "admin", privateKey, fakeCert)
err = fakeID.RevokeSelf()
if err == nil {
t.Fatalf("Fake ID should not have failed revocation")
}
}

func getEnrollmentPayload(t *testing.T, c *Client) ([]byte, error) {
req := &api.EnrollmentRequest{
Name: user,
Expand Down
12 changes: 10 additions & 2 deletions lib/serverauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,20 @@ func (ah *fcaAuthHandler) serveHTTP(w http.ResponseWriter, r *http.Request) erro
return authError
case token:

ca := ah.server.caMap[req.CAName]

// verify token
cert, err2 := util.VerifyToken(ah.server.caMap[req.CAName].csp, authHdr, body)
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)

Expand All @@ -170,7 +178,7 @@ func (ah *fcaAuthHandler) serveHTTP(w http.ResponseWriter, r *http.Request) erro
aki = strings.ToLower(strings.TrimLeft(aki, "0"))
serial = strings.ToLower(strings.TrimLeft(serial, "0"))

certs, err := ah.server.caMap[req.CAName].CertDBAccessor().GetCertificate(serial, aki)
certs, err := ca.CertDBAccessor().GetCertificate(serial, aki)
if err != nil {
return authError
}
Expand Down

0 comments on commit ef110bc

Please sign in to comment.