Skip to content

Commit

Permalink
split signing/verification key
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexCuse committed Mar 22, 2024
1 parent 3fd1a72 commit a965eef
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 49 deletions.
8 changes: 4 additions & 4 deletions app/tokens/oauth/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,21 @@ func (c *Claims) Sign(signingKey jose.SigningKey) (string, error) {

// Parse will deserialize a string into Claims if and only if the claims pass all validations. In
// this case the token must contain a nonce already known from a different channel (like a cookie).
func Parse(tokenStr string, cfg *app.Config, nonce string) (*Claims, error) {
func Parse(tokenStr string, signingKey interface{}, authNURL string, nonce string) (*Claims, error) {
token, err := jwt.ParseSigned(tokenStr)
if err != nil {
return nil, errors.Wrap(err, "ParseSigned")
}

claims := Claims{}
err = token.Claims(cfg.OAuthSigningKey, &claims)
err = token.Claims(signingKey, &claims)
if err != nil {
return nil, errors.Wrap(err, "Claims")
}

err = claims.Claims.Validate(jwt.Expected{
Audience: jwt.Audience{cfg.AuthNURL.String()},
Issuer: cfg.AuthNURL.String(),
Audience: jwt.Audience{authNURL},
Issuer: authNURL,
})
if err != nil {
return nil, errors.Wrap(err, "Validate")
Expand Down
28 changes: 20 additions & 8 deletions app/tokens/oauth/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,25 @@ func TestOAuthToken(t *testing.T) {
assert.True(t, token.Audience.Contains("https://authn.example.com"))
assert.NotEmpty(t, token.IssuedAt)

tokenStr, err := token.Sign(jose.SigningKey{Algorithm: jose.HS256, Key: cfg.OAuthSigningKey})
key := jose.SigningKey{Algorithm: jose.HS256, Key: cfg.OAuthSigningKey}

tokenStr, err := token.Sign(key)
require.NoError(t, err)

_, err = oauth.Parse(tokenStr, cfg, nonce)
_, err = oauth.Parse(tokenStr, key.Key, cfg.AuthNURL.String(), nonce)
require.NoError(t, err)
})

t.Run("parsing with an unknown nonce", func(t *testing.T) {
token, err := oauth.New(cfg, nonce, "https://app.example.com/return")
require.NoError(t, err)

tokenStr, err := token.Sign(jose.SigningKey{Algorithm: jose.HS256, Key: cfg.OAuthSigningKey})
key := jose.SigningKey{Algorithm: jose.HS256, Key: cfg.OAuthSigningKey}

tokenStr, err := token.Sign(key)
require.NoError(t, err)

_, err = oauth.Parse(tokenStr, cfg, "wrong")
_, err = oauth.Parse(tokenStr, key.Key, cfg.AuthNURL.String(), "wrong")
assert.Error(t, err)
})

Expand All @@ -52,11 +56,16 @@ func TestOAuthToken(t *testing.T) {
AuthNURL: cfg.AuthNURL,
OAuthSigningKey: []byte("old-a-reno"),
}
oldKey := jose.SigningKey{Algorithm: jose.HS256, Key: oldCfg.OAuthSigningKey}

token, err := oauth.New(cfg, nonce, "https://app.example.com/return")
require.NoError(t, err)
tokenStr, err := token.Sign(jose.SigningKey{Algorithm: jose.HS256, Key: oldCfg.OAuthSigningKey})

tokenStr, err := token.Sign(oldKey)
require.NoError(t, err)
_, err = oauth.Parse(tokenStr, cfg, nonce)

key := jose.SigningKey{Algorithm: jose.HS256, Key: cfg.OAuthSigningKey}
_, err = oauth.Parse(tokenStr, key.Key, cfg.AuthNURL.String(), nonce)
assert.Error(t, err)
})

Expand All @@ -67,9 +76,12 @@ func TestOAuthToken(t *testing.T) {
}
token, err := oauth.New(&oldCfg, nonce, "https://app.example.com/return")
require.NoError(t, err)
tokenStr, err := token.Sign(jose.SigningKey{Algorithm: jose.HS256, Key: cfg.OAuthSigningKey})

key := jose.SigningKey{Algorithm: jose.HS256, Key: cfg.OAuthSigningKey}

tokenStr, err := token.Sign(key)
require.NoError(t, err)
_, err = oauth.Parse(tokenStr, cfg, nonce)
_, err = oauth.Parse(tokenStr, key.Key, cfg.AuthNURL.String(), nonce)
assert.Error(t, err)
})
}
5 changes: 3 additions & 2 deletions lib/oauth/apple.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func NewAppleProvider(credentials *Credentials) (*Provider, error) {
return nil, fmt.Errorf("apple credentials: %w", constructErr)
}

