Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor challenge verification #580

Merged
merged 2 commits into from
May 17, 2022
Merged

Refactor challenge verification #580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
acadb79
Break up token parsing and proof of possession verification
May 12, 2022
57dbda7
Group proof of possession logic by challenge type
May 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pkg/api/error.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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 @@ -466,28 +464,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 @@ -496,22 +479,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