From 67dd999e5c6419026f24620ba30d67ce8f2234fc Mon Sep 17 00:00:00 2001 From: Ichinose Shogo Date: Thu, 23 Sep 2021 12:24:21 +0900 Subject: [PATCH 1/3] verify signing method --- .../github-app-token/github/oidc/client.go | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/provider/github-app-token/github/oidc/client.go b/provider/github-app-token/github/oidc/client.go index b033ff01..987b7fff 100644 --- a/provider/github-app-token/github/oidc/client.go +++ b/provider/github-app-token/github/oidc/client.go @@ -51,6 +51,7 @@ 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) { + // get JSON Web Key Set config, err := c.GetConfig(ctx) if err != nil { return nil, err @@ -63,10 +64,30 @@ 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 signing method + switch key.KeyType() { + case "RSA": + if _, ok := token.Method.(*jwt.SigningMethodRSA); !ok { + return nil, errors.New("oidc: unexpected signing method") + } + case "EC": + if _, ok := token.Method.(*jwt.SigningMethodECDSA); !ok { + return nil, errors.New("oidc: unexpected signing method") + } + case "OKP": + 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 }) } From 33506b3bcd95268f928870cd08b573d5fd5a3e47 Mon Sep 17 00:00:00 2001 From: Ichinose Shogo Date: Thu, 23 Sep 2021 12:36:31 +0900 Subject: [PATCH 2/3] verify the certificates --- .../github-app-token/github/oidc/client.go | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/provider/github-app-token/github/oidc/client.go b/provider/github-app-token/github/oidc/client.go index 987b7fff..8fbc5d30 100644 --- 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,8 @@ 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 { @@ -71,17 +77,28 @@ func (c *Client) ParseWithClaims(ctx context.Context, tokenString string, claims 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 - switch key.KeyType() { - case "RSA": + 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 "EC": + case *ecdsa.PublicKey: if _, ok := token.Method.(*jwt.SigningMethodECDSA); !ok { return nil, errors.New("oidc: unexpected signing method") } - case "OKP": + case ed25519.PublicKey: if _, ok := token.Method.(*jwt.SigningMethodEd25519); !ok { return nil, errors.New("oidc: unexpected signing method") } From fa331fdb68f3a24fe8386a53b32d90deb8f76b7f Mon Sep 17 00:00:00 2001 From: Ichinose Shogo Date: Thu, 23 Sep 2021 12:54:27 +0900 Subject: [PATCH 3/3] sanity check of the certificate --- provider/github-app-token/github/jwk/ecdsa.go | 27 +++++++++++++++++++ provider/github-app-token/github/jwk/rsa.go | 27 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/provider/github-app-token/github/jwk/ecdsa.go b/provider/github-app-token/github/jwk/ecdsa.go index f9f8fbf5..292fe068 100644 --- 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 index eabcb767..673b0b20 100644 --- 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 }