signingKey, constructErr := apple.ParsePrivateKey(credentials.SigningKey, keyID)
signingKey, publicKey, constructErr := apple.ParsePrivateKey(credentials.SigningKey, keyID)
if constructErr != nil {
return nil, fmt.Errorf("failed to parse signing key: %w", constructErr)
}
Expand Down Expand Up @@ -56,6 +56,7 @@ func NewAppleProvider(credentials *Credentials) (*Provider, error) {
}
},
// So we need to handle returns via POST instead of GET
MethodOverride: http.MethodPost,
MethodOverride: http.MethodPost,
VerificationKey: *publicKey,
}), nil
}
15 changes: 10 additions & 5 deletions lib/oauth/apple/privkey.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package apple

import (
"crypto/ecdsa"
"crypto/x509"
"encoding/pem"
"fmt"
Expand All @@ -11,22 +12,26 @@ import (
)

// ParseAppleKey takes a PEM encoded ES256 key and returns a jose.SigningKey.
func ParsePrivateKey(keyBytes []byte, keyID string) (*jose.SigningKey, error) {
func ParsePrivateKey(keyBytes []byte, keyID string) (*jose.SigningKey, *jose.JSONWebKey, error) {
keyBlock, _ := pem.Decode(keyBytes)
if keyBlock == nil {
return nil, fmt.Errorf("failed to decode PEM data")
return nil, nil, fmt.Errorf("failed to decode PEM data")
}
key, err := x509.ParsePKCS8PrivateKey(keyBlock.Bytes)
if err != nil {
return nil, fmt.Errorf("failed to parse EC key: %w", err)
return nil, nil, fmt.Errorf("failed to parse EC key: %w", err)
}

pk := key.(*ecdsa.PrivateKey).Public()

publicKey := &jose.JSONWebKey{Key: pk, Algorithm: string(jose.ES256), Use: "sig"}

return &jose.SigningKey{Key: jose.JSONWebKey{
Key: key,
KeyID: keyID,
Algorithm: "ES256",
Algorithm: string(jose.ES256),
Use: "sig",
}, Algorithm: jose.ES256}, nil
}, Algorithm: jose.ES256}, publicKey, nil
}

// GenerateSecret creates a signed JWT as specified at
Expand Down
21 changes: 7 additions & 14 deletions lib/oauth/apple/privkey_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package apple_test

