fix(schema): validate public and private key for keys entity (#11923)

KAG-390

---------

Co-authored-by: Datong Sun <[email protected]>

changelog/unreleased/kong/validate_private_key.yml

@@ -0,0 +1,3 @@
+message: Validate private and public key for `keys` entity to ensure they match each other.
+type: bugfix
+scope: Core

kong/db/dao/keys.lua

@@ -76,14 +76,20 @@ local function _load_pkey(key, part)
     pk, err = pkey.new(key.jwk, { format = "JWK" })
   end
   if key.pem then
-    if not key.pem[part] then
-      return nil, fmt("%s key not found.", part)
+    -- public key can be derived from private key, but not vice versa
+    if part == "private_key" and not key.pem[part] then
+      return nil, "could not load a private key from public key material"
     end
     pk, err = pkey.new(key.pem[part], { format = "PEM" })
   end
   if not pk then
     return nil, "could not load pkey. " .. err
   end
+
+  if part == "private_key" and not pk:is_private() then
+    return nil, "could not load a private key from public key material"
+  end
+
   return pk
 end
 

kong/db/schema/typedefs.lua

@@ -654,20 +654,34 @@ local function validate_pem_keys(values)
   local private_key = values.private_key
 
   -- unless it's a vault reference
-  if kong.vault.is_reference(private_key) or
-     kong.vault.is_reference(public_key) then
+  if kong and (
+     kong.vault.is_reference(private_key) or
+     kong.vault.is_reference(public_key)) then
     return true
   end
 
-  local pk, err = openssl_pkey.new(public_key, { format = "PEM" })
-  if not pk or err then
-    return false, "could not load public key"
+  local pubkey, privkey, err
+
+  if public_key and public_key ~= null then
+    pubkey, err = openssl_pkey.new(public_key, { format = "PEM", type = "pu" })
+    if not pubkey or err then
+      return false, "could not load public key"
+    end
   end
 
-  local ppk, perr = openssl_pkey.new(private_key, { format = "PEM" })
-  if not ppk or perr then
-    return false, "could not load private key" .. (perr or "")
+  if private_key and private_key ~= null then
+    privkey, err = openssl_pkey.new(private_key, { format = "PEM", type = "pr" })
+    if not privkey or err then
+      return false, "could not load private key" .. (err or "")
+    end
   end
+
+  if privkey and pubkey then
+    if privkey:to_PEM("public") ~= pubkey:to_PEM() then
+      return false, "public key does not match private key"
+    end
+  end
+
   return true
 end
 
@@ -691,6 +705,9 @@ typedefs.pem = Schema.define {
       },
     },
   },
+  entity_checks = {
+    { at_least_one_of = { "private_key", "public_key" } }
+  },
   custom_validator = validate_pem_keys,
   description = "A pair of PEM-encoded public and private keys, which can be either a string or a reference to a credential in Kong Vault. If provided as strings, they must be valid PEM-encoded keys."
 

kong/plugins/acme/client.lua

@@ -234,13 +234,7 @@ local function get_account_key(conf)
     local key_set, key_set_err = kong.db.key_sets:select_by_name(conf.key_set)
 
     if key_set_err then
-      kong.log.warn("error loading keyset ", conf.key_set, " : ", key_set_err)
-      return nil, key_set_err
-    end
-
-    if not key_set then
-      kong.log.warn("could not load keyset nil value was returned")
-      return nil, error("nil returned by key_sets:select_by_name for key_set ", conf.key_set)
+      return nil, "could not load keyset: " .. key_set_err
     end
 
     lookup.set = {id = key_set.id}
@@ -250,13 +244,7 @@ local function get_account_key(conf)
   local key, key_err = kong.db.keys:select_by_cache_key(cache_key)
 
   if key_err then
-    kong.log.warn("error loading key ", kid, " : ", key_err)
-    return nil, key_err
-  end
-
-  if not key then
-    kong.log.warn("could not load key nil value was returned")
-    return nil, error("nil returned by keys:select_by_cache_key for key ", conf.key_id)
+    return nil, "could not load keys: " .. key_err
   end
 
   return kong.db.keys:get_privkey(key)

spec/01-unit/01-db/01-schema/03-typedefs_spec.lua

@@ -203,4 +203,24 @@ describe("typedefs", function()
     assert.equal(false, uuid2.auto)
  end)
 
+ it("features pem", function()
+  local Test = Schema.new({
+    fields = {
+      { f = typedefs.pem }
+    }
+  })
+  local tmpkey = openssl_pkey.new { type = 'EC', curve = 'prime256v1' }
+  assert.truthy(Test:validate({ f = { public_key = tmpkey:to_PEM("public") }}))
+  assert.truthy(Test:validate({ f = { private_key = tmpkey:to_PEM("private") }}))
+  assert.falsy( Test:validate({ f = { private_key = tmpkey:to_PEM("public") }}))
+  assert.falsy(Test:validate({ f = { public_key = tmpkey:to_PEM("private") }}))
+  assert.truthy(Test:validate({ f = { public_key = tmpkey:to_PEM("public"),
+                                private_key = tmpkey:to_PEM("private") }}))
+  local anotherkey = openssl_pkey.new { type = 'EC', curve = 'prime256v1' }
+  assert.falsy( Test:validate({ f = { public_key = anotherkey:to_PEM("public"),
+                                private_key = tmpkey:to_PEM("private") }}))
+  assert.falsy( Test:validate({ f = { public_key = tmpkey:to_PEM("public"),
+                                private_key = anotherkey:to_PEM("private") }}))
+end)
+
 end)

spec/02-integration/03-db/18-keys_spec.lua

@@ -207,5 +207,47 @@ for _, strategy in helpers.all_strategies() do
       assert.is_not_nil(decoded_jwk.q)
       assert.is_not_nil(decoded_jwk.qi)
     end)
+
+    it(":get_privkey errors if only got pubkey [pem]", function()
+      local pem_t, err = db.keys:insert {
+        name = "pem_key",
+        set = init_key_set,
+        kid = "999",
+        pem = { public_key = pem_pub }
+      }
+      assert.is_nil(err)
+      assert(pem_t)
+
+      local pem_pub_t, g_err = db.keys:get_pubkey(pem_t)
+      assert.is_nil(g_err)
+      assert.matches("-----BEGIN PUBLIC KEY", pem_pub_t)
+
+      local pem_priv, p_err = db.keys:get_privkey(pem_t)
+      assert.is_nil(pem_priv)
+      assert.matches("could not load a private key from public key material", p_err)
+    end)
+
+    it(":get_privkey errors if only got pubkey [jwk]", function()
+      jwk.d = nil
+      local jwk_t, _ = db.keys:insert {
+        name = "jwk_key",
+        set = init_key_set,
+        kid = jwk.kid,
+        jwk = cjson.encode(jwk)
+      }
+      assert(jwk_t)
+
+      local jwk_pub_t, g_err = db.keys:get_pubkey(jwk_t)
+      assert.is_nil(g_err)
+      local jwk_pub_o = cjson.decode(jwk_pub_t)
+      assert.is_not_nil(jwk_pub_o.e)
+      assert.is_not_nil(jwk_pub_o.kid)
+      assert.is_not_nil(jwk_pub_o.kty)
+      assert.is_not_nil(jwk_pub_o.n)
+
+      local jwk_priv, p_err = db.keys:get_privkey(jwk_t)
+      assert.is_nil(jwk_priv)
+      assert.matches("could not load a private key from public key material", p_err)
+    end)
   end)
 end