Skip to content

Commit

Permalink
Refactor challenge verification (#580)
Browse files Browse the repository at this point in the history 
* Break up token parsing and proof of possession verification

Signed-off-by: Nathan Smith <nathan@chainguard.dev>

* Group proof of possession logic by challenge type

There are two challenges a caller can use to prove they possess their
private key:
- Submit a CSR
- Sign the subject or email from their ID token
Previously the logic to verify these two types of challenges was
interweaved. This work splits the verification into two different
branches and groups the logic of each type of verification together.

Signed-off-by: Nathan Smith <nathan@chainguard.dev>
  • Loading branch information
@nsmith5
nsmith5 authored May 17, 2022
1 parent a1b85d4 commit c041c98
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 71 deletions.
4 changes: 3 additions & 1 deletion pkg/api/error.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ const (
insecurePublicKey = "The public key supplied in the request is insecure"
//nolint
invalidCredentials = "There was an error processing the credentials for this request"
genericCAError = "error communicating with CA backend"
// nolint
invalidIdentityToken = "There was an error processing the identity token"
genericCAError = "error communicating with CA backend"
)

func handleFulcioGRPCError(ctx context.Context, code codes.Code, err error, message string, fields ...interface{}) error {
Expand Down
78 changes: 50 additions & 28 deletions pkg/api/grpc_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ package api

import (
"context"
"crypto/x509"
"crypto"
"encoding/base64"
"encoding/json"
"fmt"
Expand Down Expand Up @@ -70,43 +70,65 @@ func (g *grpcCAServer) CreateSigningCertificate(ctx context.Context, request *fu
}
}

principal, err := authorize(ctx, token)
// Authenticate OIDC ID token by checking signature
idtoken, err := authorize(ctx, token)
if err != nil {
return nil, handleFulcioGRPCError(ctx, codes.Unauthenticated, err, invalidCredentials)
}
// Parse authenticated ID token into principal
// TODO:(nsmith5) replace this and authorize call above with
// just identity.IssuerPool.Authenticate()
principal, err := challenges.PrincipalFromIDToken(ctx, idtoken)
if err != nil {
return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, invalidIdentityToken)
}

// optionally parse CSR
var csr *x509.CertificateRequest
var publicKey crypto.PublicKey
// Verify caller is in possession of their private key and extract
// public key from request.
if len(request.GetCertificateSigningRequest()) > 0 {
csr, err = cryptoutils.ParseCSR(request.GetCertificateSigningRequest())
// Option 1: Verify CSR
csr, err := cryptoutils.ParseCSR(request.GetCertificateSigningRequest())
if err != nil {
return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, invalidCSR)
}
}

// fetch public key from request or CSR
var pubKeyContent string
var proofOfPossession []byte
if request.GetPublicKeyRequest() != nil {
if request.GetPublicKeyRequest().PublicKey != nil {
pubKeyContent = request.GetPublicKeyRequest().PublicKey.Content
// Parse public key and check for weak key parameters
publicKey = csr.PublicKey
if err := cryptoutils.ValidatePubKey(publicKey); err != nil {
return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, insecurePublicKey)
}
proofOfPossession = request.GetPublicKeyRequest().ProofOfPossession
}
publicKey, err := challenges.ParsePublicKey(pubKeyContent, csr)
if err != nil {
return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, invalidPublicKey)
}

// validate public key, checking for weak key parameters
if err := cryptoutils.ValidatePubKey(publicKey); err != nil {
return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, insecurePublicKey)
}
if err := csr.CheckSignature(); err != nil {
return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, invalidSignature)
}
} else {
// Option 2: Check the signature for proof of possession of a private key
var (
pubKeyContent string
proofOfPossession []byte
err error
)
if request.GetPublicKeyRequest() != nil {
if request.GetPublicKeyRequest().PublicKey != nil {
pubKeyContent = request.GetPublicKeyRequest().PublicKey.Content
}
proofOfPossession = request.GetPublicKeyRequest().ProofOfPossession
}

// verify challenge
subject, err := challenges.ExtractSubject(ctx, principal, publicKey, csr, proofOfPossession)
if err != nil {
return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, invalidSignature)
// Parse public key and check for weak parameters
publicKey, err = challenges.ParsePublicKey(pubKeyContent)
if err != nil {
return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, invalidPublicKey)
}
if err := cryptoutils.ValidatePubKey(publicKey); err != nil {
return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, insecurePublicKey)
}

// Check proof of possession signature
if err := challenges.CheckSignature(publicKey, proofOfPossession, principal.Name(ctx)); err != nil {
return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, invalidSignature)
}
}

