Added check to prevent upload of certificates (#980)

pagopa · Sep 17, 2024 · 900cf0a · 900cf0a
1 parent 7852ad7
commit 900cf0a
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 20 deletions.
diff --git a/packages/authorization-process/src/utilities/errorMappers.ts b/packages/authorization-process/src/utilities/errorMappers.ts
@@ -120,6 +120,7 @@ export const createKeysErrorMapper = (error: ApiError<ErrorCodes>): number =>
     .with(
       "tooManyKeysPerClient",
       "notAllowedPrivateKeyException",
+      "notAllowedCertificateException",
       "jwkDecodingError",
       () => HTTP_STATUS_BAD_REQUEST
     )
@@ -220,6 +221,7 @@ export const createProducerKeychainKeyErrorMapper = (
     .with(
       "tooManyKeysPerProducerKeychain",
       "notAllowedPrivateKeyException",
+      "notAllowedCertificateException",
       () => HTTP_STATUS_BAD_REQUEST
     )
     .with("keyAlreadyExists", () => HTTP_STATUS_CONFLICT)

diff --git a/packages/authorization-process/test/createKeys.test.ts b/packages/authorization-process/test/createKeys.test.ts
@@ -10,6 +10,7 @@ import {
   UserId,
   generateId,
   invalidKey,
+  notAllowedCertificateException,
   notAllowedPrivateKeyException,
   toClientV2,
 } from "pagopa-interop-models";
@@ -399,4 +400,27 @@ describe("createKeys", () => {
       invalidKey(keySeed.key, "error:1E08010C:DECODER routines::unsupported")
     );
   });
+  it("should throw notAllowedCertificateException if the key contains a certificate", async () => {
+    mockSelfcareV2ClientCall([mockSelfCareUsers]);
+
+    await addOneClient(mockClient);
+
+    const cert =
+      "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUROakNDQWg2Z0F3SUJBZ0lHQVpJQXNpaThNQTBHQ1NxR1NJYjNEUUVCQ3dVQU1Gd3hEakFNQmdOVkJBTU1CVkJ5YjNaaE1Rc3dDUVlEVlFRR0V3SlZVekVPTUF3R0ExVUVDd3dGY0hKdmRtRXhEakFNQmdOVkJBY01CWEJ5YjNaaE1SMHdHd1lKS29aSWh2Y05BUWtCRmc1d2NtOTJZVUJ0WVdsc0xtTnZiVEFlRncweU5EQTVNVGN4TlRVMU1qaGFGdzB5TlRBNU1UY3hOVFUxTWpoYU1Gd3hEakFNQmdOVkJBTU1CVkJ5YjNaaE1Rc3dDUVlEVlFRR0V3SlZVekVPTUF3R0ExVUVDd3dGY0hKdmRtRXhEakFNQmdOVkJBY01CWEJ5YjNaaE1SMHdHd1lKS29aSWh2Y05BUWtCRmc1d2NtOTJZVUJ0WVdsc0xtTnZiVENDQVNJd0RRWUpLb1pJaHZjTkFRRUJCUUFEZ2dFUEFEQ0NBUW9DZ2dFQkFLR0xCZHJacmpRLzRqd2h3Y1ZhU0kvTlgyV1ozMGx3VE5wVHlhMjhXSDhEOGlHVTlZdG1CL0s3cXUxQjhaOGtGaWJtditteGh1dS8rbStGRTg4SmIwM3pnU0liT1JwY0FSaThmWGFuZzM1eG8rdmh1bE5iL2x5bWthaFZ5ekRCSER0aXl5WUlUZTdsMmNPT0hPdm5MbDhRZERpZUhjOUNmcVJYTDhVeFlNaG1wd2QyMlVOK1BsNE1SNXFhbVFFZkp4cGxLdllva1NYTUdrb1QxWEJHcitmSDBsL0ZKRmxZT3R6QUdvSm5xUkNwTDRiNlZUeGxZZlZuUXhaNnpSQkRlbTd0dDhraGJncm1Va2hIWG1YaTNuL1A2RktEQnNyNG9WbjlUU2QyUENwL1VzQmtiKzB1RktuVFlyZyt5aSt5NVBZb2NmbTZUbnl1bUVOU2VHNGZxSUVDQXdFQUFUQU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FRRUFGdDRjOUs0TWJOV0Ewb1k1RzdiWHQ1RW5FeXNLNTh0R1RSU2xpTG14aTRxSXE3eTNQMjExTUcvTDhpc211WVNybkF2Q29Yb1ZJbDZldzloOGh4eGl4N2QvR3dNc3lISzhQcm5SamZIU2pmL1ZzcGJXdTVPOEQzV1RtanNjOTVnMTZIbmgrS2NiaWFzN0FFNi95d2EvK1ZrdG55dnROL21vQzdtc2R3eForS3o1Nld0WTkzWVpBTkQwSUx2Y1pVRlNMbUdsNTI5Q2lxdlByVURlY2RFVWFob3J4VXozdWJ1ZmJjNW5PWTV6RU1FNkNWNG9SUWJzWTBmbWxoU1Q5Y20xc1ZSUmJsV1VNSTcyeGRGZFJIc04wcFpNaWYvVWpKTGhBTTQ0SUxSZXYwenRjMnlYMGtZUXBSejJmcWtucnY3S1ZRU1dwcjM4QlZSV3NJU0J5NnFZbHc9PQotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0t";
+
+    expect(
+      authorizationService.createKeys({
+        clientId: mockClient.id,
+        authData: mockAuthData,
+        keysSeeds: [
+          {
+            ...keySeed,
+            key: cert,
+          },
+        ],
+        correlationId: generateId(),
+        logger: genericLogger,
+      })
+    ).rejects.toThrowError(notAllowedCertificateException());
+  });
 });
