From 916f028271d291b7559d2a517fc18b56a2d17f65 Mon Sep 17 00:00:00 2001 From: Howard Zhong Date: Wed, 22 Jan 2025 07:57:51 -0600 Subject: [PATCH 1/8] Implement new private key directory This permits multiple private keys to be associated with a Pelican service and synchronize its keys with the registry. --- cmd/generate_keygen.go | 6 +- cmd/generate_keygen_test.go | 15 +- cmd/registry_client.go | 9 +- config/config.go | 12 +- config/encrypted.go | 14 +- config/encrypted_test.go | 21 +- config/init_server_creds.go | 346 +++++++++++++++--- config/init_server_creds_test.go | 36 ++ director/director_test.go | 5 +- director/origin_api_test.go | 5 +- director/stat_test.go | 5 +- docs/parameters.yaml | 17 +- ...key_refresh_and_update_namespace_pubkey.go | 141 +++++++ launcher_utils/register_namespace_test.go | 155 +++++++- launchers/launcher.go | 7 + origin/origin_db_test.go | 4 +- param/parameters.go | 2 + param/parameters_struct.go | 2 + registry/client_commands.go | 182 ++++++++- registry/client_commands_test.go | 290 +++++++++++++-- registry/registry.go | 29 +- registry/registry_db.go | 12 +- registry/registry_pubkey_update.go | 270 ++++++++++++++ token/token_create_test.go | 5 +- web_ui/prometheus_test.go | 21 +- web_ui/ui_test.go | 6 +- 26 files changed, 1451 insertions(+), 166 deletions(-) create mode 100644 launcher_utils/key_refresh_and_update_namespace_pubkey.go create mode 100644 registry/registry_pubkey_update.go diff --git a/cmd/generate_keygen.go b/cmd/generate_keygen.go index 8f0f03d4c..f2c4832b5 100644 --- a/cmd/generate_keygen.go +++ b/cmd/generate_keygen.go @@ -39,7 +39,7 @@ func keygenMain(cmd *cobra.Command, args []string) error { return errors.Wrap(err, "failed to get the current working directory") } if privateKeyPath == "" { - privateKeyPath = filepath.Join(wd, "issuer.jwk") + privateKeyPath = filepath.Join(wd, "issuer-keys") } else { privateKeyPath = filepath.Clean(strings.TrimSpace(privateKeyPath)) } @@ -68,9 +68,9 @@ func keygenMain(cmd *cobra.Command, args []string) error { return fmt.Errorf("file exists for public key under %s", publicKeyPath) } - viper.Set(param.IssuerKey.GetName(), privateKeyPath) + viper.Set(param.IssuerKeysDirectory.GetName(), privateKeyPath) - // GetIssuerPublicJWKS will generate the private key at IssuerKey if it does not exist + // GetIssuerPublicJWKS will generate the private key in IssuerKeysDirectory if it does not exist // and parse the private key and generate the corresponding public key for us pubkey, err := config.GetIssuerPublicJWKS() if err != nil { diff --git a/cmd/generate_keygen_test.go b/cmd/generate_keygen_test.go index 3eb111509..436a84446 100644 --- a/cmd/generate_keygen_test.go +++ b/cmd/generate_keygen_test.go @@ -33,7 +33,7 @@ import ( // Create tmpdir, change cwd, and setup clean up functions func setupTestRun(t *testing.T) string { - config.ResetIssuerJWKPtr() + config.ResetIssuerPrivateKeys() wd, err := os.Getwd() require.NoError(t, err) @@ -45,13 +45,12 @@ func setupTestRun(t *testing.T) string { err := os.Chdir(wd) require.NoError(t, err) server_utils.ResetTestState() - config.ResetIssuerJWKPtr() }) return tmpDir } -func checkKeys(t *testing.T, privateKey, publicKey string) { - _, err := config.LoadPrivateKey(privateKey, false) +func checkKeys(t *testing.T, publicKey string) { + _, err := config.GetIssuerPrivateJWK() require.NoError(t, err) jwks, err := jwk.ReadFile(publicKey) @@ -64,11 +63,10 @@ func checkKeys(t *testing.T, privateKey, publicKey string) { } func TestKeygenMain(t *testing.T) { - config.ResetIssuerJWKPtr() + config.ResetIssuerPrivateKeys() t.Cleanup(func() { server_utils.ResetTestState() - config.ResetIssuerJWKPtr() }) t.Run("no-args-gen-to-wd", func(t *testing.T) { @@ -81,7 +79,6 @@ func TestKeygenMain(t *testing.T) { checkKeys( t, - filepath.Join(tempDir, "issuer.jwk"), filepath.Join(tempDir, "issuer-pub.jwks"), ) }) @@ -97,7 +94,6 @@ func TestKeygenMain(t *testing.T) { checkKeys( t, - privateKeyPath, filepath.Join(tempWd, "issuer-pub.jwks"), ) }) @@ -113,7 +109,6 @@ func TestKeygenMain(t *testing.T) { checkKeys( t, - filepath.Join(tempWd, "issuer.jwk"), publicKeyPath, ) }) @@ -130,7 +125,6 @@ func TestKeygenMain(t *testing.T) { checkKeys( t, - privateKeyPath, filepath.Join(tempWd, "issuer-pub.jwks"), ) }) @@ -147,7 +141,6 @@ func TestKeygenMain(t *testing.T) { checkKeys( t, - filepath.Join(tempWd, "issuer.jwk"), publicKeyPath, ) }) diff --git a/cmd/registry_client.go b/cmd/registry_client.go index 8bc03d910..710254eef 100644 --- a/cmd/registry_client.go +++ b/cmd/registry_client.go @@ -32,14 +32,12 @@ import ( "net/url" "os" - "github.com/lestrrat-go/jwx/v2/jwk" "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" "github.com/pelicanplatform/pelican/config" - "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/registry" ) @@ -109,16 +107,11 @@ func registerANamespace(cmd *cobra.Command, args []string) { os.Exit(1) } - privateKeyRaw, err := config.LoadPrivateKey(param.IssuerKey.GetString(), false) + privateKey, err := config.GetIssuerPrivateJWK() if err != nil { log.Error("Failed to load private key", err) os.Exit(1) } - privateKey, err := jwk.FromRaw(privateKeyRaw) - if err != nil { - log.Error("Failed to create JWK private key", err) - os.Exit(1) - } if withIdentity { // We haven't added support to pass sitename from CLI, so leave it empty diff --git a/config/config.go b/config/config.go index 250212058..c5083e4bc 100644 --- a/config/config.go +++ b/config/config.go @@ -1022,6 +1022,7 @@ func SetServerDefaults(v *viper.Viper) error { v.SetDefault(param.Xrootd_Authfile.GetName(), filepath.Join(configDir, "xrootd", "authfile")) v.SetDefault(param.Xrootd_MacaroonsKeyFile.GetName(), filepath.Join(configDir, "macaroons-secret")) v.SetDefault(param.IssuerKey.GetName(), filepath.Join(configDir, "issuer.jwk")) + v.SetDefault(param.IssuerKeysDirectory.GetName(), filepath.Join(configDir, "issuer-keys")) v.SetDefault(param.Server_UIPasswordFile.GetName(), filepath.Join(configDir, "server-web-passwd")) v.SetDefault(param.Server_UIActivationCodeFile.GetName(), filepath.Join(configDir, "server-web-activation-code")) v.SetDefault(param.OIDC_ClientIDFile.GetName(), filepath.Join(configDir, "oidc-client-id")) @@ -1409,12 +1410,12 @@ func InitServer(ctx context.Context, currentServers server_structs.ServerType) e return err } - // Reset issuerPrivateJWK to ensure test cases can use their own temp IssuerKey - ResetIssuerJWKPtr() + // Reset IssuerKeys to ensure test cases can use their own temp IssuerKeysDirectory + ResetIssuerPrivateKeys() // As necessary, generate private keys, JWKS and corresponding certs - // Note: This function will generate a private key in the location stored by the viper var "IssuerKey" + // Note: This function will generate a private key in the location stored by the viper var "IssuerKeysDirectory" // iff there isn't any valid private key present in that location _, err = GetIssuerPublicJWKS() if err != nil { @@ -1465,6 +1466,8 @@ func SetClientDefaults(v *viper.Viper) error { configDir := v.GetString("ConfigDir") v.SetDefault(param.IssuerKey.GetName(), filepath.Join(configDir, "issuer.jwk")) + v.SetDefault(param.IssuerKeysDirectory.GetName(), filepath.Join(configDir, "issuer-keys")) + upperPrefix := GetPreferredPrefix() if upperPrefix == OsdfPrefix || upperPrefix == StashPrefix { v.SetDefault("Federation.TopologyNamespaceURL", "https://topology.opensciencegrid.org/osdf/namespaces") @@ -1618,7 +1621,8 @@ func ResetConfig() { globalFedInfo = pelican_url.FederationDiscovery{} globalFedErr = nil - ResetIssuerJWKPtr() + ResetIssuerPrivateKeys() + ResetClientInitialized() // other than what's above, resetting Origin exports will be done by ResetTestState() in server_utils pkg diff --git a/config/encrypted.go b/config/encrypted.go index 189edde28..155eb3786 100644 --- a/config/encrypted.go +++ b/config/encrypted.go @@ -22,7 +22,6 @@ import ( "crypto/aes" "crypto/cipher" "crypto/ed25519" - "crypto/elliptic" "crypto/rand" "crypto/sha256" "crypto/sha512" @@ -42,8 +41,6 @@ import ( "golang.org/x/crypto/nacl/box" "golang.org/x/term" "gopkg.in/yaml.v3" - - "github.com/pelicanplatform/pelican/param" ) // If we prompted the user for a new password while setting up the file, @@ -380,17 +377,18 @@ func SaveConfigContents_internal(config *OSDFConfig, forcePassword bool) error { // Take a hash, and use the hash's bytes as the secret. func GetSecret() (string, error) { // Use issuer private key as the source to generate the secret - issuerKeyFile := param.IssuerKey.GetString() - err := GeneratePrivateKey(issuerKeyFile, elliptic.P256(), false) + privateKey, err := GetIssuerPrivateJWK() if err != nil { return "", err } - privateKey, err := LoadPrivateKey(issuerKeyFile, false) - if err != nil { + + // Extract the underlying ECDSA private key in native Go crypto key type + var rawKey interface{} + if err := privateKey.Raw(&rawKey); err != nil { return "", err } - derPrivateKey, err := x509.MarshalPKCS8PrivateKey(privateKey) + derPrivateKey, err := x509.MarshalPKCS8PrivateKey(rawKey) if err != nil { return "", err diff --git a/config/encrypted_test.go b/config/encrypted_test.go index bd100ea64..6fc3cefff 100644 --- a/config/encrypted_test.go +++ b/config/encrypted_test.go @@ -37,8 +37,8 @@ func TestGetSecret(t *testing.T) { }) t.Run("generate-32B-hash", func(t *testing.T) { tmp := t.TempDir() - keyName := filepath.Join(tmp, "issuer.key") - viper.Set(param.IssuerKey.GetName(), keyName) + keyDir := filepath.Join(tmp, "issuer-keys") + viper.Set(param.IssuerKeysDirectory.GetName(), keyDir) get, err := GetSecret() require.NoError(t, err) @@ -55,8 +55,8 @@ func TestEncryptString(t *testing.T) { t.Run("encrypt-without-err", func(t *testing.T) { tmp := t.TempDir() - keyName := filepath.Join(tmp, "issuer.key") - viper.Set(param.IssuerKey.GetName(), keyName) + keyDir := filepath.Join(tmp, "issuer-keys") + viper.Set(param.IssuerKeysDirectory.GetName(), keyDir) get, err := EncryptString("Some secret to encrypt") require.NoError(t, err) @@ -72,8 +72,8 @@ func TestDecryptString(t *testing.T) { }) t.Run("decrypt-without-err", func(t *testing.T) { tmp := t.TempDir() - keyName := filepath.Join(tmp, "issuer.key") - viper.Set(param.IssuerKey.GetName(), keyName) + keyDir := filepath.Join(tmp, "issuer-keys") + viper.Set(param.IssuerKeysDirectory.GetName(), keyDir) secret := "Some secret to encrypt" @@ -88,8 +88,8 @@ func TestDecryptString(t *testing.T) { t.Run("diff-secrets-yield-diff-result", func(t *testing.T) { tmp := t.TempDir() - keyName := filepath.Join(tmp, "issuer.key") - viper.Set(param.IssuerKey.GetName(), keyName) + keyDir := filepath.Join(tmp, "issuer-keys") + viper.Set(param.IssuerKeysDirectory.GetName(), keyDir) secret := "Some secret to encrypt" @@ -97,8 +97,9 @@ func TestDecryptString(t *testing.T) { require.NoError(t, err) assert.NotEmpty(t, getEncrypt) - newKeyName := filepath.Join(tmp, "new-issuer.key") - viper.Set(param.IssuerKey.GetName(), newKeyName) + ResetConfig() + newKeyDir := filepath.Join(tmp, "new-issuer-keys") + viper.Set(param.IssuerKeysDirectory.GetName(), newKeyDir) getDecrypt, err := DecryptString(getEncrypt) require.NoError(t, err) diff --git a/config/init_server_creds.go b/config/init_server_creds.go index 8d2772311..be72c0a9e 100644 --- a/config/init_server_creds.go +++ b/config/init_server_creds.go @@ -28,11 +28,13 @@ import ( "crypto/x509/pkix" "encoding/pem" "fmt" + "io/fs" "math/big" "os" "os/exec" "path/filepath" "runtime" + "sync" "sync/atomic" "time" @@ -44,15 +46,77 @@ import ( "github.com/pelicanplatform/pelican/param" ) +type IssuerKeys struct { + // CurrentKey is the private key used to sign tokens and payloads. It corresponds to the + // private key with the highest lexicographical filename among the legacy key file + // (if present) and all .pem files in IssuerKeyDirectory. + CurrentKey jwk.Key + + // AllKeys holds all valid private keys as a [keyID:key] map, including those from .pem files + // in IssuerKeyDirectory and legacy key file at IssuerKey (if exists). A token or + // payload signature is considered valid if any of these keys could have produced it. + AllKeys map[string]jwk.Key +} + var ( - // This is the private JWK for the server to sign tokens. This key remains - // the same if the IssuerKey is unchanged - issuerPrivateJWK atomic.Pointer[jwk.Key] + issuerKeys atomic.Pointer[IssuerKeys] + + // Used to ensure initialization func init() is only called once + initOnce sync.Once ) -// Reset the atomic pointer to issuer private jwk -func ResetIssuerJWKPtr() { - issuerPrivateJWK.Store(nil) +// Set a private key as the issuer key +func setIssuerKey(key jwk.Key) { + newKeys := IssuerKeys{ + CurrentKey: key, + AllKeys: getIssuerPrivateKeysCopy(), // Get a copy of the existing keys + } + newKeys.AllKeys[key.KeyID()] = key // Add the new key to the copy + issuerKeys.Store(&newKeys) +} + +// Resets the entire keys struct, including current and all keys. CurrentKey is implicitly set to nil +func ResetIssuerPrivateKeys() { + issuerKeys.Store(&IssuerKeys{ + AllKeys: make(map[string]jwk.Key), + }) +} + +// Safely load the current map and create a copy for modification +func getIssuerPrivateKeysCopy() map[string]jwk.Key { + currentKeysPtr := issuerKeys.Load() + if currentKeysPtr == nil { + return make(map[string]jwk.Key) + } + + currentKeys := *currentKeysPtr + newMap := make(map[string]jwk.Key, len(currentKeys.AllKeys)) + for k, v := range currentKeys.AllKeys { + newMap[k] = v + } + return newMap +} + +// Read the current map +func GetIssuerPrivateKeys() map[string]jwk.Key { + keysPtr := issuerKeys.Load() + if keysPtr == nil { + return make(map[string]jwk.Key) + } + + return (*keysPtr).AllKeys +} + +// Helper function to create a directory and set proper permissions to save private keys +func createDirForKeys(dir string) error { + gid, err := GetDaemonGID() + if err != nil { + return errors.Wrap(err, "failed to get deamon gid") + } + if err := MkdirAll(dir, 0750, -1, gid); err != nil { + return errors.Wrapf(err, "failed to set the permission of %s", dir) + } + return nil } // Return a pointer to an ECDSA private key or RSA private key read from keyLocation. @@ -155,7 +219,7 @@ func GeneratePrivateKey(keyLocation string, curve elliptic.Curve, allowRSA bool) } // If we're generating a new key, log a warning in case the user intended to pass an existing key (maybe they made a typo) - log.Warningf("IssuerKey is set to %v but the file does not exist. Will generate a new private key", param.IssuerKey.GetString()) + log.Warningf("Will generate a new private key at location: %v", keyLocation) keyDir := filepath.Dir(keyLocation) if err := MkdirAll(keyDir, 0750, -1, gid); err != nil { @@ -503,44 +567,186 @@ func GenerateCert() error { return nil } -// Helper function to load the issuer/server's private key to sign tokens it issues. -// Only intended to be called internally -func loadIssuerPrivateJWK(issuerKeyFile string) (jwk.Key, error) { - // Check to see if we already had an IssuerKey or generate one - if err := GeneratePrivateKey(issuerKeyFile, elliptic.P256(), false); err != nil { - return nil, errors.Wrap(err, "Failed to generate new private key") - } - contents, err := os.ReadFile(issuerKeyFile) +// Helper function to initialize the in-memory map to save all private keys +func initKeysMap() { + initialMap := make(map[string]jwk.Key) + issuerKeys.Store(&IssuerKeys{ + AllKeys: initialMap, + }) +} + +// Helper function to load one .pem file from specified filename +func loadSinglePEM(path string) (jwk.Key, error) { + contents, err := os.ReadFile(path) if err != nil { - return nil, errors.Wrap(err, "Failed to read issuer key file") + return nil, errors.Wrap(err, "failed to read key file") } + key, err := jwk.ParseKey(contents, jwk.WithPEM(true)) if err != nil { - return nil, errors.Wrapf(err, "Failed to parse issuer key file %v", issuerKeyFile) + return nil, errors.Wrapf(err, "failed to parse issuer key file %v", path) } // Add the algorithm to the key, needed for verifying tokens elsewhere - err = key.Set(jwk.AlgorithmKey, jwa.ES256) + if err := key.Set(jwk.AlgorithmKey, jwa.ES256); err != nil { + return nil, errors.Wrap(err, "failed to set algorithm") + } + + // Ensure key has an ID + if err := jwk.AssignKeyID(key); err != nil { + return nil, errors.Wrap(err, "failed to assign key ID") + } + + return key, nil +} + +// Helper function to load/refresh all key files from both legacy IssuerKey file and specified directory +// find the most recent private key based on lexicographical order of their filenames +func loadPEMFiles(dir string) (jwk.Key, error) { + var mostRecentKey jwk.Key + var mostRecentFileName string + latestKeys := getIssuerPrivateKeysCopy() + + // Load legacy private key if it exists - parsing the file at IssuerKey act as if it is included in IssuerKeysDirectory + issuerKeyPath := param.IssuerKey.GetString() + if issuerKeyPath != "" { + if _, err := os.Stat(issuerKeyPath); err == nil { + issuerKey, err := loadSinglePEM(issuerKeyPath) + if err != nil { + log.Warnf("Failed to load key %s: %v", issuerKeyPath, err) + } else { + latestKeys[issuerKey.KeyID()] = issuerKey + if mostRecentFileName == "" || filepath.Base(issuerKeyPath) > mostRecentFileName { + mostRecentFileName = filepath.Base(issuerKeyPath) + mostRecentKey = issuerKey + } + } + } + } + + // Ensure input directory dir exists, if not, create it with proper permissions + if _, err := os.Stat(dir); os.IsNotExist(err) { + if err := createDirForKeys(dir); err != nil { + return nil, errors.Wrapf(err, "failed to create directory and set permissions: %s", dir) + } + } + // Traverse the directory for .pem files in lexical order + err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + if !d.IsDir() && filepath.Ext(d.Name()) == ".pem" { + // Parse the private key in this file and add to the in-memory keys map + key, err := loadSinglePEM(path) + if err != nil { + log.Warnf("Failed to load key %s: %v", path, err) + return nil // Skip this file and continue + } + + latestKeys[key.KeyID()] = key + + // Update the most recent key based on lexicographical order of filenames + if mostRecentFileName == "" || d.Name() > mostRecentFileName { + mostRecentFileName = d.Name() + mostRecentKey = key + } + } + return nil + }) + if err != nil { - return nil, errors.Wrap(err, "Failed to add alg specification to key header") + return nil, errors.Wrapf(err, "failed to traverse directory %s that stores private keys", dir) + } + + // Create a new private key and set as issuer key when neither legacy private key at IssuerKey + // nor any .pem file at IssuerKeysDirectory exists + if len(latestKeys) == 0 || mostRecentKey == nil { + newKey, err := generatePEMandSetIssuerKey(dir) + if err != nil { + return nil, errors.Wrapf(err, "failed to create a new .pem file to save private key") + } + return newKey, nil + } + + // Save current key and all up-to-date valid private keys and the in-memory issuerKeys + newKeys := IssuerKeys{ + CurrentKey: mostRecentKey, + AllKeys: latestKeys, } + issuerKeys.Store(&newKeys) + log.Debugf("Set private key %s as the issuer key", mostRecentKey.KeyID()) + + return mostRecentKey, nil +} - // Assign key id to the private key so that the public key obtainer thereafter - // has the same kid - err = jwk.AssignKeyID(key) +// Create a new .pem file (combining GeneratePrivateKey and LoadPrivateKey functions) +func GeneratePEM(dir string) (jwk.Key, error) { + // Generate a unique filename using a POSIX mkstemp-like logic + // Create a temp file, store its filename, then immediately delete this temp file + filenamePattern := fmt.Sprintf("pelican_generated_%d_*.pem", + time.Now().UnixNano()) + if err := createDirForKeys(dir); err != nil { + return nil, errors.Wrapf(err, "failed to create directory and set permissions: %s", dir) + } + tempFile, err := os.CreateTemp(dir, filenamePattern) if err != nil { - return nil, errors.Wrap(err, "Failed to assign key ID to private key") + return nil, errors.Wrap(err, "failed to remove temp file") + } + keyPath := tempFile.Name() + if err := tempFile.Close(); err != nil { + return nil, errors.Wrap(err, "failed to close temp file") + } + if err := os.Remove(keyPath); err != nil { + return nil, errors.Wrap(err, "failed to remove temp file") } - // Store the key in the in-memory cache - issuerPrivateJWK.Store(&key) + if err := GeneratePrivateKey(keyPath, elliptic.P256(), false); err != nil { + return nil, errors.Wrapf(err, "failed to generate new private key at %s", keyPath) + } + key, err := loadSinglePEM(keyPath) + if err != nil { + log.Errorf("Failed to load key %s: %v", keyPath, err) + return nil, errors.Wrapf(err, "failed to load key from %s", keyPath) + } + + log.Debugf("Generated private key %s", key.KeyID()) return key, nil } +// Generate a new .pem file and then set the private key it contains as the issuer key +func generatePEMandSetIssuerKey(dir string) (jwk.Key, error) { + newKey, err := GeneratePEM(dir) + if err != nil { + return nil, errors.Wrapf(err, "failed to create a new .pem file to save private key") + } + + setIssuerKey(newKey) + + return newKey, nil +} + +// Re-scan the disk to load the current valid private keys, return the issuer key to sign tokens it issues +// The issuer key is the key with the highest lexicographical filename +func loadIssuerPrivateKey(issuerKeysDir string) (jwk.Key, error) { + // Ensure initKeysMap is only called once across the program’s runtime + initOnce.Do(func() { + initKeysMap() + }) + + issuerKey, err := loadPEMFiles(issuerKeysDir) + if err != nil { + return nil, errors.Wrapf(err, `failed to re-scan %s to load .pem files and set the key file with the + highest lexicographical order as the current issuer key`, issuerKeysDir) + } + + return issuerKey, err +} + // Helper function to load the issuer/server's public key for other servers // to verify the token signed by this server. Only intended to be called internally -func loadIssuerPublicJWKS(existingJWKS string, issuerKeyFile string) (jwk.Set, error) { +func loadIssuerPublicJWKS(existingJWKS string, issuerKeysDir string) (jwk.Set, error) { jwks := jwk.NewSet() if existingJWKS != "" { var err error @@ -549,40 +755,53 @@ func loadIssuerPublicJWKS(existingJWKS string, issuerKeyFile string) (jwk.Set, e return nil, errors.Wrap(err, "Failed to read issuer JWKS file") } } - key := issuerPrivateJWK.Load() - if key == nil { - // This returns issuerPrivateJWK if it's non-nil, or find and parse private JWK - // located at IssuerKey if there is one, or generate a new private key - loadedKey, err := loadIssuerPrivateJWK(issuerKeyFile) + keys := GetIssuerPrivateKeys() + if len(keys) == 0 { + // Retrieve issuerPrivateKeys if it's non-empty, or find and parse all private key + // files on disk, or generate a new private key + _, err := loadIssuerPrivateKey(issuerKeysDir) if err != nil { return nil, errors.Wrap(err, "Failed to load issuer private JWK") } - key = &loadedKey + // Reload the keys after the key refresh/creation + keys = GetIssuerPrivateKeys() } - pkey, err := jwk.PublicKeyOf(*key) - if err != nil { - return nil, errors.Wrapf(err, "Failed to generate public key from file %v", issuerKeyFile) - } + // Traverse all private keys and add their public keys to the JWKS + for _, key := range keys { + pkey, err := jwk.PublicKeyOf(key) + if err != nil { + return nil, errors.Wrapf(err, "failed to generate public key from key %s", key.KeyID()) + } - if err = jwks.AddKey(pkey); err != nil { - return nil, errors.Wrap(err, "Failed to add public key to new JWKS") + if err = jwks.AddKey(pkey); err != nil { + return nil, errors.Wrapf(err, "Failed to add public key %s to new JWKS", key.KeyID()) + } } return jwks, nil } -// Return the private JWK for the server to sign tokens +// Return the private JWK for the server to sign tokens and payloads func GetIssuerPrivateJWK() (jwk.Key, error) { - key := issuerPrivateJWK.Load() - if key == nil { - issuerKeyFile := param.IssuerKey.GetString() - newKey, err := loadIssuerPrivateJWK(issuerKeyFile) + keysPtr := issuerKeys.Load() + issuerKeysDir := param.IssuerKeysDirectory.GetString() + + // Re-scan the private keys dir when no issuer key in memory + if keysPtr == nil || keysPtr.CurrentKey == nil { + newKey, err := loadIssuerPrivateKey(issuerKeysDir) if err != nil { return nil, errors.Wrap(err, "Failed to load issuer private key") } - key = &newKey + newKeys := IssuerKeys{ + CurrentKey: newKey, + AllKeys: map[string]jwk.Key{newKey.KeyID(): newKey}, + } + + issuerKeys.Store(&newKeys) + + keysPtr = issuerKeys.Load() // Reload after store } - return *key, nil + return (*keysPtr).CurrentKey, nil } // Check if a valid JWKS file exists at Server_IssuerJwks, return that file if so; @@ -595,8 +814,8 @@ func GetIssuerPrivateJWK() (jwk.Key, error) { // i.e. "/.well-known/issuer.jwks" func GetIssuerPublicJWKS() (jwk.Set, error) { existingJWKS := param.Server_IssuerJwks.GetString() - issuerKeyFile := param.IssuerKey.GetString() - return loadIssuerPublicJWKS(existingJWKS, issuerKeyFile) + issuerKeysDir := param.IssuerKeysDirectory.GetString() + return loadIssuerPublicJWKS(existingJWKS, issuerKeysDir) } // Check if there is a session secret exists at param.Server_SessionSecretFile and is not empty if there is one. @@ -700,3 +919,36 @@ func LoadSessionSecret() ([]byte, error) { } return rest, nil } + +func areKeysDifferent(a, b map[string]jwk.Key) bool { + if len(a) != len(b) { + return true + } + + for key := range a { + if _, exists := b[key]; !exists { + return true + } + } + + for key := range b { + if _, exists := a[key]; !exists { + return true + } + } + + return false // All keys are the same +} + +func RefreshKeys() (bool, error) { + before := GetIssuerPrivateKeys() + _, err := loadIssuerPrivateKey(param.IssuerKeysDirectory.GetString()) + if err != nil { + return false, err + } + after := GetIssuerPrivateKeys() + keysChanged := areKeysDifferent(before, after) + + log.Debugf("Private keys directory refreshed successfully") + return keysChanged, nil +} diff --git a/config/init_server_creds_test.go b/config/init_server_creds_test.go index e29cc2f9d..7051eca38 100644 --- a/config/init_server_creds_test.go +++ b/config/init_server_creds_test.go @@ -150,3 +150,39 @@ func TestLoadPrivateKey(t *testing.T) { require.Nil(t, privateKey) }) } + +func TestMultiPrivateKey(t *testing.T) { + t.Run("generate-and-load-single-key", func(t *testing.T) { + ResetConfig() + defer ResetConfig() + tempDir := t.TempDir() + issuerKeysDir := filepath.Join(tempDir, "issuer-keys") + + key, err := loadIssuerPrivateKey(issuerKeysDir) + require.NoError(t, err) + require.NotNil(t, key) + }) + + // This test also imitates the origin API endpoint "/newIssuerKey" + t.Run("second-private-key", func(t *testing.T) { + ResetConfig() + defer ResetConfig() + tempDir := t.TempDir() + issuerKeysDir := filepath.Join(tempDir, "issuer-keys") + + key, err := loadIssuerPrivateKey(issuerKeysDir) + require.NoError(t, err) + require.NotNil(t, key) + + // Create another private key + secondKey, err := generatePEMandSetIssuerKey(issuerKeysDir) + require.NoError(t, err) + require.NotNil(t, secondKey) + assert.NotEqual(t, key.KeyID(), secondKey.KeyID()) + + // Check if the active private key points to the latest key + latestKey, err := GetIssuerPrivateJWK() + require.NoError(t, err) + assert.Equal(t, secondKey.KeyID(), latestKey.KeyID()) + }) +} diff --git a/director/director_test.go b/director/director_test.go index 887ab7f1b..1ffaceb0d 100644 --- a/director/director_test.go +++ b/director/director_test.go @@ -45,6 +45,7 @@ import ( "github.com/stretchr/testify/require" "github.com/pelicanplatform/pelican/config" + "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/server_structs" "github.com/pelicanplatform/pelican/server_utils" "github.com/pelicanplatform/pelican/test_utils" @@ -1345,8 +1346,8 @@ func TestDiscoverOriginCache(t *testing.T) { viper.Set("Server.ExternalWebUrl", mockDirectorUrl) tDir := t.TempDir() - kfile := filepath.Join(tDir, "testKey") - viper.Set("IssuerKey", kfile) + kDir := filepath.Join(tDir, "testKeyDir") + viper.Set(param.IssuerKeysDirectory.GetName(), kDir) viper.Set("ConfigDir", t.TempDir()) config.InitConfig() diff --git a/director/origin_api_test.go b/director/origin_api_test.go index 1c3c350d7..5f676d305 100644 --- a/director/origin_api_test.go +++ b/director/origin_api_test.go @@ -35,6 +35,7 @@ import ( "golang.org/x/sync/errgroup" "github.com/pelicanplatform/pelican/config" + "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/server_structs" "github.com/pelicanplatform/pelican/server_utils" "github.com/pelicanplatform/pelican/test_utils" @@ -50,10 +51,10 @@ func TestVerifyAdvertiseToken(t *testing.T) { server_utils.ResetTestState() tDir := t.TempDir() - kfile := filepath.Join(tDir, "t-key") + kDir := filepath.Join(tDir, "t-issuer-keys") //Setup a private key and a token - viper.Set("IssuerKey", kfile) + viper.Set(param.IssuerKeysDirectory.GetName(), kDir) viper.Set("Federation.DirectorURL", "https://director-url.org") diff --git a/director/stat_test.go b/director/stat_test.go index 49ea263ef..d5270c70b 100644 --- a/director/stat_test.go +++ b/director/stat_test.go @@ -36,6 +36,7 @@ import ( "github.com/stretchr/testify/require" "github.com/pelicanplatform/pelican/config" + "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/server_structs" "github.com/pelicanplatform/pelican/server_utils" "github.com/pelicanplatform/pelican/utils" @@ -827,8 +828,8 @@ func TestSendHeadReq(t *testing.T) { mockOriginAd.URL = *realServerUrl tDir := t.TempDir() - kfile := filepath.Join(tDir, "testKey") - viper.Set("IssuerKey", kfile) + kDir := filepath.Join(tDir, "testKeyDir") + viper.Set(param.IssuerKeysDirectory.GetName(), kDir) viper.Set("ConfigDir", t.TempDir()) config.InitConfig() diff --git a/docs/parameters.yaml b/docs/parameters.yaml index 6bfe6c10b..ddf91e144 100644 --- a/docs/parameters.yaml +++ b/docs/parameters.yaml @@ -64,14 +64,29 @@ components: ["cache", "director", "origin", "registry"] --- name: IssuerKey description: |+ + [Deprecated] Use IssuerKeysDirectory instead. The key file set by this parameter will automatically move to IssuerKeysDirectory. + A filepath to the file containing a PEM-encoded ecdsa private key which later will be parsed into a JWK and serves as the private key to sign various JWTs issued by this server. A public JWK will be derived from this private key and used as the key for token verification. type: filename +deprecated: true +replacedby: none root_default: /etc/pelican/issuer.jwk default: $ConfigBase/issuer.jwk -components: ["client", "registry", "director"] +components: ["origin", "cache", "registry", "director"] +--- +name: IssuerKeysDirectory +description: |+ + A filepath to the directory used for storing one or multiple PEM-encoded ecdsa private keys. The most recent modified + private key will be parsed into a JWK and serves as the active private key to sign various JWTs issued by this server. + + A public JWK will be derived from this private key and used as the key for token verification. +type: filename +root_default: /etc/pelican/issuer-keys +default: $ConfigBase/issuer-keys +components: ["origin", "cache", "registry", "director"] --- name: Transport.DialerTimeout description: |+ diff --git a/launcher_utils/key_refresh_and_update_namespace_pubkey.go b/launcher_utils/key_refresh_and_update_namespace_pubkey.go new file mode 100644 index 000000000..7679ca047 --- /dev/null +++ b/launcher_utils/key_refresh_and_update_namespace_pubkey.go @@ -0,0 +1,141 @@ +/*************************************************************** + * + * Copyright (C) 2024, Pelican Project, Morgridge Institute for Research + * + * Licensed under the Apache License, Version 2.0 (the "License"); you + * may not use this file except in compliance with the License. You may + * obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ***************************************************************/ + +package launcher_utils + +import ( + "context" + "net/url" + "time" + + "github.com/lestrrat-go/jwx/v2/jwk" + "github.com/pkg/errors" + log "github.com/sirupsen/logrus" + "golang.org/x/sync/errgroup" + + "github.com/pelicanplatform/pelican/config" + "github.com/pelicanplatform/pelican/param" + "github.com/pelicanplatform/pelican/registry" + "github.com/pelicanplatform/pelican/server_structs" + "github.com/pelicanplatform/pelican/server_utils" +) + +func updateNamespacesPubKeyPrep(ctx context.Context, prefixes []string) (jwk.Key, string, error) { + // Validate the namespace format + for _, prefix := range prefixes { + if prefix == "" { + err := errors.New("Invalid empty prefix for public key update") + return nil, "", err + } + if prefix[0] != '/' { + err := errors.New("Prefix specified for public key update must start with a '/'") + return nil, "", err + } + } + + // Generate the endpoint url that can update the public key of prefixes + fedInfo, err := config.GetFederation(ctx) + if err != nil { + return nil, "", err + } + registryEndpoint := fedInfo.RegistryEndpoint + if registryEndpoint == "" { + err = errors.New("No registry endpoint specified; try passing the `-f` flag specifying the federation name") + return nil, "", err + } + + prefixPubKeyUpdateUrl, err := url.JoinPath(registryEndpoint, "api", "v1.0", "registry", "updateNamespacesPubKey") + if err != nil { + err = errors.Wrap(err, "Failed to construct public key update endpoint URL: %v") + return nil, "", err + } + + // Obtain server's issuer private key + key, err := config.GetIssuerPrivateJWK() + if err != nil { + err = errors.Wrap(err, "Failed to obtain server's issuer private key") + return nil, "", err + } + + return key, prefixPubKeyUpdateUrl, nil +} + +func updateNamespacesPubKey(ctx context.Context, prefixes []string) error { + siteName := param.Xrootd_Sitename.GetString() + + key, url, err := updateNamespacesPubKeyPrep(ctx, prefixes) + if err != nil { + return err + } + if err = registry.NamespacesPubKeyUpdate(key, prefixes, siteName, url); err != nil { + return err + } + return nil +} + +func triggerNamespacesPubKeyUpdate(ctx context.Context) error { + extUrlStr := param.Server_ExternalWebUrl.GetString() + extUrl, _ := url.Parse(extUrlStr) + namespace := server_structs.GetOriginNs(extUrl.Host) + if err := updateNamespacesPubKey(ctx, []string{namespace}); err != nil { + log.Errorf("Error updating the public key of the registered origin namespace %s: %v", namespace, err) + } + + originExports, err := server_utils.GetOriginExports() + if err != nil { + return err + } + originExportsNs := make([]string, len(originExports)) + for i, export := range originExports { + originExportsNs[i] = export.FederationPrefix + } + if err := updateNamespacesPubKey(ctx, originExportsNs); err != nil { + log.Errorf("Error updating the public key of origin-exported namespace(s): %v", err) + } + return nil +} + +// Check the directory containing .pem files regularly, load new private key(s) +// For origin server, if new file(s) are detected, then register the new public key +func LaunchIssuerKeysDirRefresh(ctx context.Context, egrp *errgroup.Group, modules server_structs.ServerType) { + egrp.Go(func() error { + ticker := time.NewTicker(5 * time.Minute) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + log.Debugln("Stopping periodic check for private keys directory.") + return nil + case <-ticker.C: + // Refresh the disk to pick up any new private key + keysChanged, err := config.RefreshKeys() + if err != nil { + return nil + } + + // Update public key registered with namespace in registry db when the private key(s) changed in an origin + if modules.IsEnabled(server_structs.OriginType) && keysChanged { + if err = triggerNamespacesPubKeyUpdate(ctx); err != nil { + return err + } + } + } + } + }) +} diff --git a/launcher_utils/register_namespace_test.go b/launcher_utils/register_namespace_test.go index f6e273b63..f303e02fb 100644 --- a/launcher_utils/register_namespace_test.go +++ b/launcher_utils/register_namespace_test.go @@ -46,6 +46,9 @@ import ( ) func TestRegistration(t *testing.T) { + t.Cleanup(func() { + server_utils.ResetTestState() + }) // Use a temp os directory to better control the deletion of the directory. // Fixes issue on Windows where we are trying to delete a file in use so this // better waits for the file/process to be shut down before deletion @@ -59,6 +62,8 @@ func TestRegistration(t *testing.T) { server_utils.ResetTestState() viper.Set("ConfigDir", tempConfigDir) + keysDir := filepath.Join(tempConfigDir, "issuer-keys") + viper.Set(param.IssuerKeysDirectory.GetName(), keysDir) config.InitConfig() viper.Set("Registry.DbLocation", filepath.Join(tempConfigDir, "test.sql")) @@ -159,10 +164,158 @@ func TestRegistration(t *testing.T) { assert.NoError(t, err) assert.Equal(t, keyStatus, noKeyPresent) - // Redo the namespace prep, ensure that isPresent is true + // Redo the namespace prep, ensure that isRegistered is true prefix = param.Origin_FederationPrefix.GetString() _, registerURL, isRegistered, err = registerNamespacePrep(ctx, prefix) + assert.True(t, isRegistered) assert.Equal(t, svr.URL+"/api/v1.0/registry", registerURL) assert.NoError(t, err) +} + +func TestMultiKeysRegistration(t *testing.T) { + t.Cleanup(func() { + server_utils.ResetTestState() + }) + // Use a temp os directory to better control the deletion of the directory. + // Fixes issue on Windows where we are trying to delete a file in use so this + // better waits for the file/process to be shut down before deletion + tempConfigDir, err := os.MkdirTemp("", "test") + assert.NoError(t, err) + defer os.RemoveAll(tempConfigDir) + + ctx, cancel, egrp := test_utils.TestContext(context.Background(), t) + defer func() { require.NoError(t, egrp.Wait()) }() + defer cancel() + + server_utils.ResetTestState() + viper.Set("ConfigDir", tempConfigDir) + keysDir := filepath.Join(tempConfigDir, "issuer-keys") + viper.Set(param.IssuerKeysDirectory.GetName(), keysDir) + + config.InitConfig() + viper.Set("Registry.DbLocation", filepath.Join(tempConfigDir, "test.sql")) + err = config.InitServer(ctx, server_structs.OriginType) + require.NoError(t, err) + + err = registry.InitializeDB() + require.NoError(t, err) + defer func() { + err := registry.ShutdownRegistryDB() + assert.NoError(t, err) + }() + + gin.SetMode(gin.TestMode) + engine := gin.Default() + + // Ensure we have a issuer key + _, err = config.GetIssuerPublicJWKS() + require.NoError(t, err) + privKey, err := config.GetIssuerPrivateJWK() + require.NoError(t, err) + key, err := privKey.PublicKey() + require.NoError(t, err) + assert.NoError(t, jwk.AssignKeyID(key)) + keyId := key.KeyID() + require.NotEmpty(t, keyId) + keysMap := config.GetIssuerPrivateKeys() + require.Equal(t, 1, len(keysMap)) + + // Create a new issuer key and rotate out the old one + secondKey, err := config.GeneratePEM(keysDir) + require.NoError(t, err) + require.NotEqual(t, privKey.KeyID(), secondKey.KeyID()) + keysChange, err := config.RefreshKeys() + require.True(t, keysChange) + require.NoError(t, err) + secondPubKey, err := secondKey.PublicKey() + require.NoError(t, err) + activeKey, err := config.GetIssuerPrivateJWK() + require.NoError(t, err) + require.Equal(t, secondKey, activeKey) + keysMap = config.GetIssuerPrivateKeys() + require.Equal(t, secondKey, keysMap[secondKey.KeyID()]) + require.Equal(t, privKey, keysMap[key.KeyID()]) + require.Equal(t, 2, len(keysMap)) + secondKeyId := secondKey.KeyID() + require.NotEmpty(t, keyId) + + //Configure registry + registry.RegisterRegistryAPI(engine.Group("/")) + + //Create a test HTTP server that sends requests to gin + svr := httptest.NewServer(engine) + defer svr.CloseClientConnections() + defer svr.Close() + + viper.Set("Federation.RegistryUrl", svr.URL) + viper.Set("Origin.FederationPrefix", "/test123") + + // Re-run the InitServer to reflect the new RegistryUrl set above + require.NoError(t, config.InitServer(ctx, server_structs.OriginType)) + + // Test registration succeeds + prefix := param.Origin_FederationPrefix.GetString() + key, registerURL, isRegistered, err := registerNamespacePrep(ctx, prefix) + require.NoError(t, err) + assert.False(t, isRegistered) + assert.Equal(t, registerURL, svr.URL+"/api/v1.0/registry") + err = registerNamespaceImpl(key, prefix, "mock_site_name", registerURL) + require.NoError(t, err) + + // Test we can query for the new key + req, err := http.NewRequest("GET", svr.URL+"/api/v1.0/registry", nil) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + tr := config.GetTransport() + client := &http.Client{Transport: tr} + + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, 200, resp.StatusCode) + + // Test new key is the same one we registered. + entries := []server_structs.Namespace{} + err = json.Unmarshal(body, &entries) + require.NoError(t, err) + require.Equal(t, len(entries), 1) + assert.Equal(t, entries[0].Prefix, "/test123") + keySet, err := jwk.Parse([]byte(entries[0].Pubkey)) + require.NoError(t, err) + registryKey, isPresent := keySet.LookupKeyID(secondKeyId) + assert.True(t, isPresent) + assert.True(t, jwk.Equal(registryKey, secondPubKey)) + assert.Equal(t, "mock_site_name", entries[0].AdminMetadata.SiteName) + + // Test the functionality of the keyIsRegistered function + keyStatus, err := keyIsRegistered(key, svr.URL+"/api/v1.0/registry", "/test123") + assert.NoError(t, err) + require.Equal(t, keyStatus, keyMatch) + + // Generate a new key, test we get mismatch + privKeyAltRaw, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + privKeyAlt, err := jwk.FromRaw(privKeyAltRaw) + require.NoError(t, err) + keyAlt, err := privKeyAlt.PublicKey() + require.NoError(t, err) + assert.NoError(t, jwk.AssignKeyID(keyAlt)) + keyStatus, err = keyIsRegistered(keyAlt, svr.URL+"/api/v1.0/registry", "/test123") + assert.NoError(t, err) + assert.Equal(t, keyStatus, keyMismatch) + + // Verify that no key is registered for an alternate prefix + keyStatus, err = keyIsRegistered(key, svr.URL, "test456") + assert.NoError(t, err) + assert.Equal(t, keyStatus, noKeyPresent) + + // Redo the namespace prep, ensure that isRegistered is true + prefix = param.Origin_FederationPrefix.GetString() + _, registerURL, isRegistered, err = registerNamespacePrep(ctx, prefix) + require.NoError(t, err) assert.True(t, isRegistered) + assert.Equal(t, svr.URL+"/api/v1.0/registry", registerURL) } diff --git a/launchers/launcher.go b/launchers/launcher.go index 3eef5342d..443f10a07 100644 --- a/launchers/launcher.go +++ b/launchers/launcher.go @@ -196,6 +196,13 @@ func LaunchModules(ctx context.Context, modules server_structs.ServerType) (serv lc.Register(ctx, rootGroup) } + // 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) + } + log.Info("Starting web engine...") lnReference = nil egrp.Go(func() error { diff --git a/origin/origin_db_test.go b/origin/origin_db_test.go index eda64c907..80a707463 100644 --- a/origin/origin_db_test.go +++ b/origin/origin_db_test.go @@ -62,8 +62,8 @@ func setupMockOriginDB(t *testing.T) { // Setup encryption tmp := t.TempDir() - keyName := filepath.Join(tmp, "issuer.key") - viper.Set(param.IssuerKey.GetName(), keyName) + keyDir := filepath.Join(tmp, "issuer-keys") + viper.Set(param.IssuerKeysDirectory.GetName(), keyDir) // Also update the refresh token to be encrypted for idx := range mockGC { diff --git a/param/parameters.go b/param/parameters.go index 6f0fec797..b0db767db 100644 --- a/param/parameters.go +++ b/param/parameters.go @@ -56,6 +56,7 @@ func GetDeprecated() map[string][]string { "Director.EnableStat": {"Director.CheckOriginPresence"}, "DisableHttpProxy": {"Client.DisableHttpProxy"}, "DisableProxyFallback": {"Client.DisableProxyFallback"}, + "IssuerKey": {"none"}, "MinimumDownloadSpeed": {"Client.MinimumDownloadSpeed"}, "Origin.EnableDirListing": {"Origin.EnableListings"}, "Origin.EnableFallbackRead": {"Origin.EnableDirectReads"}, @@ -167,6 +168,7 @@ var ( Federation_TopologyNamespaceUrl = StringParam{"Federation.TopologyNamespaceUrl"} Federation_TopologyUrl = StringParam{"Federation.TopologyUrl"} IssuerKey = StringParam{"IssuerKey"} + IssuerKeysDirectory = StringParam{"IssuerKeysDirectory"} Issuer_AuthenticationSource = StringParam{"Issuer.AuthenticationSource"} Issuer_GroupFile = StringParam{"Issuer.GroupFile"} Issuer_GroupSource = StringParam{"Issuer.GroupSource"} diff --git a/param/parameters_struct.go b/param/parameters_struct.go index c51990013..99b976ff2 100644 --- a/param/parameters_struct.go +++ b/param/parameters_struct.go @@ -121,6 +121,7 @@ type Config struct { UserStripDomain bool `mapstructure:"userstripdomain" yaml:"UserStripDomain"` } `mapstructure:"issuer" yaml:"Issuer"` IssuerKey string `mapstructure:"issuerkey" yaml:"IssuerKey"` + IssuerKeysDirectory string `mapstructure:"issuerkeysdirectory" yaml:"IssuerKeysDirectory"` LocalCache struct { DataLocation string `mapstructure:"datalocation" yaml:"DataLocation"` HighWaterMarkPercentage int `mapstructure:"highwatermarkpercentage" yaml:"HighWaterMarkPercentage"` @@ -434,6 +435,7 @@ type configWithType struct { UserStripDomain struct { Type string; Value bool } } IssuerKey struct { Type string; Value string } + IssuerKeysDirectory struct { Type string; Value string } LocalCache struct { DataLocation struct { Type string; Value string } HighWaterMarkPercentage struct { Type string; Value int } diff --git a/registry/client_commands.go b/registry/client_commands.go index c56f481bf..23b5332dd 100644 --- a/registry/client_commands.go +++ b/registry/client_commands.go @@ -26,6 +26,7 @@ import ( "encoding/json" "fmt" "os" + "strings" "time" "github.com/lestrrat-go/jwx/v2/jwk" @@ -40,15 +41,16 @@ import ( ) type clientResponseData struct { - VerificationURL string `json:"verification_url"` - DeviceCode string `json:"device_code"` - Status string `json:"status"` - AccessToken string `json:"access_token"` - ServerNonce string `json:"server_nonce"` - ServerPayload string `json:"server_payload"` - ServerSignature string `json:"server_signature"` - Message string `json:"msg"` - Error string `json:"error"` + VerificationURL string `json:"verification_url"` + DeviceCode string `json:"device_code"` + Status string `json:"status"` + AccessToken string `json:"access_token"` + ServerNonce string `json:"server_nonce"` + ServerPayload string `json:"server_payload"` + ServerSignature string `json:"server_signature"` + RegisteredKeys map[string]string `json:"registered_keys"` + Message string `json:"msg"` + Error string `json:"error"` } func NamespaceRegisterWithIdentity(privateKey jwk.Key, namespaceRegistryEndpoint string, prefix string, siteName string) error { @@ -281,3 +283,165 @@ func NamespaceDelete(endpoint string, prefix string) error { fmt.Println(string(respData)) return nil } + +func NamespacesPubKeyUpdate(privateKey jwk.Key, prefixes []string, siteName string, namespacePubKeyEndUpdatepoint string) error { + publicKey, err := privateKey.PublicKey() + if err != nil { + return errors.Wrapf(err, "failed to generate public key for namespace registration") + } + if err = jwk.AssignKeyID(publicKey); err != nil { + return errors.Wrap(err, "failed to assign key ID to public key") + } + if err = publicKey.Set("alg", "ES256"); err != nil { + return errors.Wrap(err, "failed to assign signature algorithm to public key") + } + keySet := jwk.NewSet() + if err = keySet.AddKey(publicKey); err != nil { + return errors.Wrap(err, "failed to add public key to new JWKS") + } + + // Let's check that we can convert to JSON and get the right thing... + jsonbuf, err := json.Marshal(keySet) + if err != nil { + return errors.Wrap(err, "failed to marshal the public key into JWKS JSON") + } + log.Debugln("Constructed JWKS from loading public key:", string(jsonbuf)) + + clientNonce, err := generateNonce() + if err != nil { + return errors.Wrap(err, "failed to generate client nonce") + } + + data := map[string]interface{}{ + "client_nonce": clientNonce, + "pubkey": keySet, + "prefixes": prefixes, + } + + tr := config.GetTransport() + resp, err := utils.MakeRequest(context.Background(), tr, namespacePubKeyEndUpdatepoint, "POST", data, nil) + + var respData clientResponseData + // Handle case where there was an error encoded in the body + if err != nil { + if strings.Contains(err.Error(), "status code 404") { + log.Warnf("Registered namespace public key enUpdatedpoint returned 404 Not Found: %s. This endpoint is available in Pelican v7.12 or later.", namespacePubKeyEndUpdatepoint) + return nil + } + if unmarshalErr := json.Unmarshal(resp, &respData); unmarshalErr == nil { + return errors.Wrapf(err, "Server responded with an error: %s. %s", respData.Message, respData.Error) + } + return errors.Wrapf(err, "Server responded with an error and failed to parse JSON response from the server. Raw server response is %s", resp) + } + + // No error + if err = json.Unmarshal(resp, &respData); err != nil { + return errors.Wrapf(err, "Failure when parsing JSON response from the server with a successful request. Raw server response is %s", resp) + } + + // Create client payload by concatenating client_nonce and server_nonce + clientPayload := clientNonce + respData.ServerNonce + + // Sign the payload + privateKeyRaw := &ecdsa.PrivateKey{} + if err = privateKey.Raw(privateKeyRaw); err != nil { + return errors.Wrap(err, "failed to get an ECDSA private key") + } + signature, err := signPayload([]byte(clientPayload), privateKeyRaw) + if err != nil { + return errors.Wrap(err, "failed to sign payload") + } + + privateKeys := config.GetIssuerPrivateKeys() + if len(privateKeys) == 0 { + return errors.Errorf("The server doesn't have any private key in memory") + } + issuerPubKeysSet := jwk.NewSet() + for _, privKey := range privateKeys { + pubKey, err := privKey.PublicKey() + if err != nil { + return errors.Wrapf(err, "failed to get the public key of a private key") + } + if err = issuerPubKeysSet.AddKey(pubKey); err != nil { + return errors.Wrap(err, "failed to add public key to all keys JWKS") + } + } + + var matchedKeyId string + var keyUpdateAuthzSignature []byte + for _, prefix := range prefixes { + // Parse the public key(s) of the registered prefix + registeredKeys := respData.RegisteredKeys[prefix] + registryDbKeySet, err := jwk.ParseString(registeredKeys) + if err != nil { + log.Errorf("Failed to parse public key as JWKS of registered namespace %s: %v", prefix, err) + return errors.Wrapf(err, "Invalid public key format of registered namespace %s", prefix) + } + for i := 0; i < registryDbKeySet.Len(); i++ { + registeredPubKey, ok := registryDbKeySet.Key(i) + if !ok { + continue + } + // Look for the issuer public key this origin previously registered the namespace with + issuerPubKey, found := issuerPubKeysSet.LookupKeyID(registeredPubKey.KeyID()) + if !found { + continue + } + // Sign the payload with the issuer private key matched with the registered public key + issuerPrivKey := privateKeys[issuerPubKey.KeyID()] + rawkey := &ecdsa.PrivateKey{} + if err := issuerPrivKey.Raw(rawkey); err != nil { + return errors.Wrap(err, "failed to generate raw private key from the issuer private key matched with the registered public key") + } + if keyUpdateAuthzSignature, err = signPayload([]byte(clientPayload), rawkey); err != nil { + return errors.Wrap(err, "failed to sign the payload with the issuer private key matched with the registered public key") + } + matchedKeyId = issuerPubKey.KeyID() + break + } + } + if keyUpdateAuthzSignature == nil || matchedKeyId == "" { + return errors.Errorf("Origin does not possess valid key to update public keys of registered namespace(s)") + } + + // Create data for the second POST request + unidentifiedPayload := map[string]interface{}{ + "pubkey": keySet, + "all_pubkeys": issuerPubKeysSet, + "prefixes": prefixes, + "site_name": siteName, + "client_nonce": clientNonce, + "server_nonce": respData.ServerNonce, + "client_payload": clientPayload, + "client_signature": hex.EncodeToString(signature), + "key_update_authz_signature": hex.EncodeToString(keyUpdateAuthzSignature), + "matched_key_id": matchedKeyId, + "server_payload": respData.ServerPayload, + "server_signature": respData.ServerSignature, + } + + // Send the second POST request + resp, err = utils.MakeRequest(context.Background(), tr, namespacePubKeyEndUpdatepoint, "POST", unidentifiedPayload, nil) + + // Handle case where there was an error encoded in the body + if unmarshalErr := json.Unmarshal(resp, &respData); unmarshalErr == nil { + if err != nil { + if strings.Contains(err.Error(), "status code 404") { + log.Warnf("Registered namespace public key enUpdatedpoint returned 404 Not Found: %s. This endpoint is available in Pelican v7.12 or later.", namespacePubKeyEndUpdatepoint) + return nil + } + log.Errorf("Server responded with an error: %v. %s. %s", respData.Message, respData.Error, err) + return errors.Wrapf(err, "Server responded with an error: %s. %s", respData.Message, respData.Error) + } + if respData.Message != "" { + log.Debugf("Server responded to registration confirmation successfully with message: %s", respData.Message) + } + } else { // Error decoding JSON + if err != nil { + return errors.Wrapf(err, "Server responded with an error and failed to parse JSON response from the server. Raw response is %s", resp) + } + return errors.Wrapf(unmarshalErr, "Failure when parsing JSON response from the server with a successful request. Raw server response is %s", resp) + } + + return nil +} diff --git a/registry/client_commands_test.go b/registry/client_commands_test.go index 7adc1c900..7c949fda5 100644 --- a/registry/client_commands_test.go +++ b/registry/client_commands_test.go @@ -26,28 +26,32 @@ import ( "net/http/httptest" "os" "path/filepath" + "sort" "testing" "github.com/gin-gonic/gin" + "github.com/lestrrat-go/jwx/v2/jwk" "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/pelicanplatform/pelican/config" + "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/server_structs" "github.com/pelicanplatform/pelican/server_utils" "github.com/pelicanplatform/pelican/test_utils" ) func registryMockup(ctx context.Context, t *testing.T, testName string) *httptest.Server { + tDir := t.TempDir() + issuerTempDir := filepath.Join(tDir, testName) - issuerTempDir := filepath.Join(t.TempDir(), testName) - - ikey := filepath.Join(issuerTempDir, "issuer.jwk") - viper.Set("IssuerKey", ikey) + ikeyDir := filepath.Join(issuerTempDir, "issuer-keys") + viper.Set("IssuerKeysDirectory", ikeyDir) viper.Set("Registry.DbLocation", filepath.Join(issuerTempDir, "test.sql")) viper.Set("Server.WebPort", 8444) - config.InitConfigDir(viper.GetViper()) + viper.Set("ConfigDir", tDir) + config.InitConfig() err := config.InitServer(ctx, server_structs.RegistryType) require.NoError(t, err) @@ -66,11 +70,38 @@ func registryMockup(ctx context.Context, t *testing.T, testName string) *httptes return svr } +func getSortedKids(ctx context.Context, jsonStr string) ([]string, error) { + set, err := jwk.Parse([]byte(jsonStr)) + if err != nil { + return nil, err + } + var kids []string + keysIter := set.Keys(ctx) + for keysIter.Next(ctx) { + key := keysIter.Pair().Value.(jwk.Key) + + kid, ok := key.Get("kid") + if !ok { + continue + } + kids = append(kids, kid.(string)) + } + sort.Strings(kids) + + return kids, nil +} + func TestServeNamespaceRegistry(t *testing.T) { ctx, cancel, egrp := test_utils.TestContext(context.Background(), t) - defer func() { require.NoError(t, egrp.Wait()) }() - defer cancel() - + t.Cleanup(func() { + if r := recover(); r != nil { + t.Errorf("Test panicked: %v", r) + } + + cancel() + assert.NoError(t, egrp.Wait()) + server_utils.ResetTestState() + }) server_utils.ResetTestState() svr := registryMockup(ctx, t, "serveregistry") @@ -140,21 +171,210 @@ func TestServeNamespaceRegistry(t *testing.T) { stdoutCapture = string(capturedOutput[:n]) assert.Equal(t, "[]\n", stdoutCapture) }) +} + +func TestNamespaceRegisteredPubKeyUpdate(t *testing.T) { + ctx, cancel, egrp := test_utils.TestContext(context.Background(), t) + t.Cleanup(func() { + if r := recover(); r != nil { + t.Errorf("Test panicked: %v", r) + } + + cancel() + assert.NoError(t, egrp.Wait()) + server_utils.ResetTestState() + }) server_utils.ResetTestState() + + svr := registryMockup(ctx, t, "pub-key-update") + defer func() { + err := ShutdownRegistryDB() + assert.NoError(t, err) + svr.CloseClientConnections() + svr.Close() + }() + + _, err := config.GetIssuerPublicJWKS() + require.NoError(t, err) + privKey, err := config.GetIssuerPrivateJWK() + require.NoError(t, err) + + //Test functionality of registering a namespace (without identity) + err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/foo/bar", "mock_site_name") + require.NoError(t, err) + + t.Run("test-registered-namespace-pubkey-update-with-new-active-key", func(t *testing.T) { + // Imitate LaunchIssuerKeysDirRefresh function + newKey, err := config.GeneratePEM(param.IssuerKeysDirectory.GetString()) + require.NoError(t, err) + keysChange, err := config.RefreshKeys() + require.True(t, keysChange) + require.NoError(t, err) + privKey2, err := config.GetIssuerPrivateJWK() + require.NoError(t, err) + require.Equal(t, newKey.KeyID(), privKey2.KeyID()) + err = NamespacesPubKeyUpdate(privKey2, []string{"/foo/bar"}, "mock_site_name", svr.URL+"/api/v1.0/registry/updateNamespacesPubKey") + require.NoError(t, err) + }) + + t.Run("test-namespace-pubkey-update-using-new-key-with-previous-registered-key", func(t *testing.T) { + // The origin currently holds two private keys (privKey, privKey2), whose + // corresponding public keys are registered in the registry database. + tempDir := filepath.Join(t.TempDir(), "somewhere") + privKeyX, err := config.GeneratePEM(tempDir) + require.NoError(t, err) + allkeys := config.GetIssuerPrivateKeys() + require.Len(t, allkeys, 2) + + // Even though privKeyX is not in the origin's IssuerKeysDirectory, NamespacesPubKeyUpdate should succeed. + // This is because the origin holds (privKey, privKey2), satisfying the requirement of having at least one previously registered key + err = NamespacesPubKeyUpdate(privKeyX, []string{"/foo/bar"}, "mock_site_name", svr.URL+"/api/v1.0/registry/updateNamespacesPubKey") + require.NoError(t, err) + }) + + t.Run("test-namespace-pubkey-update-with-unregistered-key", func(t *testing.T) { + // Delete all private keys previously added to disk and memory + config.ResetIssuerPrivateKeys() + issuerKeysDir := param.IssuerKeysDirectory.GetString() + err = os.RemoveAll(issuerKeysDir) + + privKey3, err := config.GeneratePEM(issuerKeysDir) + require.NoError(t, err) + keysChange, err := config.RefreshKeys() + require.True(t, keysChange) + require.NoError(t, err) + // privKey3 is the active issuer key of the origin, but it cannot be used to update registry db because it is not previously registered + err = NamespacesPubKeyUpdate(privKey3, []string{"/foo/bar"}, "mock_site_name", svr.URL+"/api/v1.0/registry/updateNamespacesPubKey") + require.ErrorContains(t, err, "Origin does not possess valid key to update public keys of registered namespace(s)") + }) + } -func TestRegistryKeyChainingOSDF(t *testing.T) { +func TestMultiPubKeysRegisteredOnNamespace(t *testing.T) { ctx, cancel, egrp := test_utils.TestContext(context.Background(), t) - defer func() { require.NoError(t, egrp.Wait()) }() - defer cancel() + server_utils.ResetTestState() + t.Cleanup(func() { + if r := recover(); r != nil { + t.Errorf("Test panicked: %v", r) + } + + cancel() + assert.NoError(t, egrp.Wait()) + server_utils.ResetTestState() + }) + + tDir := t.TempDir() + + svr := registryMockup(ctx, t, "MultiPubKeysRegisteredOnNamespace") + defer func() { + err := ShutdownRegistryDB() + assert.NoError(t, err) + svr.CloseClientConnections() + svr.Close() + }() + + issuerKeysDir := param.IssuerKeysDirectory.GetString() + config.ResetIssuerPrivateKeys() + os.RemoveAll(issuerKeysDir) + privKeys := config.GetIssuerPrivateKeys() + require.Len(t, privKeys, 0) + + // Construct a client(origin) that has [p1,p2,p3] and p3 is the issuer key + privKey1, err := config.GeneratePEM(issuerKeysDir) + require.NoError(t, err) + privKey2, err := config.GeneratePEM(issuerKeysDir) + require.NoError(t, err) + keysChange, err := config.RefreshKeys() + require.True(t, keysChange) + require.NoError(t, err) + privKeys = config.GetIssuerPrivateKeys() + require.Len(t, privKeys, 2) + + prefix := "/mascot/bucky" + err = NamespaceRegister(privKey2, svr.URL+"/api/v1.0/registry", "", prefix, "mock_site_name") + require.NoError(t, err) + privKey3, err := config.GeneratePEM(issuerKeysDir) + require.NoError(t, err) + keysChange, err = config.RefreshKeys() + require.True(t, keysChange) + require.NoError(t, err) + + // Construct a public keys JWKS [p2,p4] to save in registry DB, imitating admin manually adding p4 + registryDbJwks := jwk.NewSet() + pubKey2, err := jwk.PublicKeyOf(privKey2) + require.NoError(t, err) + err = registryDbJwks.AddKey(pubKey2) + require.NoError(t, err) + privKey4, err := config.GeneratePEM(filepath.Join(tDir, "elsewhere")) + require.NoError(t, err) + pubKey4, err := jwk.PublicKeyOf(privKey4) + require.NoError(t, err) + err = registryDbJwks.AddKey(pubKey4) + require.NoError(t, err) + jwksBytes, err := json.Marshal(registryDbJwks) + require.NoError(t, err) + jwksStr := string(jwksBytes) + + // Test functionality of a namespace registered with multi public keys [p2,p4] + err = setNamespacePubKey(prefix, jwksStr) // set the registered public keys to [p2,p4] + require.NoError(t, err) + ns, err := getNamespaceByPrefix(prefix) + require.NoError(t, err) + require.Equal(t, jwksStr, ns.Pubkey) + + privKeys = config.GetIssuerPrivateKeys() + require.Len(t, privKeys, 3) + + // Client allValidKeys:[p1,p2,p3] (issuerKey:p3) ---UPDATE--> Registry [p2,p4] + // => should update Registry to [p1,p2,p3] (complete overwrite) + err = NamespacesPubKeyUpdate(privKey3, []string{prefix}, "mock_site_name", svr.URL+"/api/v1.0/registry/updateNamespacesPubKey") + require.NoError(t, err) + ns, err = getNamespaceByPrefix(prefix) + require.NoError(t, err) + + expectedJwks := jwk.NewSet() + + pubKey1, err := jwk.PublicKeyOf(privKey1) + require.NoError(t, err) + err = expectedJwks.AddKey(pubKey1) + require.NoError(t, err) + + err = expectedJwks.AddKey(pubKey2) + require.NoError(t, err) + + pubKey3, err := jwk.PublicKeyOf(privKey3) + require.NoError(t, err) + err = expectedJwks.AddKey(pubKey3) + require.NoError(t, err) + + expectedJwksBytes, err := json.Marshal(expectedJwks) + require.NoError(t, err) + expectedJwksStr := string(expectedJwksBytes) + + expectedKids, err := getSortedKids(ctx, expectedJwksStr) + require.NoError(t, err) + actualKids, err := getSortedKids(ctx, ns.Pubkey) + require.NoError(t, err) + require.Equal(t, expectedKids, actualKids) +} + +func TestRegistryKeyChainingOSDF(t *testing.T) { server_utils.ResetTestState() + + ctx, cancel, egrp := test_utils.TestContext(context.Background(), t) + t.Cleanup(func() { + if r := recover(); r != nil { + t.Errorf("Test panicked: %v", r) + } + + cancel() + assert.NoError(t, egrp.Wait()) + server_utils.ResetTestState() + }) + _, err := config.SetPreferredPrefix(config.OsdfPrefix) assert.NoError(t, err) - viper.Set("Federation.DirectorUrl", "https://osdf-director.osg-htc.org") - viper.Set("Federation.RegistryUrl", "https://osdf-registry.osg-htc.org") - viper.Set("Federation.JwkUrl", "https://osg-htc.org/osdf/public_signing_key.jwks") - viper.Set("Federation.BrokerUrl", "https://osdf-director.osg-htc.org") // On by default, but just to make things explicit viper.Set("Registry.RequireKeyChaining", true) @@ -176,10 +396,10 @@ func TestRegistryKeyChainingOSDF(t *testing.T) { topoSvr.Close() }() - _, err = config.GetIssuerPublicJWKS() - require.NoError(t, err) privKey, err := config.GetIssuerPrivateJWK() require.NoError(t, err) + _, err = config.GetIssuerPublicJWKS() + require.NoError(t, err) // Start by registering /foo/bar with the default key err = NamespaceRegister(privKey, registrySvr.URL+"/api/v1.0/registry", "", "/foo/bar", "") @@ -200,16 +420,18 @@ func TestRegistryKeyChainingOSDF(t *testing.T) { assert.Contains(t, err.Error(), "A superspace or subspace of this namespace /topo/foo already exists in the OSDF topology: /topo/foo. To register a Pelican equivalence, you need to present your identity.") // Now we create a new key and try to use it to register a super/sub space. These shouldn't succeed - viper.Set("IssuerKey", t.TempDir()+"/keychaining") - viper.Set("ConfigDir", t.TempDir()) + config.ResetIssuerPrivateKeys() + tDir2 := t.TempDir() + viper.Set("IssuerKeysDirectory", tDir2+"/keychaining2") + viper.Set("ConfigDir", tDir2) config.InitConfig() err = config.InitServer(ctx, server_structs.RegistryType) require.NoError(t, err) - _, err = config.GetIssuerPublicJWKS() - require.NoError(t, err) privKey, err = config.GetIssuerPrivateJWK() require.NoError(t, err) + _, err = config.GetIssuerPublicJWKS() + require.NoError(t, err) err = NamespaceRegister(privKey, registrySvr.URL+"/api/v1.0/registry", "", "/foo/bar/baz", "") require.ErrorContains(t, err, "Cannot register a namespace that is suffixed or prefixed") @@ -242,11 +464,19 @@ func TestRegistryKeyChainingOSDF(t *testing.T) { } func TestRegistryKeyChaining(t *testing.T) { + server_utils.ResetTestState() + ctx, cancel, egrp := test_utils.TestContext(context.Background(), t) - defer func() { require.NoError(t, egrp.Wait()) }() - defer cancel() + t.Cleanup(func() { + if r := recover(); r != nil { + t.Errorf("Test panicked: %v", r) + } + + cancel() + assert.NoError(t, egrp.Wait()) + server_utils.ResetTestState() + }) - server_utils.ResetTestState() // On by default, but just to make things explicit viper.Set("Registry.RequireKeyChaining", true) @@ -258,10 +488,10 @@ func TestRegistryKeyChaining(t *testing.T) { registrySvr.Close() }() - _, err := config.GetIssuerPublicJWKS() - require.NoError(t, err) privKey, err := config.GetIssuerPrivateJWK() require.NoError(t, err) + _, err = config.GetIssuerPublicJWKS() + require.NoError(t, err) // Start by registering /foo/bar with the default key err = NamespaceRegister(privKey, registrySvr.URL+"/api/v1.0/registry", "", "/foo/bar", "") @@ -272,16 +502,16 @@ func TestRegistryKeyChaining(t *testing.T) { require.NoError(t, err) // Now we create a new key and try to use it to register a super/sub space. These shouldn't succeed - viper.Set("IssuerKey", t.TempDir()+"/keychaining") + viper.Set("IssuerKeysDirectory", t.TempDir()+"/keychaining2") viper.Set("ConfigDir", t.TempDir()) config.InitConfig() err = config.InitServer(ctx, server_structs.RegistryType) require.NoError(t, err) - _, err = config.GetIssuerPublicJWKS() - require.NoError(t, err) privKey, err = config.GetIssuerPrivateJWK() require.NoError(t, err) + _, err = config.GetIssuerPublicJWKS() + require.NoError(t, err) err = NamespaceRegister(privKey, registrySvr.URL+"/api/v1.0/registry", "", "/foo/bar/baz", "") require.ErrorContains(t, err, "Cannot register a namespace that is suffixed or prefixed") @@ -296,6 +526,4 @@ func TestRegistryKeyChaining(t *testing.T) { err = NamespaceRegister(privKey, registrySvr.URL+"/api/v1.0/registry", "", "/foo", "") require.NoError(t, err) - - server_utils.ResetTestState() } diff --git a/registry/registry.go b/registry/registry.go index ec93602d2..9de46fc09 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -173,16 +173,26 @@ func loadServerKeys() (*ecdsa.PrivateKey, error) { // Note: go 1.21 introduces `OnceValues` which automates this procedure. // TODO: Reimplement the function once we switch to a minimum of 1.21 serverCredsLoad.Do(func() { - issuerFileName := param.IssuerKey.GetString() - var privateKey crypto.PrivateKey - privateKey, serverCredsErr = config.LoadPrivateKey(issuerFileName, false) - - switch key := privateKey.(type) { - case *ecdsa.PrivateKey: - serverCredsPrivKey = key - default: - serverCredsErr = errors.Errorf("unsupported key type for server issuer key: %T", key) + var privateKey jwk.Key + privateKey, serverCredsErr = config.GetIssuerPrivateJWK() + if serverCredsErr != nil { + return + } + + // Extract the underlying ECDSA private key + var rawKey interface{} + if err := privateKey.Raw(&rawKey); err != nil { + serverCredsErr = errors.Errorf("failed to extract raw key: %v", err) + return } + + ecdsaKey, ok := rawKey.(*ecdsa.PrivateKey) + if !ok { + serverCredsErr = errors.Errorf("unsupported key type for server issuer key: %T", rawKey) + return + } + + serverCredsPrivKey = ecdsaKey }) return serverCredsPrivKey, serverCredsErr @@ -1187,6 +1197,7 @@ func RegisterRegistryAPI(router *gin.RouterGroup) { registryAPI.GET("/*wildcard", wildcardHandler) registryAPI.POST("/checkNamespaceExists", checkNamespaceExistsHandler) registryAPI.POST("/checkNamespaceStatus", checkApprovalHandler) + registryAPI.POST("/updateNamespacesPubKey", updateNamespacesPubKey) registryAPI.DELETE("/*wildcard", deleteNamespaceHandler) } diff --git a/registry/registry_db.go b/registry/registry_db.go index a10d1e2ac..024f282cc 100644 --- a/registry/registry_db.go +++ b/registry/registry_db.go @@ -428,7 +428,6 @@ func AddNamespace(ns *server_structs.Namespace) error { if ns.AdminMetadata.Status == "" { ns.AdminMetadata.Status = server_structs.RegPending } - return db.Save(&ns).Error } @@ -485,6 +484,17 @@ func updateNamespaceStatusById(id int, status server_structs.RegistrationStatus, return db.Model(ns).Where("id = ?", id).Update("admin_metadata", string(adminMetadataByte)).Error } +func setNamespacePubKey(prefix string, pubkeyDbString string) error { + if prefix == "" { + return errors.New("invalid prefix. Prefix must not be empty") + } + if pubkeyDbString == "" { + return errors.New("invalid pubkeyDbString. pubkeyDbString must not be empty") + } + ns := server_structs.Namespace{} + return db.Model(ns).Where("prefix = ? ", prefix).Update("pubkey", pubkeyDbString).Error +} + func deleteNamespaceByID(id int) error { return db.Delete(&server_structs.Namespace{}, id).Error } diff --git a/registry/registry_pubkey_update.go b/registry/registry_pubkey_update.go new file mode 100644 index 000000000..67f1f5181 --- /dev/null +++ b/registry/registry_pubkey_update.go @@ -0,0 +1,270 @@ +/*************************************************************** + * + * Copyright (C) 2024, Pelican Project, Morgridge Institute for Research + * + * Licensed under the Apache License, Version 2.0 (the "License"); you + * may not use this file except in compliance with the License. You may + * obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ***************************************************************/ + +package registry + +import ( + "crypto/ecdsa" + "encoding/hex" + "encoding/json" + "fmt" + "net/http" + + "github.com/gin-gonic/gin" + "github.com/lestrrat-go/jwx/v2/jwk" + "github.com/pkg/errors" + log "github.com/sirupsen/logrus" + + "github.com/pelicanplatform/pelican/server_structs" +) + +type RegisteredPrefixUpdate struct { + ClientNonce string `json:"client_nonce"` + ClientPayload string `json:"client_payload"` + ClientSignature string `json:"client_signature"` + KeyUpdateAuthzSignature string `json:"key_update_authz_signature"` + MatchedKeyId string `json:"matched_key_id"` + + ServerNonce string `json:"server_nonce"` + ServerPayload string `json:"server_payload"` + ServerSignature string `json:"server_signature"` + + Pubkey json.RawMessage `json:"pubkey"` + AllPubkeys json.RawMessage `json:"all_pubkeys"` + Prefixes []string `json:"prefixes"` +} + +// Generate server nonce for key-sign challenge when updating the public key of registered namespace(s) +func updateNsKeySignChallengeInit(data *RegisteredPrefixUpdate) (map[string]interface{}, error) { + serverNonce, err := generateNonce() + if err != nil { + return nil, errors.Wrap(err, "Failed to generate nonce for key-sign challenge") + } + + serverPayload := []byte(data.ClientNonce + data.ServerNonce) + + privateKey, err := loadServerKeys() + if err != nil { + return nil, errors.Wrap(err, "Server is unable to generate a key sign challenge: Failed to load the server's private key") + } + + serverSignature, err := signPayload(serverPayload, privateKey) + if err != nil { + return nil, errors.Wrap(err, "Failed to sign payload for key-sign challenge") + } + + registeredKeysOnNs := make(map[string]string) + for _, prefix := range data.Prefixes { + existingNs, err := getNamespaceByPrefix(prefix) + if err != nil { + log.Errorf("Namespace %s not exists: %v", prefix, err) + return nil, errors.Wrapf(err, "Server encountered an error retrieving namespace %s", prefix) + } + registeredKeysOnNs[prefix] = existingNs.Pubkey + } + res := map[string]interface{}{ + "server_nonce": serverNonce, + "client_nonce": data.ClientNonce, + "server_payload": hex.EncodeToString(serverPayload), + "server_signature": hex.EncodeToString(serverSignature), + "registered_keys": registeredKeysOnNs, + } + return res, nil +} + +// 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) { + // Validate the client's jwks as a set here + key, err := validateJwks(string(data.Pubkey)) + if err != nil { + return nil, badRequestError{Message: err.Error()} + } + var rawkey interface{} // This is the raw key, like *ecdsa.PrivateKey + if err := key.Raw(&rawkey); err != nil { + return nil, errors.Wrap(err, "failed to generate raw pubkey from jwks") + } + + // Verify the Proof of Possession of the client and server's active private keys + clientPayload := []byte(data.ClientNonce + data.ServerNonce) + clientSignature, err := hex.DecodeString(data.ClientSignature) + if err != nil { + return nil, errors.Wrap(err, "Failed to decode the client's signature") + } + clientVerified := verifySignature(clientPayload, clientSignature, (rawkey).(*ecdsa.PublicKey)) + serverPayload, err := hex.DecodeString(data.ServerPayload) + if err != nil { + return nil, errors.Wrap(err, "Failed to decode the server's payload") + } + + serverSignature, err := hex.DecodeString(data.ServerSignature) + if err != nil { + return nil, errors.Wrap(err, "Failed to decode the server's signature") + } + + serverPrivateKey, err := loadServerKeys() + if err != nil { + return nil, errors.Wrap(err, "Failed to decode the server's private key") + } + serverPubkey := serverPrivateKey.PublicKey + serverVerified := verifySignature(serverPayload, serverSignature, &serverPubkey) + + // Overwrite the namespace's public key(s) with the latest keys in the origin + if clientVerified && serverVerified { + for _, prefix := range data.Prefixes { + log.Debug("Start updating namespace: ", prefix) + + // We ensure the prefix exists in registry db via updateNsKeySignChallengeInit function + + existingNs, err := getNamespaceByPrefix(prefix) + if err != nil { + log.Errorf("Failed to get existing namespace to update: %v", err) + return nil, errors.Wrap(err, "Server encountered an error getting existing namespace to update") + } + + // Verify the origin is authorized to update + registryDbKeySet := jwk.NewSet() + err = json.Unmarshal([]byte(existingNs.Pubkey), ®istryDbKeySet) + if err != nil { + log.Errorf("Failed to parse public key as JWKS of registered namespace %s: %v", prefix, err) + return nil, errors.Wrapf(err, "Invalid public key format of registered namespace %s", prefix) + } + + // Get client's signature from payload + keyUpdateAuthzSignature, err := hex.DecodeString(data.KeyUpdateAuthzSignature) + if err != nil { + return nil, errors.Wrap(err, "Failed to decode the client's key update authorization signature") + } + + // Look for the matched key sent back from the client in `registryDbKeySet` + matchedKey, found := registryDbKeySet.LookupKeyID(data.MatchedKeyId) + if !found { + return nil, permissionDeniedError{ + Message: fmt.Sprintf("The client that tries to update prefix '%s' cannot be authorized because it doesn't contain any public key matching the existing namespace's public key in db", prefix), + } + } + + rawkey := &ecdsa.PublicKey{} + if err := matchedKey.Raw(rawkey); err != nil { + return nil, errors.Wrap(err, "failed to generate the raw key from the matched key") + } + + keyUpdateAuthzVerified := verifySignature(clientPayload, keyUpdateAuthzSignature, rawkey) + if !keyUpdateAuthzVerified { + return nil, permissionDeniedError{ + Message: fmt.Sprintf("The client that tries to update prefix '%s' cannot be authorized because it fails to pass the proof of possession verification", prefix), + } + } + + // 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) + } + clientPubkeyString := string(allPubkeyData) + + // Perform the update action when the latest keys in the origin are different from the registered ones + if clientPubkeyString != existingNs.Pubkey { + 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 { + log.Errorf("Failed to update the public key of namespace %s: %v", prefix, err) + return nil, errors.Wrap(err, "Server encountered an error updating the public key of an existing namespace") + } + returnMsg := map[string]interface{}{ + "message": fmt.Sprintf("Updated the public key of namespace %s:", prefix), + } + 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 + } + + } + } + + return nil, errors.Errorf("Unable to verify the client's public key, or an encountered an error with its own: "+ + "server verified:%t, client verified:%t", serverVerified, clientVerified) + +} + +// Handle the registered namespace public key update with nonce generation and verifcation +func updateNsKeySignChallenge(data *RegisteredPrefixUpdate) (map[string]interface{}, error) { + if data.ClientNonce != "" && data.ClientPayload != "" && data.ClientSignature != "" && + data.ServerNonce != "" && data.ServerPayload != "" && data.ServerSignature != "" { + res, err := updateNsKeySignChallengeCommit(data) + if err != nil { + return nil, err + } else { + return res, nil + } + } else if data.ClientNonce != "" { + res, err := updateNsKeySignChallengeInit(data) + if err != nil { + return nil, err + } else { + return res, nil + } + } else { + return nil, badRequestError{Message: "Key sign challenge is missing parameters"} + } +} + +func updateNamespacesPubKey(ctx *gin.Context) { + + var reqData RegisteredPrefixUpdate + if err := ctx.BindJSON(&reqData); err != nil { + log.Errorln("Bad request: ", err) + ctx.JSON(http.StatusBadRequest, server_structs.SimpleApiResp{ + Status: server_structs.RespFailed, + Msg: fmt.Sprint("Bad Request: ", err.Error())}) + return + } + + res, err := updateNsKeySignChallenge(&reqData) + if err != nil { + if errors.As(err, &permissionDeniedError{}) { + ctx.JSON(http.StatusForbidden, + server_structs.SimpleApiResp{ + Status: server_structs.RespFailed, + Msg: fmt.Sprintf("You don't have permission to update the registered public key of the prefix: %v", err), + }) + return + } else if errors.As(err, &badRequestError{}) { + ctx.JSON(http.StatusBadRequest, server_structs.SimpleApiResp{ + Status: server_structs.RespFailed, + Msg: fmt.Sprintf("Bad request for key-sign challenge: %v", err), + }) + return + } else { + ctx.JSON(http.StatusInternalServerError, server_structs.SimpleApiResp{ + Status: server_structs.RespFailed, + Msg: fmt.Sprintf("Server encountered an error during key-sign challenge: %v", err), + }) + log.Warningf("Failed to complete key sign challenge without identity requirement: %v", err) + return + } + } else { + ctx.JSON(http.StatusOK, res) + return + } +} diff --git a/token/token_create_test.go b/token/token_create_test.go index eb55c61e3..9d2249125 100644 --- a/token/token_create_test.go +++ b/token/token_create_test.go @@ -34,6 +34,7 @@ import ( "github.com/stretchr/testify/require" "github.com/pelicanplatform/pelican/config" + "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/server_structs" "github.com/pelicanplatform/pelican/test_utils" "github.com/pelicanplatform/pelican/token_scopes" @@ -208,8 +209,8 @@ func TestCreateToken(t *testing.T) { config.ResetConfig() viper.Set("IssuerUrl", "https://my-issuer.com") tDir := t.TempDir() - kfile := filepath.Join(tDir, "testKey") - viper.Set("IssuerKey", kfile) + kDir := filepath.Join(tDir, "testKeyDir") + viper.Set(param.IssuerKeysDirectory.GetName(), kDir) viper.Set("ConfigDir", t.TempDir()) config.InitConfig() err := config.InitServer(ctx, server_structs.DirectorType) diff --git a/web_ui/prometheus_test.go b/web_ui/prometheus_test.go index 22197aade..5e159a705 100644 --- a/web_ui/prometheus_test.go +++ b/web_ui/prometheus_test.go @@ -63,9 +63,9 @@ func TestPrometheusUnprotected(t *testing.T) { // Create temp dir for the origin key file tDir := t.TempDir() - kfile := filepath.Join(tDir, "testKey") + kDir := filepath.Join(tDir, "testKeyDir") //Setup a private key - viper.Set("IssuerKey", kfile) + viper.Set(param.IssuerKeysDirectory.GetName(), kDir) viper.Set("ConfigDir", t.TempDir()) config.InitConfig() err := config.InitServer(ctx, server_structs.OriginType) @@ -109,9 +109,9 @@ func TestPrometheusProtectionCookieAuth(t *testing.T) { // Create temp dir for the origin key file tDir := t.TempDir() - kfile := filepath.Join(tDir, "testKey") - //Setup a private key - viper.Set("IssuerKey", kfile) + kDir := filepath.Join(tDir, "testKeyDir") + // Setup a private key directory + viper.Set(param.IssuerKeysDirectory.GetName(), kDir) viper.Set("ConfigDir", t.TempDir()) config.InitConfig() err := config.InitServer(ctx, server_structs.OriginType) @@ -170,10 +170,10 @@ func TestPrometheusProtectionOriginHeaderScope(t *testing.T) { // Create temp dir for the origin key file tDir := t.TempDir() - kfile := filepath.Join(tDir, "testKey") + keysDir := filepath.Join(tDir, "testKeys") //Setup a private key and a token - viper.Set("IssuerKey", kfile) + viper.Set("IssuerKeysDirectory", keysDir) // Setting the ConfigDir to t.TempDir() causes issues with this test on Windows because // the process tries to clean up the directory before the test is done with it. @@ -237,15 +237,16 @@ func TestPrometheusProtectionOriginHeaderScope(t *testing.T) { } // Create a new private key by re-initializing config to point at a new temp dir - k2file := filepath.Join(tDir, "testKey2") - viper.Set("IssuerKey", k2file) + k2dir := filepath.Join(tDir, "whatever", "testDir2") + viper.Set("IssuerKeysDirectory", k2dir) err = config.InitServer(ctx, server_structs.OriginType) require.NoError(t, err) token := createToken("monitoring.query", issuerUrl) // Re-init the config again, this time pointing at the original key - viper.Set("IssuerKey", kfile) + config.ResetIssuerPrivateKeys() + viper.Set("IssuerKeysDirectory", keysDir) err = config.InitServer(ctx, server_structs.OriginType) require.NoError(t, err) diff --git a/web_ui/ui_test.go b/web_ui/ui_test.go index 131f91462..50fdbd041 100644 --- a/web_ui/ui_test.go +++ b/web_ui/ui_test.go @@ -97,7 +97,7 @@ func TestMain(m *testing.M) { } //Override viper default for testing - viper.Set("IssuerKey", filepath.Join(tempJWKDir, "issuer.jwk")) + viper.Set(param.IssuerKeysDirectory.GetName(), filepath.Join(tempJWKDir, "issuer-keys")) // Ensure we load up the default configs. dirname, err := os.MkdirTemp("", "tmpDir") @@ -219,8 +219,8 @@ func TestHandleWebUIAuth(t *testing.T) { }) tmpDir := t.TempDir() - issuerFile := filepath.Join(tmpDir, "issuer.key") - viper.Set(param.IssuerKey.GetName(), issuerFile) + issuerDirectory := filepath.Join(tmpDir, "issuer-keys") + viper.Set(param.IssuerKeysDirectory.GetName(), issuerDirectory) viper.Set(param.Server_ExternalWebUrl.GetName(), "https://example.com") _, err := config.GetIssuerPrivateJWK() From 9c09a3d44bc50e509f25a7c4ccd78c61070175e8 Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Wed, 22 Jan 2025 14:41:27 -0600 Subject: [PATCH 2/8] Accept 200 or 201 on uploads The next version of XRootD will respond with a 201 (Created) on successful upload (which is the correct status code). Unfortunately, `pelican` had a hard check for 200 (OK) instead. Accept either. Additionally, use the symbolic name for these so we can better `grep` through the source code to find use of particular status codes. --- client/handle_http.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/client/handle_http.go b/client/handle_http.go index b233ff747..c0079647a 100644 --- a/client/handle_http.go +++ b/client/handle_http.go @@ -2035,7 +2035,7 @@ Loop: trailer := resp.HTTPResponse.Trailer if errorStatus := trailer.Get("X-Transfer-Status"); errorStatus != "" { statusCode, statusText := parseTransferStatus(errorStatus) - if statusCode != 200 { + if statusCode != http.StatusOK { log.WithFields(fields).Debugln("Got error from file transfer") err = errors.New("transfer error: " + statusText) return @@ -2044,7 +2044,7 @@ Loop: } // Valid responses include 200 and 206. The latter occurs if the download was resumed after a // prior attempt. - if resp.HTTPResponse.StatusCode != 200 && resp.HTTPResponse.StatusCode != 206 { + if resp.HTTPResponse.StatusCode != http.StatusOK && resp.HTTPResponse.StatusCode != http.StatusPartialContent { log.WithFields(fields).Debugln("Got failure status code:", resp.HTTPResponse.StatusCode) return 0, 0, -1, serverVersion, &HttpErrResp{resp.HTTPResponse.StatusCode, fmt.Sprintf("Request failed (HTTP status %d): %s", resp.HTTPResponse.StatusCode, resp.Err().Error())} @@ -2271,7 +2271,9 @@ Loop: log.Debugln("File closed") case response := <-responseChan: attempt.ServerVersion = response.Header.Get("Server") - if response.StatusCode != 200 { + // Note: Accept both 200 and 201 as success codes; the latter is the correct one, but + // older versions of XRootD incorrectly use 200. + if response.StatusCode != http.StatusOK && response.StatusCode != http.StatusCreated { log.Errorln("Got failure status code:", response.StatusCode) lastError = &HttpErrResp{response.StatusCode, fmt.Sprintf("Request failed (HTTP status %d)", response.StatusCode)} @@ -2337,7 +2339,10 @@ func runPut(request *http.Request, responseChan chan<- *http.Response, errorChan } dump, _ = httputil.DumpResponse(response, true) log.Debugf("Dumping response: %s", dump) - if response.StatusCode != 200 { + // Note: XRootD used to always return 200 (OK) on upload, even when it was supposed to turn + // HTTP 201 (Created). Check for both here; in the future we may want to remove the 200 check + // if we decide to drop support for the older versions of XRootD. + if response.StatusCode != http.StatusOK && response.StatusCode != http.StatusCreated { log.Errorln("Error status code:", response.Status) log.Debugln("From the server:") textResponse, err := io.ReadAll(response.Body) From f5e2d3198fdc0305b66be8743c4173c2d35e82b5 Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Wed, 22 Jan 2025 14:46:17 -0600 Subject: [PATCH 3/8] Restore the behavior of the keygen tool In the branch for multi-key support, the keygen tool was changed to create keys in the key directory instead of the working directory. While that might have been a better original design, I don't want to switch the semantic (and particularly the documentation!) at this point. Restoring the original behavior. --- cmd/generate_keygen.go | 40 +++++++++++++++++++++++++++++-------- cmd/generate_keygen_test.go | 9 +++++++-- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/cmd/generate_keygen.go b/cmd/generate_keygen.go index f2c4832b5..a26816fe2 100644 --- a/cmd/generate_keygen.go +++ b/cmd/generate_keygen.go @@ -19,27 +19,47 @@ package main import ( + "crypto" + "crypto/elliptic" "encoding/json" "fmt" "os" "path/filepath" "strings" + "github.com/lestrrat-go/jwx/v2/jwk" "github.com/pkg/errors" "github.com/spf13/cobra" - "github.com/spf13/viper" "github.com/pelicanplatform/pelican/config" - "github.com/pelicanplatform/pelican/param" ) +func createJWKS(privKey crypto.PrivateKey) (jwk.Set, error) { + key, err := jwk.FromRaw(privKey) + if err != nil { + return nil, errors.Wrap(err, "failed to generate JWK from private key") + } + jwks := jwk.NewSet() + + pkey, err := jwk.PublicKeyOf(key) + if err != nil { + return nil, errors.Wrapf(err, "failed to generate public key from key %s", key.KeyID()) + } + + if err = jwks.AddKey(pkey); err != nil { + return nil, errors.Wrapf(err, "Failed to add public key %s to new JWKS", key.KeyID()) + } + + return jwks, nil +} + func keygenMain(cmd *cobra.Command, args []string) error { wd, err := os.Getwd() if err != nil { return errors.Wrap(err, "failed to get the current working directory") } if privateKeyPath == "" { - privateKeyPath = filepath.Join(wd, "issuer-keys") + privateKeyPath = filepath.Join(wd, "issuer.jwk") } else { privateKeyPath = filepath.Clean(strings.TrimSpace(privateKeyPath)) } @@ -68,15 +88,19 @@ func keygenMain(cmd *cobra.Command, args []string) error { return fmt.Errorf("file exists for public key under %s", publicKeyPath) } - viper.Set(param.IssuerKeysDirectory.GetName(), privateKeyPath) + if err := config.GeneratePrivateKey(privateKeyPath, elliptic.P256(), false); err != nil { + return errors.Wrapf(err, "failed to generate new private key at %s", privateKeyPath) + } + privKey, err := config.LoadPrivateKey(privateKeyPath, false) + if err != nil { + return errors.Wrapf(err, "failed to load private key from %s", privateKeyPath) + } - // GetIssuerPublicJWKS will generate the private key in IssuerKeysDirectory if it does not exist - // and parse the private key and generate the corresponding public key for us - pubkey, err := config.GetIssuerPublicJWKS() + pubJWKS, err := createJWKS(privKey) if err != nil { return err } - bytes, err := json.MarshalIndent(pubkey, "", " ") + bytes, err := json.MarshalIndent(pubJWKS, "", " ") if err != nil { return errors.Wrap(err, "failed to generate json from jwks") } diff --git a/cmd/generate_keygen_test.go b/cmd/generate_keygen_test.go index 436a84446..c839875e0 100644 --- a/cmd/generate_keygen_test.go +++ b/cmd/generate_keygen_test.go @@ -49,8 +49,8 @@ func setupTestRun(t *testing.T) string { return tmpDir } -func checkKeys(t *testing.T, publicKey string) { - _, err := config.GetIssuerPrivateJWK() +func checkKeys(t *testing.T, privateKey, publicKey string) { + _, err := config.LoadPrivateKey(privateKey, false) require.NoError(t, err) jwks, err := jwk.ReadFile(publicKey) @@ -79,6 +79,7 @@ func TestKeygenMain(t *testing.T) { checkKeys( t, + filepath.Join(tempDir, "issuer.jwk"), filepath.Join(tempDir, "issuer-pub.jwks"), ) }) @@ -94,6 +95,7 @@ func TestKeygenMain(t *testing.T) { checkKeys( t, + privateKeyPath, filepath.Join(tempWd, "issuer-pub.jwks"), ) }) @@ -109,6 +111,7 @@ func TestKeygenMain(t *testing.T) { checkKeys( t, + filepath.Join(tempWd, "issuer.jwk"), publicKeyPath, ) }) @@ -125,6 +128,7 @@ func TestKeygenMain(t *testing.T) { checkKeys( t, + privateKeyPath, filepath.Join(tempWd, "issuer-pub.jwks"), ) }) @@ -141,6 +145,7 @@ func TestKeygenMain(t *testing.T) { checkKeys( t, + filepath.Join(tempWd, "issuer.jwk"), publicKeyPath, ) }) From d89ae9004ecb46b5ac9060e053f5d5c7f6ccbb6a Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Wed, 22 Jan 2025 14:49:56 -0600 Subject: [PATCH 4/8] Fix parameter documentation to match actual behavior --- docs/parameters.yaml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/parameters.yaml b/docs/parameters.yaml index ddf91e144..3380350e9 100644 --- a/docs/parameters.yaml +++ b/docs/parameters.yaml @@ -64,12 +64,16 @@ components: ["cache", "director", "origin", "registry"] --- name: IssuerKey description: |+ - [Deprecated] Use IssuerKeysDirectory instead. The key file set by this parameter will automatically move to IssuerKeysDirectory. + [Deprecated] Use IssuerKeysDirectory instead. - A filepath to the file containing a PEM-encoded ecdsa private key which later will be parsed - into a JWK and serves as the private key to sign various JWTs issued by this server. + A filepath to the file containing a PEM-encoded ECDSA private key which will be used + to sign credentials issued by this server. - A public JWK will be derived from this private key and used as the key for token verification. + A public key will be derived from this private key and used as the key for token verification + by external services. + + The use of `IssuerKeysDirectory` is preferred as it allows administrators to have more than + one signing key. type: filename deprecated: true replacedby: none From 74ff903f2da60d042bb664046429466bdc93ed2d Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Wed, 22 Jan 2025 14:50:28 -0600 Subject: [PATCH 5/8] Use common utility function instead of hand-writing update loop --- ...key_refresh_and_update_namespace_pubkey.go | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/launcher_utils/key_refresh_and_update_namespace_pubkey.go b/launcher_utils/key_refresh_and_update_namespace_pubkey.go index 7679ca047..97895136e 100644 --- a/launcher_utils/key_refresh_and_update_namespace_pubkey.go +++ b/launcher_utils/key_refresh_and_update_namespace_pubkey.go @@ -1,6 +1,6 @@ /*************************************************************** * - * Copyright (C) 2024, Pelican Project, Morgridge Institute for Research + * Copyright (C) 2025, Pelican Project, Morgridge Institute for Research * * Licensed under the Apache License, Version 2.0 (the "License"); you * may not use this file except in compliance with the License. You may @@ -113,29 +113,25 @@ func triggerNamespacesPubKeyUpdate(ctx context.Context) error { // Check the directory containing .pem files regularly, load new private key(s) // For origin server, if new file(s) are detected, then register the new public key func LaunchIssuerKeysDirRefresh(ctx context.Context, egrp *errgroup.Group, modules server_structs.ServerType) { - egrp.Go(func() error { - ticker := time.NewTicker(5 * time.Minute) - defer ticker.Stop() - - for { - select { - case <-ctx.Done(): - log.Debugln("Stopping periodic check for private keys directory.") + server_utils.LaunchWatcherMaintenance( + ctx, + []string{param.IssuerKeysDirectory.GetString()}, + "private key refresh and registration", + time.Minute, + func( /*notifyEvent*/ bool) error { + // Refresh the disk to pick up any new private key + keysChanged, err := config.RefreshKeys() + if err != nil { return nil - case <-ticker.C: - // Refresh the disk to pick up any new private key - keysChanged, err := config.RefreshKeys() - if err != nil { - return nil - } + } - // Update public key registered with namespace in registry db when the private key(s) changed in an origin - if modules.IsEnabled(server_structs.OriginType) && keysChanged { - if err = triggerNamespacesPubKeyUpdate(ctx); err != nil { - return err - } + // Update public key registered with namespace in registry db when the private key(s) changed in an origin + if modules.IsEnabled(server_structs.OriginType) && keysChanged { + if err = triggerNamespacesPubKeyUpdate(ctx); err != nil { + return err } } - } - }) + return nil + }, + ) } From f803745e2dae358c85692e39536b69f09c555132 Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Wed, 22 Jan 2025 14:53:26 -0600 Subject: [PATCH 6/8] Use oldest, not newest, key on update There remain too many open questions for when we can safely use the newest private key for signing operations. Until then, be conservative and use the oldest one. Update the unit tests along with the new behavior. --- config/init_server_creds.go | 164 +++++++++++++--------- launcher_utils/register_namespace_test.go | 14 +- registry/client_commands_test.go | 3 +- 3 files changed, 113 insertions(+), 68 deletions(-) diff --git a/config/init_server_creds.go b/config/init_server_creds.go index be72c0a9e..e2b30259e 100644 --- a/config/init_server_creds.go +++ b/config/init_server_creds.go @@ -111,7 +111,7 @@ func GetIssuerPrivateKeys() map[string]jwk.Key { func createDirForKeys(dir string) error { gid, err := GetDaemonGID() if err != nil { - return errors.Wrap(err, "failed to get deamon gid") + return errors.Wrap(err, "failed to get daemon gid") } if err := MkdirAll(dir, 0750, -1, gid); err != nil { return errors.Wrapf(err, "failed to set the permission of %s", dir) @@ -231,10 +231,7 @@ func GeneratePrivateKey(keyLocation string, curve elliptic.Curve, allowRSA bool) return errors.Wrap(err, "Failed to create new private key file at "+keyLocation) } defer file.Close() - priv, err := ecdsa.GenerateKey(curve, rand.Reader) - if err != nil { - return err - } + // Windows does not have "chown", has to work differently currentOS := runtime.GOOS if currentOS == "windows" { @@ -251,6 +248,16 @@ func GeneratePrivateKey(keyLocation string, curve elliptic.Curve, allowRSA bool) } } + return generatePrivateKeyToFile(file, curve) +} + +// Write a PEM-encoded private key to an open file +func generatePrivateKeyToFile(file *os.File, curve elliptic.Curve) error { + priv, err := ecdsa.GenerateKey(curve, rand.Reader) + if err != nil { + return err + } + bytes, err := x509.MarshalPKCS8PrivateKey(priv) if err != nil { return err @@ -603,8 +610,8 @@ func loadSinglePEM(path string) (jwk.Key, error) { // Helper function to load/refresh all key files from both legacy IssuerKey file and specified directory // find the most recent private key based on lexicographical order of their filenames func loadPEMFiles(dir string) (jwk.Key, error) { - var mostRecentKey jwk.Key - var mostRecentFileName string + var firstKey jwk.Key + var firstFileName string latestKeys := getIssuerPrivateKeysCopy() // Load legacy private key if it exists - parsing the file at IssuerKey act as if it is included in IssuerKeysDirectory @@ -616,52 +623,60 @@ func loadPEMFiles(dir string) (jwk.Key, error) { log.Warnf("Failed to load key %s: %v", issuerKeyPath, err) } else { latestKeys[issuerKey.KeyID()] = issuerKey - if mostRecentFileName == "" || filepath.Base(issuerKeyPath) > mostRecentFileName { - mostRecentFileName = filepath.Base(issuerKeyPath) - mostRecentKey = issuerKey + if firstFileName == "" || filepath.Base(issuerKeyPath) < firstFileName { + firstFileName = filepath.Base(issuerKeyPath) + firstKey = issuerKey } } } } - // Ensure input directory dir exists, if not, create it with proper permissions - if _, err := os.Stat(dir); os.IsNotExist(err) { - if err := createDirForKeys(dir); err != nil { - return nil, errors.Wrapf(err, "failed to create directory and set permissions: %s", dir) - } + if dir == "" && issuerKeyPath == "" { + return nil, errors.New("no private key file or directory specified") } - // Traverse the directory for .pem files in lexical order - err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { - if err != nil { - return err - } - if !d.IsDir() && filepath.Ext(d.Name()) == ".pem" { - // Parse the private key in this file and add to the in-memory keys map - key, err := loadSinglePEM(path) + if dir != "" { + // Ensure input directory dir exists, if not, create it with proper permissions + if _, err := os.Stat(dir); os.IsNotExist(err) { + if err := createDirForKeys(dir); err != nil { + return nil, errors.Wrapf(err, "failed to create directory and set permissions: %s", dir) + } + } + // Traverse the directory for .pem files in lexical order + err := filepath.WalkDir(dir, func(path string, dirEnt fs.DirEntry, err error) error { if err != nil { - log.Warnf("Failed to load key %s: %v", path, err) - return nil // Skip this file and continue + return err } + // Do not recurse into directories + if (path != dir) && dirEnt.IsDir() { + return filepath.SkipDir + } + if dirEnt.Type().IsRegular() && filepath.Ext(dirEnt.Name()) == ".pem" { + // Parse the private key in this file and add to the in-memory keys map + key, err := loadSinglePEM(path) + if err != nil { + log.Warnf("Failed to load key %s: %v", path, err) + return nil // Skip this file and continue + } - latestKeys[key.KeyID()] = key + latestKeys[key.KeyID()] = key - // Update the most recent key based on lexicographical order of filenames - if mostRecentFileName == "" || d.Name() > mostRecentFileName { - mostRecentFileName = d.Name() - mostRecentKey = key + // Update the most recent key based on lexicographical order of filenames + if firstFileName == "" || dirEnt.Name() < firstFileName { + firstFileName = dirEnt.Name() + firstKey = key + } } + return nil + }) + if err != nil { + return nil, errors.Wrapf(err, "failed to traverse directory %s that stores private keys", dir) } - return nil - }) - - if err != nil { - return nil, errors.Wrapf(err, "failed to traverse directory %s that stores private keys", dir) } // Create a new private key and set as issuer key when neither legacy private key at IssuerKey // nor any .pem file at IssuerKeysDirectory exists - if len(latestKeys) == 0 || mostRecentKey == nil { + if len(latestKeys) == 0 || firstKey == nil { newKey, err := generatePEMandSetIssuerKey(dir) if err != nil { return nil, errors.Wrapf(err, "failed to create a new .pem file to save private key") @@ -671,48 +686,63 @@ func loadPEMFiles(dir string) (jwk.Key, error) { // Save current key and all up-to-date valid private keys and the in-memory issuerKeys newKeys := IssuerKeys{ - CurrentKey: mostRecentKey, + CurrentKey: firstKey, AllKeys: latestKeys, } issuerKeys.Store(&newKeys) - log.Debugf("Set private key %s as the issuer key", mostRecentKey.KeyID()) + log.Debugf("Set private key %s as the issuer key", firstKey.KeyID()) - return mostRecentKey, nil + return firstKey, nil } // Create a new .pem file (combining GeneratePrivateKey and LoadPrivateKey functions) -func GeneratePEM(dir string) (jwk.Key, error) { - // Generate a unique filename using a POSIX mkstemp-like logic - // Create a temp file, store its filename, then immediately delete this temp file - filenamePattern := fmt.Sprintf("pelican_generated_%d_*.pem", - time.Now().UnixNano()) - if err := createDirForKeys(dir); err != nil { - return nil, errors.Wrapf(err, "failed to create directory and set permissions: %s", dir) - } - tempFile, err := os.CreateTemp(dir, filenamePattern) - if err != nil { - return nil, errors.Wrap(err, "failed to remove temp file") - } - keyPath := tempFile.Name() - if err := tempFile.Close(); err != nil { - return nil, errors.Wrap(err, "failed to close temp file") - } - if err := os.Remove(keyPath); err != nil { - return nil, errors.Wrap(err, "failed to remove temp file") +func GeneratePEM(dir string) (key jwk.Key, err error) { + var fname string + var keyFile *os.File + if dir == "" { + issuerKeyLocation := param.IssuerKey.GetString() + if issuerKeyLocation == "" { + err = errors.New("no private key file or directory specified") + return + } + log.Debugln("Generating new private key in the legacy IssuerKey file", issuerKeyLocation) + fname = issuerKeyLocation + keyFile, err = os.OpenFile(issuerKeyLocation, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0400) + if err != nil { + err = errors.Wrap(err, "failed to open issuer key file") + return + } + } else { + // Generate a unique filename using a POSIX mkstemp-like logic + // Create a temp file, store its filename, then immediately delete this temp file + filenamePattern := fmt.Sprintf("pelican_generated_%d_*.pem", + time.Now().UnixNano()) + if err = createDirForKeys(dir); err != nil { + err = errors.Wrapf(err, "failed to create directory and set permissions: %s", dir) + return + } + keyFile, err = os.CreateTemp(dir, filenamePattern) + if err != nil { + err = errors.Wrap(err, "failed to remove temp file") + return + } + fname = keyFile.Name() + log.Debugln("Generating new private key in the IssuerKeys directory at", fname) } + defer keyFile.Close() - if err := GeneratePrivateKey(keyPath, elliptic.P256(), false); err != nil { - return nil, errors.Wrapf(err, "failed to generate new private key at %s", keyPath) + if err = generatePrivateKeyToFile(keyFile, elliptic.P256()); err != nil { + return nil, errors.Wrapf(err, "failed to generate private key in file %s", fname) } - key, err := loadSinglePEM(keyPath) - if err != nil { - log.Errorf("Failed to load key %s: %v", keyPath, err) - return nil, errors.Wrapf(err, "failed to load key from %s", keyPath) + if key, err = loadSinglePEM(fname); err != nil { + log.Errorf("Failed to load key %s: %v", fname, err) + err = errors.Wrapf(err, "failed to load key from %s", fname) + return } - log.Debugf("Generated private key %s", key.KeyID()) - return key, nil + log.Debugf("Generated private key with key ID %s", key.KeyID()) + return } // Generate a new .pem file and then set the private key it contains as the issuer key @@ -920,6 +950,8 @@ func LoadSessionSecret() ([]byte, error) { return rest, nil } +// Check to see if two given maps of jwk.Keys are logically +// equivalent func areKeysDifferent(a, b map[string]jwk.Key) bool { if len(a) != len(b) { return true @@ -940,6 +972,8 @@ func areKeysDifferent(a, b map[string]jwk.Key) bool { return false // All keys are the same } +// Refresh the private keys directory and return `true` if the keys have changed +// since the last refresh func RefreshKeys() (bool, error) { before := GetIssuerPrivateKeys() _, err := loadIssuerPrivateKey(param.IssuerKeysDirectory.GetString()) diff --git a/launcher_utils/register_namespace_test.go b/launcher_utils/register_namespace_test.go index f303e02fb..2fd11d7c6 100644 --- a/launcher_utils/register_namespace_test.go +++ b/launcher_utils/register_namespace_test.go @@ -220,6 +220,11 @@ func TestMultiKeysRegistration(t *testing.T) { keysMap := config.GetIssuerPrivateKeys() require.Equal(t, 1, len(keysMap)) + // Get the key name (so we can delete it later) + dirEntries, err := os.ReadDir(keysDir) + require.NoError(t, err) + require.Equal(t, 1, len(dirEntries)) + // Create a new issuer key and rotate out the old one secondKey, err := config.GeneratePEM(keysDir) require.NoError(t, err) @@ -231,7 +236,9 @@ func TestMultiKeysRegistration(t *testing.T) { require.NoError(t, err) activeKey, err := config.GetIssuerPrivateJWK() require.NoError(t, err) - require.Equal(t, secondKey, activeKey) + // Note: late in the development of this work, we switched to + // having GetIssuerPrivateJWK return the oldest active key + require.Equal(t, privKey, activeKey) keysMap = config.GetIssuerPrivateKeys() require.Equal(t, secondKey, keysMap[secondKey.KeyID()]) require.Equal(t, privKey, keysMap[key.KeyID()]) @@ -250,6 +257,9 @@ func TestMultiKeysRegistration(t *testing.T) { viper.Set("Federation.RegistryUrl", svr.URL) viper.Set("Origin.FederationPrefix", "/test123") + // Remove the original key, forcing us to register with the new one + require.NoError(t, os.Remove(filepath.Join(keysDir, dirEntries[0].Name()))) + // Re-run the InitServer to reflect the new RegistryUrl set above require.NoError(t, config.InitServer(ctx, server_structs.OriginType)) @@ -286,7 +296,7 @@ func TestMultiKeysRegistration(t *testing.T) { keySet, err := jwk.Parse([]byte(entries[0].Pubkey)) require.NoError(t, err) registryKey, isPresent := keySet.LookupKeyID(secondKeyId) - assert.True(t, isPresent) + require.True(t, isPresent) assert.True(t, jwk.Equal(registryKey, secondPubKey)) assert.Equal(t, "mock_site_name", entries[0].AdminMetadata.SiteName) diff --git a/registry/client_commands_test.go b/registry/client_commands_test.go index 7c949fda5..02502f835 100644 --- a/registry/client_commands_test.go +++ b/registry/client_commands_test.go @@ -212,7 +212,8 @@ func TestNamespaceRegisteredPubKeyUpdate(t *testing.T) { require.NoError(t, err) privKey2, err := config.GetIssuerPrivateJWK() require.NoError(t, err) - require.Equal(t, newKey.KeyID(), privKey2.KeyID()) + require.NotEqual(t, privKey.KeyID(), newKey.KeyID()) + assert.Equal(t, privKey.KeyID(), privKey2.KeyID()) err = NamespacesPubKeyUpdate(privKey2, []string{"/foo/bar"}, "mock_site_name", svr.URL+"/api/v1.0/registry/updateNamespacesPubKey") require.NoError(t, err) }) From 060c7aba647bb5bf53f8b9ab1d3f79c89ce96c1d Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Wed, 22 Jan 2025 14:58:04 -0600 Subject: [PATCH 7/8] Minor fixes from code review --- launchers/launcher.go | 5 +-- registry/client_commands.go | 7 ++++ registry/registry_pubkey_update.go | 47 +++++++++++++++++++----- registry/registry_test.go | 57 ++++++++++++++++++++++++++++++ token/token_create.go | 3 ++ 5 files changed, 106 insertions(+), 13 deletions(-) diff --git a/launchers/launcher.go b/launchers/launcher.go index 443f10a07..bf49d4b39 100644 --- a/launchers/launcher.go +++ b/launchers/launcher.go @@ -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 diff --git a/registry/client_commands.go b/registry/client_commands.go index 23b5332dd..f4d8221fc 100644 --- a/registry/client_commands.go +++ b/registry/client_commands.go @@ -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 { diff --git a/registry/registry_pubkey_update.go b/registry/registry_pubkey_update.go index 67f1f5181..b6acb4318 100644 --- a/registry/registry_pubkey_update.go +++ b/registry/registry_pubkey_update.go @@ -19,6 +19,7 @@ package registry import ( + "context" "crypto/ecdsa" "encoding/hex" "encoding/json" @@ -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) { @@ -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 { @@ -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 } } diff --git a/registry/registry_test.go b/registry/registry_test.go index 43ad3d7e0..5706f6d75 100644 --- a/registry/registry_test.go +++ b/registry/registry_test.go @@ -20,6 +20,9 @@ package registry import ( "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" "encoding/json" "fmt" "io" @@ -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") + }) +} diff --git a/token/token_create.go b/token/token_create.go index af6c99e21..ae1a6ddcc 100644 --- a/token/token_create.go +++ b/token/token_create.go @@ -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 { From 190e6775786ee1e79eff1fcb1bcbb9c797fa3dd1 Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Wed, 22 Jan 2025 15:17:30 -0600 Subject: [PATCH 8/8] Fix typo caught by new linter --- registry/registry_pubkey_update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/registry_pubkey_update.go b/registry/registry_pubkey_update.go index b6acb4318..5a21605f9 100644 --- a/registry/registry_pubkey_update.go +++ b/registry/registry_pubkey_update.go @@ -236,7 +236,7 @@ func updateNsKeySignChallengeCommit(data *RegisteredPrefixUpdate) (map[string]in } -// Handle the registered namespace public key update with nonce generation and verifcation +// Handle the registered namespace public key update with nonce generation and verification func updateNsKeySignChallenge(data *RegisteredPrefixUpdate) (map[string]interface{}, error) { if data.ClientNonce != "" && data.ClientPayload != "" && data.ClientSignature != "" && data.ServerNonce != "" && data.ServerPayload != "" && data.ServerSignature != "" {