Merge pull request #41 from shogo82148/improve-validating-key

Improve validating key
shogo82148 · Sep 23, 2021 · a9328f2 · a9328f2
2 parents f112d6e + fa331fd
commit a9328f2
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 0 deletions.
diff --git a/provider/github-app-token/github/jwk/ecdsa.go b/provider/github-app-token/github/jwk/ecdsa.go
@@ -5,6 +5,7 @@ import (
 	"crypto/elliptic"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"math/big"
 )
@@ -47,6 +48,19 @@ func parseEcdsaPrivateKey(data []byte) (Key, error) {
 	if err := key.decode(); err != nil {
 		return nil, err
 	}
+
+	// sanity check of the certificate
+	if certs := key.X509CertificateChain(); len(certs) > 0 {
+		cert := certs[0]
+		publicKey, ok := cert.PublicKey.(*ecdsa.PublicKey)
+		if !ok {
+			return nil, errors.New("jwk: public key types are mismatch")
+		}
+		if !key.privateKey.PublicKey.Equal(publicKey) {
+			return nil, errors.New("jwk: public keys are mismatch")
+		}
+	}
+
 	return &key, nil
 }
 
@@ -117,6 +131,19 @@ func parseEcdsaPublicKey(data []byte) (Key, error) {
 	if err := key.decode(); err != nil {
 		return nil, err
 	}
+
+	// sanity check of the certificate
+	if certs := key.X509CertificateChain(); len(certs) > 0 {
+		cert := certs[0]
+		publicKey, ok := cert.PublicKey.(*ecdsa.PublicKey)
+		if !ok {
+			return nil, errors.New("jwk: public key types are mismatch")
+		}
+		if !key.publicKey.Equal(publicKey) {
+			return nil, errors.New("jwk: public keys are mismatch")
+		}
+	}
+
 	return &key, nil
 }
 

diff --git a/provider/github-app-token/github/jwk/rsa.go b/provider/github-app-token/github/jwk/rsa.go
@@ -4,6 +4,7 @@ import (
 	"crypto/rsa"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"math/big"
 )
@@ -62,6 +63,19 @@ func parseRSAPrivateKey(data []byte) (Key, error) {
 	if err := key.decode(); err != nil {
 		return nil, err
 	}
+
+	// sanity check of the certificate
+	if certs := key.X509CertificateChain(); len(certs) > 0 {
+		cert := certs[0]
+		publicKey, ok := cert.PublicKey.(*rsa.PublicKey)
+		if !ok {
+			return nil, errors.New("jwk: public key types are mismatch")
+		}
+		if !key.privateKey.PublicKey.Equal(publicKey) {
+			return nil, errors.New("jwk: public keys are mismatch")
+		}
+	}
+
 	return &key, nil
 }
 
@@ -199,6 +213,19 @@ func parseRSAPublicKey(data []byte) (Key, error) {
 	if err := key.decode(); err != nil {
 		return nil, err
 	}
+
+	// sanity check of the certificate
+	if certs := key.X509CertificateChain(); len(certs) > 0 {
+		cert := certs[0]
+		publicKey, ok := cert.PublicKey.(*rsa.PublicKey)
+		if !ok {
+			return nil, errors.New("jwk: public key types are mismatch")
+		}
+		if !key.publicKey.Equal(publicKey) {
+			return nil, errors.New("jwk: public keys are mismatch")
+		}
+	}
+
 	return &key, nil
 }
 

diff --git a/provider/github-app-token/github/oidc/client.go b/provider/github-app-token/github/oidc/client.go
@@ -2,10 +2,14 @@ package oidc
 
 import (
 	"context"
+	"crypto/ecdsa"
+	"crypto/ed25519"
+	"crypto/rsa"
 	"encoding/hex"
 	"errors"
 	"fmt"
 	"net/http"
+	"time"
 
 	"github.com/golang-jwt/jwt/v4"
 )
@@ -51,6 +55,9 @@ func NewClient(httpClient Doer, issuer string, thumbprints []string) (*Client, e
 
 func (c *Client) ParseWithClaims(ctx context.Context, tokenString string, claims jwt.Claims) (*jwt.Token, error) {
 	return jwt.ParseWithClaims(tokenString, claims, func(token *jwt.Token) (interface{}, error) {
+		now := time.Now()
+
+		// get JSON Web Key Set
 		config, err := c.GetConfig(ctx)
 		if err != nil {
 			return nil, err
@@ -63,10 +70,41 @@ func (c *Client) ParseWithClaims(ctx context.Context, tokenString string, claims
 		if !ok {
 			return nil, errors.New("oidc: kid of JWT is not found")
 		}
+
+		// find the key
 		key, ok := keys.Find(kid)
 		if !ok {
 			return nil, errors.New("oidc: key is not found")
 		}
+
+		// verify the certificates
+		for _, cert := range key.X509CertificateChain() {
+			if now.After(cert.NotAfter) {
+				return nil, errors.New("oidc: the certificate is expired")
+			}
+			if now.Before(cert.NotBefore) {
+				return nil, errors.New("oidc: the certificate is not valid yet")
+			}
+		}
+
+		// verify signing method
+		publicKey := key.PublicKey()
+		switch publicKey.(type) {
+		case *rsa.PublicKey:
+			if _, ok := token.Method.(*jwt.SigningMethodRSA); !ok {
+				return nil, errors.New("oidc: unexpected signing method")
+			}
+		case *ecdsa.PublicKey:
+			if _, ok := token.Method.(*jwt.SigningMethodECDSA); !ok {
+				return nil, errors.New("oidc: unexpected signing method")
+			}
+		case ed25519.PublicKey:
+			if _, ok := token.Method.(*jwt.SigningMethodEd25519); !ok {
+				return nil, errors.New("oidc: unexpected signing method")
+			}
+		default:
+			return nil, fmt.Errorf("oidc: unknown key type: %s", key.KeyType())
+		}
 		return key.PublicKey(), nil
 	})
 }