From 3777b373a34e9e085743d28a6a74c68ac5b98df7 Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Thu, 31 Aug 2023 18:41:12 +0000 Subject: [PATCH 1/2] Use lestrrat jwks instead of custom structs in NS Registry There were missing key values in the registry database (like kty, kid, etc) because of the custom structs being used to create the jwks that got stored. By using lestrrat's jwk package, we get those values for free. --- namespace-registry/client_commands.go | 36 +++---- namespace-registry/registry.go | 142 +++++++++++++------------- 2 files changed, 82 insertions(+), 96 deletions(-) diff --git a/namespace-registry/client_commands.go b/namespace-registry/client_commands.go index d0e9be304..81ca9588b 100644 --- a/namespace-registry/client_commands.go +++ b/namespace-registry/client_commands.go @@ -6,16 +6,15 @@ import ( "bufio" "bytes" - "encoding/base64" "encoding/hex" "encoding/json" "fmt" "io" - "math/big" "net/http" "os" "github.com/pelicanplatform/pelican/config" + log "github.com/sirupsen/logrus" "github.com/spf13/viper" ) @@ -118,9 +117,13 @@ func NamespaceRegister(privateKeyPath string, namespaceRegistryEndpoint string, return errors.Wrap(err, "Failed to retrieve public key") } - jwks, err := config.JWKSMap(publicKey) - if err != nil { - return errors.Wrap(err, "Failed to convert public key to JWKS") + if log.GetLevel() == log.DebugLevel || log.GetLevel() == log.TraceLevel { + // Let's check that we can convert to JSON and get the right thing... + jsonbuf, err := json.Marshal(publicKey) + if err != nil { + return errors.Wrap(err, "failed to marshal the public key into JWKS JSON") + } + log.Debugf("Constructed JWKS from loading public key: %s\n", jsonbuf) } privateKey, err := config.LoadPrivateKey(privateKeyPath) @@ -135,7 +138,7 @@ func NamespaceRegister(privateKeyPath string, namespaceRegistryEndpoint string, data := map[string]interface{}{ "client_nonce": clientNonce, - "pubkey": fmt.Sprintf("%x", jwks["x"]), + "pubkey": publicKey, } resp, err := makeRequest(namespaceRegistryEndpoint, "POST", data) @@ -163,24 +166,11 @@ func NamespaceRegister(privateKeyPath string, namespaceRegistryEndpoint string, return errors.Wrap(err, "Failed to sign payload") } - // Create data for the second POST request - xBytes, err := base64.RawURLEncoding.DecodeString(jwks["x"]) - if err != nil { - return errors.Wrap(err, "Failed to decode jwks.x") - } - yBytes, err := base64.RawURLEncoding.DecodeString(jwks["y"]) - if err != nil { - return errors.Wrap(err, "Failed to decode jwks.y") - } - + // // Create data for the second POST request unidentifiedPayload := map[string]interface{}{ - "client_nonce": clientNonce, - "server_nonce": respData.ServerNonce, - "pubkey": map[string]string{ - "x": new(big.Int).SetBytes(xBytes).String(), - "y": new(big.Int).SetBytes(yBytes).String(), - "curve": jwks["crv"], - }, + "client_nonce": clientNonce, + "server_nonce": respData.ServerNonce, + "pubkey": publicKey, "client_payload": clientPayload, "client_signature": hex.EncodeToString(signature), "server_payload": respData.ServerPayload, diff --git a/namespace-registry/registry.go b/namespace-registry/registry.go index e82b79813..97a1af9f6 100644 --- a/namespace-registry/registry.go +++ b/namespace-registry/registry.go @@ -1,26 +1,27 @@ package nsregistry import ( + "context" "crypto" "crypto/ecdsa" - "crypto/elliptic" "crypto/rand" "crypto/sha256" "encoding/hex" "encoding/json" - "github.com/gin-gonic/gin" - "github.com/pelicanplatform/pelican/config" - "github.com/pkg/errors" - log "github.com/sirupsen/logrus" - "github.com/spf13/viper" "io" - "math/big" "net/http" "net/url" "os" "strings" "sync" + "github.com/gin-gonic/gin" + "github.com/lestrrat-go/jwx/v2/jwk" + "github.com/pelicanplatform/pelican/config" + "github.com/pkg/errors" + log "github.com/sirupsen/logrus" + "github.com/spf13/viper" + // use this sqlite driver instead of the one from // github.com/mattn/go-sqlite3, because this one // doesn't require compilation with CGO_ENABLED @@ -231,83 +232,78 @@ func keySignChallengeInit(ctx *gin.Context, data *registrationData) error { return nil } -type jwks struct { - X string `json:"x"` - Y string `json:"y"` -} - -func jwksToEcdsaPublicKey(jwks *jwks) (*ecdsa.PublicKey, error) { - xBigInt, err := new(big.Int).SetString(jwks.X, 10) - if !err { - return nil, errors.New("Failed to convert jwks.x to Big Int") - } - yBigInt, err := new(big.Int).SetString(jwks.Y, 10) - if !err { - return nil, errors.New("Failed to convert jwks.y to Big Int") +func keySignChallengeCommit(ctx *gin.Context, data *registrationData, action string) error { + // Parse the client's jwks as a set here + clientJwks, err := jwk.Parse(data.Pubkey) + if err != nil { + return errors.Wrap(err, "Couldn't parse the pubkey from the client") } - clientPubkey := &ecdsa.PublicKey{ - Curve: elliptic.P521(), - X: xBigInt, - Y: yBigInt, + if log.GetLevel() == log.DebugLevel || log.GetLevel() == log.TraceLevel { + // Let's check that we can convert to JSON and get the right thing... + jsonbuf, err := json.Marshal(clientJwks) + if err != nil { + return errors.Wrap(err, "failed to marshal the client's keyset into JSON") + } + log.Debugf("Client JWKS as seen by the registry server: %s\n", jsonbuf) } - return clientPubkey, nil -} + // Now iterate through the jwks and verify signatures: + // Note that the examples in the docs for lestrrat/jwx/v2/jwk are outdated and still point to the + // old v1 method for iterating throug keysets (ie they use clientJWKS.Iterate(context.Background()) instead of + // clientJWKS.Keys(context.Background())) + for it := clientJwks.Keys(context.Background()); it.Next(context.Background()); { + pair := it.Pair() + key := pair.Value.(jwk.Key) -func keySignChallengeCommit(ctx *gin.Context, data *registrationData, action string) error { - var pubkeyJwks jwks - if err := json.Unmarshal(data.Pubkey, &pubkeyJwks); err != nil { - ctx.JSON(500, gin.H{"error": "Failed to unmarshal the provided pubkey"}) - return errors.Wrap(err, "Failed to unmarshal the provided pubkey") - } + var rawkey interface{} // This is the raw key, like *rsa.PrivateKey or *ecdsa.PrivateKey + if err := key.Raw(&rawkey); err != nil { + return errors.Wrap(err, "failed to generate raw pubkey from jwks") + } - clientPubkey, err := jwksToEcdsaPublicKey(&pubkeyJwks) - if err != nil { - return errors.Wrap(err, "Failed to convert jwks to ECDSA pubkey") - } - clientPayload := []byte(data.ClientNonce + data.ServerNonce) - clientSignature, err := hex.DecodeString(data.ClientSignature) - if err != nil { - ctx.JSON(500, gin.H{"error": "Failed to decode client's signature"}) - return errors.Wrap(err, "Failed to decode the client's signature") - } - clientVerified := verifySignature(clientPayload, clientSignature, clientPubkey) + clientPayload := []byte(data.ClientNonce + data.ServerNonce) + clientSignature, err := hex.DecodeString(data.ClientSignature) + if err != nil { + ctx.JSON(500, gin.H{"error": "Failed to decode client's signature"}) + return 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 { - ctx.JSON(500, gin.H{"error": "Failed to decode the server's payload"}) - return errors.Wrap(err, "Failed to decode the server's payload") - } + serverPayload, err := hex.DecodeString(data.ServerPayload) + if err != nil { + ctx.JSON(500, gin.H{"error": "Failed to decode the server's payload"}) + return errors.Wrap(err, "Failed to decode the server's payload") + } - serverSignature, err := hex.DecodeString(data.ServerSignature) - if err != nil { - ctx.JSON(500, gin.H{"error": "Failed to decode the server's signature"}) - return errors.Wrap(err, "Failed to decode the server's signature") - } + serverSignature, err := hex.DecodeString(data.ServerSignature) + if err != nil { + ctx.JSON(500, gin.H{"error": "Failed to decode the server's signature"}) + return errors.Wrap(err, "Failed to decode the server's signature") + } - serverPrivateKey, err := loadServerKeys() - if err != nil { - ctx.JSON(500, gin.H{"error": "Failed to load server's private key"}) - return errors.Wrap(err, "Failed to decode the server's private key") - } - serverPubkey := serverPrivateKey.PublicKey - serverVerified := verifySignature(serverPayload, serverSignature, &serverPubkey) - - if clientVerified && serverVerified { - if action == "register" { - log.Debug("Registering namespace ", data.Prefix) - err = dbAddNamespace(ctx, data) - if err != nil { - ctx.JSON(500, gin.H{"error": "The server encountered an error while attempting to add the prefix to its database"}) - return errors.Wrapf(err, "Failed while trying to add to database") + serverPrivateKey, err := loadServerKeys() + if err != nil { + ctx.JSON(500, gin.H{"error": "Failed to load server's private key"}) + return errors.Wrap(err, "Failed to decode the server's private key") + } + serverPubkey := serverPrivateKey.PublicKey + serverVerified := verifySignature(serverPayload, serverSignature, &serverPubkey) + + if clientVerified && serverVerified { + if action == "register" { + log.Debug("Registering namespace ", data.Prefix) + err = dbAddNamespace(ctx, data) + if err != nil { + ctx.JSON(500, gin.H{"error": "The server encountered an error while attempting to add the prefix to its database"}) + return errors.Wrapf(err, "Failed while trying to add to database") + } + return nil } - return nil + } else { + ctx.JSON(500, gin.H{"error": "Server was either unable to verify the client's public key, or an encountered an error with its own"}) + return errors.Errorf("Either the server or the client could not be verified: "+ + "server verified:%t, client verified:%t", serverVerified, clientVerified) } - } else { - ctx.JSON(500, gin.H{"error": "Server was either unable to verify the client's public key, or an encountered an error with its own"}) - return errors.Errorf("Either the server or the client could not be verified: "+ - "server verified:%t, client verified:%t", serverVerified, clientVerified) } return nil } From 50156a710f044845f4ec2adb25b6f90b7ae2c07c Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Fri, 1 Sep 2023 17:32:37 +0000 Subject: [PATCH 2/2] Incorporate feedback from review I also made the decision to enforce that the jwks used to register a namespace prefix only contain one key. This simplifies things at both the client and at the registry, because we need not figure out which key was used for the handshake signature, and we need not support multiple signatures during this part of the process. Eventually we need to expose an API that allows origins to add/verify other public keys to the registry's jwks database. --- config/init_server_creds.go | 28 ------- namespace-registry/client_commands.go | 16 +++- namespace-registry/registry.go | 108 +++++++++++++------------- 3 files changed, 69 insertions(+), 83 deletions(-) diff --git a/config/init_server_creds.go b/config/init_server_creds.go index a0cd7e7de..a15ef5f21 100644 --- a/config/init_server_creds.go +++ b/config/init_server_creds.go @@ -6,7 +6,6 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" - "encoding/json" "encoding/pem" "fmt" "math/big" @@ -233,30 +232,3 @@ func GetOriginJWK() (*jwk.Key, error) { } return key, nil } - -func JWKSMap(jwks *jwk.Set) (map[string]string, error) { - // Marshal the set into JSON - jsonBytes, err := json.MarshalIndent(jwks, "", " ") - if err != nil { - return nil, err - } - - // Parse the JSON into a structure we can manipulate - var parsed map[string][]map[string]interface{} - err = json.Unmarshal(jsonBytes, &parsed) - if err != nil { - return nil, err - } - - // Convert the map[string]interface{} to map[string]string - stringMaps := make([]map[string]string, len(parsed["keys"])) - for i, m := range parsed["keys"] { - stringMap := make(map[string]string) - for k, v := range m { - stringMap[k] = fmt.Sprintf("%v", v) - } - stringMaps[i] = stringMap - } - - return stringMaps[0], nil -} diff --git a/namespace-registry/client_commands.go b/namespace-registry/client_commands.go index 81ca9588b..a7438c07c 100644 --- a/namespace-registry/client_commands.go +++ b/namespace-registry/client_commands.go @@ -2,6 +2,7 @@ package nsregistry import ( "crypto/tls" + "github.com/pkg/errors" "bufio" @@ -117,13 +118,24 @@ func NamespaceRegister(privateKeyPath string, namespaceRegistryEndpoint string, return errors.Wrap(err, "Failed to retrieve public key") } - if log.GetLevel() == log.DebugLevel || log.GetLevel() == log.TraceLevel { + /* + * TODO: For now, we only allow namespace registration to occur with a single key, but + * at some point we should expose an API for adding additional pubkeys to each + * namespace. There is a similar TODO listed in registry.go, as the choices made + * there mirror the choices made here. + * To enforce that we're only trying to register one key, we check the length here + */ + if (*publicKey).Len() > 1 { + return errors.Errorf("Only one public key can be registered in this step, but %d were provided\n", (*publicKey).Len()) + } + + if log.IsLevelEnabled(log.DebugLevel) { // Let's check that we can convert to JSON and get the right thing... jsonbuf, err := json.Marshal(publicKey) if err != nil { return errors.Wrap(err, "failed to marshal the public key into JWKS JSON") } - log.Debugf("Constructed JWKS from loading public key: %s\n", jsonbuf) + log.Debugln("Constructed JWKS from loading public key:", string(jsonbuf)) } privateKey, err := config.LoadPrivateKey(privateKeyPath) diff --git a/namespace-registry/registry.go b/namespace-registry/registry.go index 97a1af9f6..ed38a2eb0 100644 --- a/namespace-registry/registry.go +++ b/namespace-registry/registry.go @@ -1,7 +1,7 @@ package nsregistry import ( - "context" + // "context" "crypto" "crypto/ecdsa" "crypto/rand" @@ -239,71 +239,73 @@ func keySignChallengeCommit(ctx *gin.Context, data *registrationData, action str return errors.Wrap(err, "Couldn't parse the pubkey from the client") } - if log.GetLevel() == log.DebugLevel || log.GetLevel() == log.TraceLevel { + if log.IsLevelEnabled(log.DebugLevel) { // Let's check that we can convert to JSON and get the right thing... jsonbuf, err := json.Marshal(clientJwks) if err != nil { return errors.Wrap(err, "failed to marshal the client's keyset into JSON") } - log.Debugf("Client JWKS as seen by the registry server: %s\n", jsonbuf) + log.Debugln("Client JWKS as seen by the registry server:", string(jsonbuf)) } - // Now iterate through the jwks and verify signatures: - // Note that the examples in the docs for lestrrat/jwx/v2/jwk are outdated and still point to the - // old v1 method for iterating throug keysets (ie they use clientJWKS.Iterate(context.Background()) instead of - // clientJWKS.Keys(context.Background())) - for it := clientJwks.Keys(context.Background()); it.Next(context.Background()); { - pair := it.Pair() - key := pair.Value.(jwk.Key) - - var rawkey interface{} // This is the raw key, like *rsa.PrivateKey or *ecdsa.PrivateKey - if err := key.Raw(&rawkey); err != nil { - return errors.Wrap(err, "failed to generate raw pubkey from jwks") - } + /* + * TODO: This section makes the assumption that the incoming jwks only contains a single + * key, a property that is enforced by the client at the origin. Eventually we need + * to support the addition of other keys in the jwks stored for the origin. There is + * a similar TODO listed in client_commands.go, as the choices made there mirror the + * choices made here. + */ + key, exists := clientJwks.Key(0) + if !exists { + return errors.New("There was no key at index 0 in the client's JWKS. Something is wrong") + } - clientPayload := []byte(data.ClientNonce + data.ServerNonce) - clientSignature, err := hex.DecodeString(data.ClientSignature) - if err != nil { - ctx.JSON(500, gin.H{"error": "Failed to decode client's signature"}) - return errors.Wrap(err, "Failed to decode the client's signature") - } - clientVerified := verifySignature(clientPayload, clientSignature, (rawkey).(*ecdsa.PublicKey)) + var rawkey interface{} // This is the raw key, like *rsa.PrivateKey or *ecdsa.PrivateKey + if err := key.Raw(&rawkey); err != nil { + return errors.Wrap(err, "failed to generate raw pubkey from jwks") + } - serverPayload, err := hex.DecodeString(data.ServerPayload) - if err != nil { - ctx.JSON(500, gin.H{"error": "Failed to decode the server's payload"}) - return errors.Wrap(err, "Failed to decode the server's payload") - } + clientPayload := []byte(data.ClientNonce + data.ServerNonce) + clientSignature, err := hex.DecodeString(data.ClientSignature) + if err != nil { + ctx.JSON(500, gin.H{"error": "Failed to decode client's signature"}) + return 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 { + ctx.JSON(500, gin.H{"error": "Failed to decode the server's payload"}) + return errors.Wrap(err, "Failed to decode the server's payload") + } - serverSignature, err := hex.DecodeString(data.ServerSignature) - if err != nil { - ctx.JSON(500, gin.H{"error": "Failed to decode the server's signature"}) - return errors.Wrap(err, "Failed to decode the server's signature") - } + serverSignature, err := hex.DecodeString(data.ServerSignature) + if err != nil { + ctx.JSON(500, gin.H{"error": "Failed to decode the server's signature"}) + return errors.Wrap(err, "Failed to decode the server's signature") + } - serverPrivateKey, err := loadServerKeys() - if err != nil { - ctx.JSON(500, gin.H{"error": "Failed to load server's private key"}) - return errors.Wrap(err, "Failed to decode the server's private key") - } - serverPubkey := serverPrivateKey.PublicKey - serverVerified := verifySignature(serverPayload, serverSignature, &serverPubkey) - - if clientVerified && serverVerified { - if action == "register" { - log.Debug("Registering namespace ", data.Prefix) - err = dbAddNamespace(ctx, data) - if err != nil { - ctx.JSON(500, gin.H{"error": "The server encountered an error while attempting to add the prefix to its database"}) - return errors.Wrapf(err, "Failed while trying to add to database") - } - return nil + serverPrivateKey, err := loadServerKeys() + if err != nil { + ctx.JSON(500, gin.H{"error": "Failed to load server's private key"}) + return errors.Wrap(err, "Failed to decode the server's private key") + } + serverPubkey := serverPrivateKey.PublicKey + serverVerified := verifySignature(serverPayload, serverSignature, &serverPubkey) + + if clientVerified && serverVerified { + if action == "register" { + log.Debug("Registering namespace ", data.Prefix) + err = dbAddNamespace(ctx, data) + if err != nil { + ctx.JSON(500, gin.H{"error": "The server encountered an error while attempting to add the prefix to its database"}) + return errors.Wrapf(err, "Failed while trying to add to database") } - } else { - ctx.JSON(500, gin.H{"error": "Server was either unable to verify the client's public key, or an encountered an error with its own"}) - return errors.Errorf("Either the server or the client could not be verified: "+ - "server verified:%t, client verified:%t", serverVerified, clientVerified) + return nil } + } else { + ctx.JSON(500, gin.H{"error": "Server was either unable to verify the client's public key, or an encountered an error with its own"}) + return errors.Errorf("Either the server or the client could not be verified: "+ + "server verified:%t, client verified:%t", serverVerified, clientVerified) } return nil }