Skip to content

Commit

Permalink
Minor fixes from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
bbockelm committed Jan 22, 2025
1 parent f803745 commit 060c7ab
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 13 deletions.
5 changes: 1 addition & 4 deletions launchers/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,7 @@ func LaunchModules(ctx context.Context, modules server_structs.ServerType) (serv

// Start a routine to periodically refresh the private key directory
// This ensures that new or updated private keys are automatically loaded and registered
if modules.IsEnabled(server_structs.RegistryType) || modules.IsEnabled(server_structs.OriginType) ||
modules.IsEnabled(server_structs.CacheType) || modules.IsEnabled(server_structs.DirectorType) {
launcher_utils.LaunchIssuerKeysDirRefresh(ctx, egrp, modules)
}
launcher_utils.LaunchIssuerKeysDirRefresh(ctx, egrp, modules)

log.Info("Starting web engine...")
lnReference = nil
Expand Down
7 changes: 7 additions & 0 deletions registry/client_commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,13 @@ func NamespaceDelete(endpoint string, prefix string) error {
return nil
}

// Update the set of registered public keys for a given list of prefixes & site name.
//
// - `privateKey`: The private key to use to sign the request (must also be one of the
// private keys in the configuration).
// - `prefixes`: The list of prefixes to update the public keys for in the registry.
// - `siteName`: The name that the server is registered under.
// - `namespacePubKeyEndUpdatepoint`: The registry endpoint.
func NamespacesPubKeyUpdate(privateKey jwk.Key, prefixes []string, siteName string, namespacePubKeyEndUpdatepoint string) error {
publicKey, err := privateKey.PublicKey()
if err != nil {
Expand Down
47 changes: 38 additions & 9 deletions registry/registry_pubkey_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package registry

import (
"context"
"crypto/ecdsa"
"encoding/hex"
"encoding/json"
Expand Down Expand Up @@ -87,6 +88,31 @@ func updateNsKeySignChallengeInit(data *RegisteredPrefixUpdate) (map[string]inte
return res, nil
}

// Compare two jwk.Set objects to see if they are the same
//
// Similar in spirit to the internal function config.areKeysDifferent.
func compareJwks(jwks1, jwks2 jwk.Set) bool {
if jwks1.Len() != jwks2.Len() {
return false
}
ctx := context.Background()
for jwksIter1 := jwks1.Keys(ctx); jwksIter1.Next(ctx); {
found := false
key1 := jwksIter1.Pair().Value.(jwk.Key)
for jwksIter2 := jwks2.Keys(ctx); jwksIter2.Next(ctx); {
key2 := jwksIter2.Pair().Value.(jwk.Key)
if jwk.Equal(key1, key2) {
found = true
break
}
}
if !found {
return false
}
}
return true
}

// Update the public key of registered prefix(es) if the http request passed client and server verification for nonce.
// It returns the response data, and an error if any
func updateNsKeySignChallengeCommit(data *RegisteredPrefixUpdate) (map[string]interface{}, error) {
Expand Down Expand Up @@ -174,12 +200,21 @@ func updateNsKeySignChallengeCommit(data *RegisteredPrefixUpdate) (map[string]in
// Process origin's latest public key(s)
allPubkeyData, err := json.Marshal(data.AllPubkeys)
if err != nil {
return nil, errors.Wrapf(err, "Failed to convert the latest public key(s) from json to string format for the prefix %s", prefix)
return nil, errors.Wrapf(err, "Failed to convert the latest public key(s) from JSON to string format for the prefix %s", prefix)
}
clientJWKS, err := jwk.Parse(allPubkeyData)
if err != nil {
return nil, errors.Wrap(err, "Failed to parse the client's public key(s) as JWKS")
}
clientPubkeyString := string(allPubkeyData)

// Perform the update action when the latest keys in the origin are different from the registered ones
if clientPubkeyString != existingNs.Pubkey {
if compareJwks(clientJWKS, registryDbKeySet) {
returnMsg := map[string]interface{}{
"message": fmt.Sprintf("The public key of prefix %s hasn't changed -- nothing to update!", prefix),
}
log.Infof("The public key of prefix %s hasn't changed -- nothing to update!", prefix)
return returnMsg, nil
} else {
err = setNamespacePubKey(prefix, string(data.AllPubkeys))
log.Debugf("New public keys %s just replaced the old ones: %s", string(data.AllPubkeys), existingNs.Pubkey)
if err != nil {
Expand All @@ -191,12 +226,6 @@ func updateNsKeySignChallengeCommit(data *RegisteredPrefixUpdate) (map[string]in
}
log.Infof("Updated the public key of namespace %s:", prefix)
return returnMsg, nil
} else {
returnMsg := map[string]interface{}{
"message": fmt.Sprintf("The public key of prefix %s hasn't changed -- nothing to update!", prefix),
}
log.Infof("The public key of prefix %s hasn't changed -- nothing to update!", prefix)
return returnMsg, nil
}

}
Expand Down
57 changes: 57 additions & 0 deletions registry/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ package registry

import (
"bytes"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -406,3 +409,57 @@ func TestCheckNamespaceCompleteHandler(t *testing.T) {
assert.Empty(t, result.Msg)
})
}

func TestCompareJwks(t *testing.T) {
priv1, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
assert.NoError(t, err)
priv2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
assert.NoError(t, err)
assert.False(t, priv1.Equal(priv2))

pubKey1, err := jwk.FromRaw(priv1.PublicKey)
assert.NoError(t, err)
pubKey2, err := jwk.FromRaw(priv2.PublicKey)
assert.NoError(t, err)

jwks1 := jwk.NewSet()
assert.NoError(t, jwks1.AddKey(pubKey1))
jwks2 := jwk.NewSet()
assert.NoError(t, jwks2.AddKey(pubKey2))
jwks1Alt := jwk.NewSet()
assert.NoError(t, jwks1Alt.AddKey(pubKey1))

jwks2Alt, err := jwks2.Clone()
assert.NoError(t, err)
key, err := jwk.FromRaw([]byte("01234567890123456789012345678901234567890123456789ABCDEF"))
assert.NoError(t, err)
assert.NoError(t, jwks2Alt.AddKey(key))

t.Run("compare-same-jwks", func(t *testing.T) {
equal := compareJwks(jwks1, jwks1Alt)
assert.True(t, equal, "Expected JWKS sets to be equal")
})

t.Run("compare-different-jwks", func(t *testing.T) {
equal := compareJwks(jwks1, jwks2)
assert.False(t, equal, "Expected JWKS sets to be different")
})

t.Run("compare-jwks-with-different-keys", func(t *testing.T) {
equal := compareJwks(jwks1Alt, jwks2Alt)
assert.False(t, equal, "Expected JWKS sets to be different due to new raw key")
})

t.Run("compare-jwks-with-different-key-count", func(t *testing.T) {
equal := compareJwks(jwks2, jwks2Alt)
assert.False(t, equal, "Expected JWKS sets to be different due to new raw key")
})

t.Run("compare-empty-jwks", func(t *testing.T) {
jwks1 := jwk.NewSet()
jwks2 := jwk.NewSet()

equal := compareJwks(jwks1, jwks2)
assert.True(t, equal, "Expected empty JWKS sets to be equal")
})
}
3 changes: 3 additions & 0 deletions token/token_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,9 @@ func (tokenConfig *TokenConfig) CreateTokenWithKey(key jwk.Key) (string, error)
if ok, err := tokenConfig.Validate(); !ok || err != nil {
return "", errors.Wrap(err, "invalid tokenConfig")
}
if key == nil {
return "", errors.New("cannot sign a token without a key")
}

jti_bytes := make([]byte, 16)
if _, err := rand.Read(jti_bytes); err != nil {
Expand Down

0 comments on commit 060c7ab

Please sign in to comment.