import (
"crypto/ecdsa"
"testing"
"time"

Expand All @@ -11,7 +10,6 @@ import (
"github.com/keratin/authn-server/lib/oauth/apple"
"github.com/keratin/authn-server/lib/oauth/apple/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestGenerateAppleSecret(t *testing.T) {
Expand All @@ -20,14 +18,9 @@ func TestGenerateAppleSecret(t *testing.T) {
teamID := "teamID"
expiresInSeconds := int64(3600)

signingKey, err := apple.ParsePrivateKey([]byte(test.SampleKey), uuid.NewString())
signingKey, publicKey, err := apple.ParsePrivateKey([]byte(test.SampleKey), uuid.NewString())
assert.NoError(t, err)

jwk, ok := signingKey.Key.(jose.JSONWebKey)
require.True(t, ok)
pk, ok := jwk.Key.(*ecdsa.PrivateKey)
require.True(t, ok)

got, err := apple.GenerateSecret(*signingKey, keyID, clientID, teamID, expiresInSeconds)
assert.NoError(t, err)
assert.NotNil(t, got)
Expand All @@ -38,8 +31,6 @@ func TestGenerateAppleSecret(t *testing.T) {

claims := map[string]interface{}{}

publicKey := jose.JSONWebKey{Key: &pk.PublicKey, Algorithm: string(jose.ES256), Use: "sig"}

err = token.Claims(publicKey, &claims)
assert.NoError(t, err)

Expand All @@ -52,15 +43,17 @@ func TestGenerateAppleSecret(t *testing.T) {

func TestParseAppleKey(t *testing.T) {
t.Run("invalid key", func(t *testing.T) {
_, err := apple.ParsePrivateKey([]byte("invalid"), uuid.NewString())
_, _, err := apple.ParsePrivateKey([]byte("invalid"), uuid.NewString())
assert.Error(t, err)
})
t.Run("valid", func(t *testing.T) {
keyID := uuid.NewString()

got, err := apple.ParsePrivateKey([]byte(test.SampleKey), keyID)
gotPriv, gotPub, err := apple.ParsePrivateKey([]byte(test.SampleKey), keyID)
assert.NoError(t, err)
assert.NotNil(t, got)
assert.Equal(t, jose.ES256, got.Algorithm)
assert.NotNil(t, gotPriv)
assert.Equal(t, jose.ES256, gotPriv.Algorithm)
assert.NotNil(t, gotPub)
assert.Equal(t, string(jose.ES256), gotPub.Algorithm)
})
}
31 changes: 21 additions & 10 deletions lib/oauth/apple/pubkey.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ func (a *signingKeyStore) refresh() error {
return fmt.Errorf("failed to fetch apple keys: %w", keysErr)
}

defer keysResp.Body.Close()

keysBody, keysErr := io.ReadAll(keysResp.Body)
if keysErr != nil {
return fmt.Errorf("failed to read apple keys: %w", keysErr)
Expand All @@ -104,18 +106,18 @@ func (a *signingKeyStore) refresh() error {
for _, key := range keys.Keys {
// build key and place in new map
publicKey := new(rsa.PublicKey)
n := new(big.Int)
nbytes, _ := base64.URLEncoding.DecodeString(key.N + "=")
publicKey.N = n.SetBytes(nbytes)

var eInt int
ebytes, _ := base64.RawURLEncoding.DecodeString(key.E)
for _, v := range ebytes {
eInt = eInt << 8
eInt = eInt | int(v)
n, err := decodeBase64BigInt(key.N)
if err != nil {
return err
}
publicKey.N = n

e, err := decodeBase64BigInt(key.E)
if err != nil {
return err
}

publicKey.E = eInt
publicKey.E = int(e.Int64())

newKeys[key.Kid] = publicKey
}
Expand All @@ -124,3 +126,12 @@ func (a *signingKeyStore) refresh() error {

return nil
}

func decodeBase64BigInt(s string) (*big.Int, error) {
buffer, err := base64.URLEncoding.WithPadding(base64.NoPadding).DecodeString(s)
if err != nil {
return nil, err
}

return big.NewInt(0).SetBytes(buffer), nil
}
14 changes: 13 additions & 1 deletion lib/oauth/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type Provider struct {
config *oauth2.Config
UserInfo UserInfoFetcher
signingKey jose.SigningKey
verificationKey interface{}
secretOverride SecretOverride
authCodeOptions AuthCodeOptions
methodOverride string
Expand All @@ -36,11 +37,17 @@ type Overrides struct {
SecretOverride SecretOverride
AuthCodeOptions AuthCodeOptions
MethodOverride string
VerificationKey jose.JSONWebKey
}

// NewProvider returns a properly configured Provider
func NewProvider(config *oauth2.Config, userInfo UserInfoFetcher, signingKey jose.SigningKey) *Provider {
return &Provider{config: config, UserInfo: userInfo, signingKey: signingKey}
return &Provider{
config: config,
UserInfo: userInfo,
signingKey: signingKey,
verificationKey: signingKey.Key,
}
}

// NewProviderWithOverrides returns a properly configured Provider with the requested overrides.
Expand All @@ -49,6 +56,7 @@ func NewProviderWithOverrides(config *oauth2.Config, userInfo UserInfoFetcher, s
config: config,
UserInfo: userInfo,
signingKey: signingKey,
verificationKey: overrides.VerificationKey,
secretOverride: overrides.SecretOverride,
authCodeOptions: overrides.AuthCodeOptions,
methodOverride: overrides.MethodOverride,
Expand Down Expand Up @@ -81,6 +89,10 @@ func (p *Provider) SigningKey() jose.SigningKey {
return p.signingKey
}

func (p *Provider) VerificationKey() interface{} {
return p.verificationKey
}

func (p *Provider) AuthCodeOptions() []oauth2.AuthCodeOption {
if p.authCodeOptions != nil {
return p.authCodeOptions()
Expand Down
2 changes: 1 addition & 1 deletion server/handlers/get_oauth_return.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func GetOauthReturn(app *app.App, providerName string) http.HandlerFunc {
provider := app.OauthProviders[providerName]

// verify the state and nonce
state, err := getState(app.Config, r)
state, err := getState(app.Config, r, provider.VerificationKey())
if err != nil {
app.Reporter.ReportRequestError(errors.Wrap(err, "getState"), r)
failsafe := app.Config.ApplicationDomains[0].URL()
Expand Down
3 changes: 1 addition & 2 deletions server/handlers/get_oauth_return_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"net/http/httptest"
"testing"

"github.com/go-jose/go-jose/v3"
"github.com/stretchr/testify/assert"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -42,7 +41,7 @@ func TestGetOauthReturn(t *testing.T) {

token, err := oauthtoken.New(app.Config, nonce, "https://localhost:9999/return")
require.NoError(t, err)
state, err := token.Sign(jose.SigningKey{Algorithm: jose.HS256, Key: app.Config.OAuthSigningKey})
state, err := token.Sign(providerClient.SigningKey())
require.NoError(t, err)

t.Run("sign up new identity with new email", func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions server/handlers/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ func nonceCookie(cfg *app.Config, val string) *http.Cookie {
}

// getState returns a verified state token using the nonce cookie
func getState(cfg *app.Config, r *http.Request) (*oauth.Claims, error) {
func getState(cfg *app.Config, r *http.Request, verificationKey interface{}) (*oauth.Claims, error) {
nonce, err := r.Cookie(cfg.OAuthCookieName)
if err != nil {
return nil, errors.Wrap(err, "Cookie")
}
state, err := oauth.Parse(r.FormValue("state"), cfg, nonce.Value)
state, err := oauth.Parse(r.FormValue("state"), verificationKey, cfg.AuthNURL.String(), nonce.Value)
if err != nil {
return nil, errors.Wrap(err, "Parse")
}
Expand Down

0 comments on commit a965eef

Please sign in to comment.