Merge remote-tracking branch 'origin/w/8.1/improvement/ARSN-503-kms-err' into w/8.2/improvement/ARSN-503-kms-err

BourgoisMickael · BourgoisMickael · commit 5390e5a6a77c · 2025-06-25T01:14:36.000+02:00
diff --git a/lib/errors/arsenalErrors.ts b/lib/errors/arsenalErrors.ts
@@ -1,3 +1,5 @@
+import { allowedKmsErrors } from './kmsErrors';
+
 export type ErrorFormat = {
     code: number,
     description: string,
@@ -1072,13 +1074,13 @@ export const QuotaExceeded: ErrorFormat = {
 };
 
 // -------------------- KMS --------------------
-// Most other KMS Exception can't be converted to from KMIP
-const NotFoundException: ErrorFormat = {
-    code: 400, // Not 404 because it's the KMS (Encrypt/Decrypt) that fails, not the object API
-    description: 'The request was rejected because the specified entity or resource could not be found.'
-};
+type Entries<T> = {
+    [K in keyof T]: [K, T[K]];
+}[keyof T];
 
 /** need typescript@5.6 for string literal export
 https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-6.html#support-for-arbitrary-module-identifiers
 */
-module.exports['KMS.NotFoundException'] = NotFoundException;
+for (const [kmsErr, err] of Object.entries(allowedKmsErrors) as Entries<typeof allowedKmsErrors>[]) {
+    module.exports[`KMS.${kmsErr}`] = err;
+}
diff --git a/lib/errors/kmsErrors.ts b/lib/errors/kmsErrors.ts
@@ -0,0 +1,90 @@
+/**
+ * List of AWS KMS errors that are specific to KMS and where the error
+ * can be returned to the user as is with a specific status code.
+ *
+ * Avoid any error that might leak sensitive information such as hostnames
+ * or IP addresses from network related errors.
+ * KeyId is not considered sensitive.
+ * 
+ * Reference: https://docs.aws.amazon.com/kms/latest/APIReference/CommonErrors.html
+ * See specific actions like CreateKey, Encrypt, Decrypt, etc. for more details.
+ * 
+ * Other errors not listed will return only the same error code but HTTP status 500,
+ * and message will be generic and details will be in logs only.
+ */
+export const allowedKmsErrors = {
+    AccessDeniedException: {
+        code: 400,
+        description: 'You do not have sufficient access to perform this action.',
+    },
+    AlreadyExistsException: {
+        code: 400,
+        description: 'The request was rejected because it attempted to create a resource that already exists.',
+    },
+    DisabledException: {
+        code: 400,
+        description: 'The request was rejected because the specified KMS key is not enabled.',
+    },
+    IncorrectKeyException: {
+        code: 400,
+        description: 'The request was rejected because the specified KMS key cannot decrypt the data',
+    },
+    InvalidAliasNameException: {
+        code: 400,
+        description: 'The request was rejected because the specified alias name is not valid.',
+    },
+    InvalidArnException: {
+        code: 400,
+        description: 'The request was rejected because a specified ARN, or an ARN in a key policy, is not valid.',
+    },
+    InvalidCiphertextException: {
+        code: 400,
+        description: 'The request was rejected because the specified ciphertext, or additional authenticated data, ' +
+            'is corrupted, missing, or otherwise invalid.',
+    },
+    InvalidGrantTokenException: {
+        code: 400,
+        description: 'The request was rejected because the specified grant token is not valid.',
+    },
+    InvalidKeyUsageException: {
+        code: 400,
+        description: 'The KeyUsage or algorithm  is incompatible with the API operation.',
+    },
+    KMSInternalException: {
+        code: 500,
+        description: 'The request was rejected because an internal exception occurred. The request can be retried.',
+    },
+    KMSInvalidStateException: {
+        code: 400,
+        description:
+            'The request was rejected because the state of the specified resource is not valid for this request.',
+    },
+    KeyUnavailableException: {
+        code: 500,
+        description: 'The request was rejected because the specified KMS key was not available. ' +
+            'You can retry the request.',
+    },
+    LimitExceededException: {
+        code: 400,
+        description: 'The request was rejected because a length constraint or quota was exceeded.',
+    },
+    MalformedPolicyDocumentException: {
+        code: 400,
+        description:
+            'The request was rejected because the specified policy is not syntactically or semantically correct.',
+    },
+    /** Not 404 because it's the KMS (Encrypt/Decrypt) that fails, not the object API */
+    NotFoundException: {
+        code: 400,
+        description: 'The request was rejected because the specified entity or resource could not be found.',
+    },
+    TagException: {
+        code: 400,
+        description: 'The request was rejected because one or more tags are not valid.',
+    },
+    UnsupportedOperationException: {
+        code: 400,
+        description: 'The request was rejected because a specified parameter is not supported or a specified ' +
+            'resource is not valid for this operation.',
+    },
+} as const;
diff --git a/lib/network/utils.ts b/lib/network/utils.ts
@@ -1,4 +1,6 @@
-import { errorInstances } from '../errors';
+import type { AWSError } from 'aws-sdk';
+import { ArsenalError, errorInstances } from '../errors';
+import { allowedKmsErrors } from '../errors/kmsErrors';
 
 /**
  * Normalize errors according to arsenal definitions with a custom prefix
@@ -28,6 +30,27 @@ export function arsenalErrorKMIP(err: string | Error) {
     return _normalizeArsenalError(err, 'KMIP:');
 }
 
-export function arsenalErrorAWSKMS(err: string | Error) {
+const allowedKmsErrorCodes = Object.keys(allowedKmsErrors) as unknown as (keyof typeof allowedKmsErrors)[];
+
+function isAWSError(err: string | Error | AWSError): err is AWSError {
+    return (err as AWSError).code !== undefined
+        && (err as AWSError).retryable !== undefined;
+}
+
+export function arsenalErrorAWSKMS(err: string | Error | AWSError) {
+    if (isAWSError(err)) {
+        if (allowedKmsErrorCodes.includes(err.code as keyof typeof allowedKmsErrors)) {
+            return errorInstances[`KMS.${err.code}`].customizeDescription(err.message);
+        } else {
+            // Encapsulate into a generic ArsenalError but keep the aws error code
+            return ArsenalError.unflatten({
+                is_arsenal_error: true,
+                type: `KMS.${err.code}`, // aws s3 prefix kms errors with KMS.
+                code: 500,
+                description: `unexpected AWS_KMS error`,
+                stack: err.stack,
+            });
+        }
+    }
     return _normalizeArsenalError(err, 'AWS_KMS:');
 }
diff --git a/lib/storage/data/DataWrapper.js b/lib/storage/data/DataWrapper.js
@@ -113,15 +113,10 @@ class DataWrapper {
                     stream.pipe(objectGetInfo.decipherStream);
                     return cb(null, objectGetInfo.decipherStream);
                 }
-                // this should not happen anymore, could be deleted
+                // There can still be unprepared decipherBundle
+                // for objectCopy and objectPutCopyPart
+                // Errors are caught and returned by the copy flow
                 if (objectGetInfo.cipheredDataKey) {
-                    process.emitWarning(
-                        'DataWrapper.get should not use kms.createDecipherBundle',
-                        {
-                            code: 'S3C-9891',
-                            detail: 'Use a version of cloudserver that generate kms decipher stream',
-                        },
-                    );
                     const serverSideEncryption = {
                         cryptoScheme: objectGetInfo.cryptoScheme,
                         masterKeyId: objectGetInfo.masterKeyId,
@@ -809,7 +804,7 @@ class DataWrapper {
                 (err, cipherBundle) => {
                     if (err) {
                         log.debug('error getting cipherBundle');
-                        return cb(errors.InternalError);
+                        return cb(err);
                     }
                     return this._putForCopy(cipherBundle, stream, part,
                         dataStoreContext, destBackendInfo, log, cb);
@@ -882,7 +877,7 @@ class DataWrapper {
                 (err, cipherBundle) => {
                     if (err) {
                         log.debug('error getting cipherBundle', { error: err });
-                        return cb(errors.InternalError);
+                        return cb(err);
                     }
                     return this.put(cipherBundle, stream, numberPartSize,
                         dataStoreContext, destBackendInfo, log,
diff --git a/tests/unit/errors.spec.ts b/tests/unit/errors.spec.ts
@@ -96,6 +96,16 @@ describe('Errors: ', () => {
         // don't use errorInstances if you need stacktrace
         expect(first.stack).toEqual(second.stack);
     });
+
+    it('should have KMS.NotFoundException', () => {
+        const err = errorInstances['KMS.NotFoundException'];
+        expect(err).toBeInstanceOf(ArsenalError);
+        expect(err.is['KMS.NotFoundException']).toBeTruthy();
+        expect(err.type).toEqual('KMS.NotFoundException');
+        expect(err.code).toEqual(400);
+        expect(err.description).toEqual(
+            'The request was rejected because the specified entity or resource could not be found.');
+    });
 });
 
 describe('Backward compatibility flag', () => {