var csc *certauth.CodeSigningCertificate
Expand All @@ -115,7 +137,7 @@ func (g *grpcCAServer) CreateSigningCertificate(ctx context.Context, request *fu
// For CAs that do not support embedded SCTs or if the CT log is not configured
if sctCa, ok := g.ca.(certauth.EmbeddedSCTCA); !ok || g.ct == nil {
// currently configured CA doesn't support pre-certificate flow required to embed SCT in final certificate
csc, err = g.ca.CreateCertificate(ctx, subject, publicKey)
csc, err = g.ca.CreateCertificate(ctx, principal, publicKey)
if err != nil {
// if the error was due to invalid input in the request, return HTTP 400
if _, ok := err.(certauth.ValidationError); ok {
Expand Down Expand Up @@ -165,7 +187,7 @@ func (g *grpcCAServer) CreateSigningCertificate(ctx context.Context, request *fu
result.GetSignedCertificateDetachedSct().SignedCertificateTimestamp = sctBytes
}
} else {
precert, err := sctCa.CreatePrecertificate(ctx, subject, publicKey)
precert, err := sctCa.CreatePrecertificate(ctx, principal, publicKey)
if err != nil {
// if the error was due to invalid input in the request, return HTTP 400
if _, ok := err.(certauth.ValidationError); ok {
Expand Down
26 changes: 5 additions & 21 deletions pkg/challenges/challenges.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ func validateAllowedDomain(subjectHostname, issuerHostname string) error {
return fmt.Errorf("hostname top-level and second-level domains do not match: %s, %s", subjectHostname, issuerHostname)
}

func ExtractSubject(ctx context.Context, tok *oidc.IDToken, publicKey crypto.PublicKey, csr *x509.CertificateRequest, challenge []byte) (identity.Principal, error) {
func PrincipalFromIDToken(ctx context.Context, tok *oidc.IDToken) (identity.Principal, error) {
iss, ok := config.FromContext(ctx).GetIssuer(tok.Issuer)
if !ok {
return nil, fmt.Errorf("configuration can not be loaded for issuer %v", tok.Issuer)
Expand All @@ -520,31 +520,15 @@ func ExtractSubject(ctx context.Context, tok *oidc.IDToken, publicKey crypto.Pub
return nil, err
}

// verify the proof of possession of the private key
if csr != nil {
err = csr.CheckSignature()
if err != nil {
return nil, err
}
} else {
if err := CheckSignature(publicKey, challenge, principal.Name(ctx)); err != nil {
return nil, err
}
}

return principal, nil
}

// ParsePublicKey parses a PEM or DER encoded public key, or extracts the public
// key from the provided CSR. Returns an error if decoding fails or if no public
// key is found.
func ParsePublicKey(encodedPubKey string, csr *x509.CertificateRequest) (crypto.PublicKey, error) {
if csr == nil && len(encodedPubKey) == 0 {
// ParsePublicKey parses a PEM or DER encoded public key. Returns an error if
// decoding fails or if no public key is found.
func ParsePublicKey(encodedPubKey string) (crypto.PublicKey, error) {
if len(encodedPubKey) == 0 {
return nil, errors.New("public key not provided")
}
if csr != nil {
return csr.PublicKey, nil
}
// try to unmarshal as PEM
publicKey, err := cryptoutils.UnmarshalPEMToPublicKey([]byte(encodedPubKey))
if err != nil {
Expand Down
25 changes: 4 additions & 21 deletions pkg/challenges/challenges_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ import (
"crypto/rsa"
"crypto/sha256"
"crypto/x509"
"crypto/x509/pkix"
"encoding/asn1"
"encoding/pem"
"errors"
"fmt"
"net/url"
Expand Down Expand Up @@ -671,28 +669,13 @@ func TestCheckSignatureRSA(t *testing.T) {
}

func TestParsePublicKey(t *testing.T) {
// succeeds with CSR
priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
failErr(t, err)
csrTmpl := &x509.CertificateRequest{Subject: pkix.Name{CommonName: "test"}}
derCSR, err := x509.CreateCertificateRequest(rand.Reader, csrTmpl, priv)
failErr(t, err)
pemCSR := pem.EncodeToMemory(&pem.Block{
Type: "CERTIFICATE REQUEST",
Bytes: derCSR,
})
parsedCSR, err := cryptoutils.ParseCSR(pemCSR)
failErr(t, err)
pubKey, err := ParsePublicKey("", parsedCSR)
failErr(t, err)
if err := cryptoutils.EqualKeys(pubKey, priv.Public()); err != nil {
t.Fatalf("expected equal public keys")
}

// succeeds with PEM-encoded key
pemKey, err := cryptoutils.MarshalPublicKeyToPEM(priv.Public())
failErr(t, err)
pubKey, err = ParsePublicKey(string(pemKey), nil)
pubKey, err := ParsePublicKey(string(pemKey))
failErr(t, err)
if err := cryptoutils.EqualKeys(pubKey, priv.Public()); err != nil {
t.Fatalf("expected equal public keys")
Expand All @@ -701,22 +684,22 @@ func TestParsePublicKey(t *testing.T) {
// succeeds with DER-encoded key
derKey, err := cryptoutils.MarshalPublicKeyToDER(priv.Public())
failErr(t, err)
pubKey, err = ParsePublicKey(string(derKey), nil)
pubKey, err = ParsePublicKey(string(derKey))
failErr(t, err)
if err := cryptoutils.EqualKeys(pubKey, priv.Public()); err != nil {
t.Fatalf("expected equal public keys")
}

// fails with no public key
_, err = ParsePublicKey("", nil)
_, err = ParsePublicKey("")
if err == nil || err.Error() != "public key not provided" {
t.Fatalf("expected error parsing no public key, got %v", err)
}

// fails with invalid public key (private key)
pemPrivKey, err := cryptoutils.MarshalPrivateKeyToPEM(priv)
failErr(t, err)
_, err = ParsePublicKey(string(pemPrivKey), nil)
_, err = ParsePublicKey(string(pemPrivKey))
if err == nil || err.Error() != "error parsing PEM or DER encoded public key" {
t.Fatalf("expected error parsing invalid public key, got %v", err)
}
Expand Down

0 comments on commit c041c98

Please sign in to comment.