diff --git a/packages/commons/src/auth/jwk.ts b/packages/commons/src/auth/jwk.ts
@@ -2,6 +2,7 @@ import crypto, { JsonWebKey, KeyObject } from "crypto";
 import {
   invalidKey,
   jwkDecodingError,
+  notAllowedCertificateException,
   notAllowedPrivateKeyException,
 } from "pagopa-interop-models";
 
@@ -24,32 +25,37 @@ export const calculateKid = (jwk: JsonWebKey): string => {
   return crypto.createHash("sha256").update(jwkString).digest("base64url");
 };
 
-function createPublicKey(key: string): KeyObject {
-  /*
-  Validation of a public key in PEM format.
-  The standard library does not provide a specific method.
-  Note: crypto.createPublicKey cannot be used directly because it succeeds also when providing a private key.
-  In order to perform the check, the function:
-    1. tries to create a private key
-      - success: the value is a private key and the function fails
-      - failure: the value is not a private key and the function proceeds
-    2. tries to create a public key
-      - success: the value is a public key
-      - failure: the value is not a key
-  */
-  const pemKey = decodeBase64ToPem(key);
+function assertNotCertificate(key: string): void {
+  try {
+    new crypto.X509Certificate(key);
+  } catch (error) {
+    return;
+  }
+  throw notAllowedCertificateException();
+}
+
+function assertNotPrivateKey(key: string): void {
   try {
-    crypto.createPrivateKey(pemKey);
+    crypto.createPrivateKey(key);
   } catch {
-    try {
-      return crypto.createPublicKey(pemKey);
-    } catch (error) {
-      throw invalidKey(key, error);
-    }
+    return;
   }
   throw notAllowedPrivateKeyException();
 }
 
+function createPublicKey(key: string): KeyObject {
+  const pemKey = decodeBase64ToPem(key);
+
+  assertNotPrivateKey(pemKey);
+  assertNotCertificate(pemKey);
+
+  try {
+    return crypto.createPublicKey(pemKey);
+  } catch (error) {
+    throw invalidKey(key, error);
+  }
+}
+
 export function sortJWK(jwk: JsonWebKey): JsonWebKey {
   return [...Object.keys(jwk)]
     .sort()

diff --git a/packages/models/src/errors.ts b/packages/models/src/errors.ts
@@ -166,6 +166,7 @@ const errorCodes = {
   missingRequiredJWKClaim: "10002",
   invalidKey: "10003",
   tooManyRequestsError: "10004",
+  notAllowedCertificateException: "10005",
 } as const;
 
 export type CommonErrorCodes = keyof typeof errorCodes;
@@ -365,6 +366,14 @@ export function notAllowedPrivateKeyException(): ApiError<CommonErrorCodes> {
   });
 }
 
+export function notAllowedCertificateException(): ApiError<CommonErrorCodes> {
+  return new ApiError({
+    detail: `The received key is a certificate`,
+    code: "notAllowedCertificateException",
+    title: "Not allowed certificate exception",
+  });
+}
+
 export function missingRequiredJWKClaim(): ApiError<CommonErrorCodes> {
   return new ApiError({
     detail: `One or more required JWK claims are missing`,