address feedback

Author: josecorella

modules/kms-keyring-node/src/kms_hkeyring_node.ts

@@ -260,9 +260,10 @@ export class KmsHierarchicalKeyRingNode
     readOnlyProperty(this, '_cmc', cache)
 
     if (utf8Sorting === undefined) {
-      utf8Sorting = true
+      readOnlyProperty(this, '_utf8Sorting', false)
+    } else {
+      readOnlyProperty(this, '_utf8Sorting', utf8Sorting)
     }
-    readOnlyProperty(this, '_utf8Sorting', utf8Sorting)
 
     Object.freeze(this)
     /* Postcondition: The HKR object must be frozen */

modules/kms-keyring-node/test/kms_hkeyring_node.constructor.test.ts

@@ -287,7 +287,7 @@ describe('KmsHierarchicalKeyRingNode: constructor', () => {
         keyStore,
         cacheLimitTtl,
       })._utf8Sorting
-    ).to.equal(true)
+    ).to.equal(false)
   })
 
   it('utf8Sorting value is set correctly', () => {

modules/raw-aes-keyring-browser/src/raw_aes_keyring_browser.ts

@@ -18,11 +18,7 @@ import {
   importForWebCryptoDecryptionMaterial,
   AwsEsdkJsCryptoKey,
 } from '@aws-crypto/material-management-browser'
-import {
-  serializeFactory,
-  concatBuffers,
-  SerializeOptions,
-} from '@aws-crypto/serialize'
+import { serializeFactory, concatBuffers } from '@aws-crypto/serialize'
 import {
   _onEncrypt,
   _onDecrypt,
@@ -56,14 +52,15 @@ export type RawAesKeyringWebCryptoInput = {
   keyName: string
   masterKey: AwsEsdkJsCryptoKey
   wrappingSuite: WrappingSuiteIdentifier
-  utf8Sorting?: boolean | true
+  utf8Sorting?: boolean
 }
 
 export class RawAesKeyringWebCrypto extends KeyringWebCrypto {
   public declare keyNamespace: string
   public declare keyName: string
   declare _wrapKey: WrapKey<WebCryptoAlgorithmSuite>
   declare _unwrapKey: UnwrapKey<WebCryptoAlgorithmSuite>
+  public declare _utf8Sorting: boolean
 
   constructor(input: RawAesKeyringWebCryptoInput) {
     super()
@@ -83,12 +80,15 @@ export class RawAesKeyringWebCrypto extends KeyringWebCrypto {
         flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY,
       })
 
-    const maybeUtf8Sorting = utf8Sorting ?? true
-    const serializeOptions: SerializeOptions = { utf8Sorting: maybeUtf8Sorting }
-    const { serializeEncryptionContext } = serializeFactory(
-      fromUtf8,
-      serializeOptions
-    )
+    if (utf8Sorting === undefined) {
+      readOnlyProperty(this, '_utf8Sorting', false)
+    } else {
+      readOnlyProperty(this, '_utf8Sorting', utf8Sorting)
+    }
+    // default will be false
+    const { serializeEncryptionContext } = serializeFactory(fromUtf8, {
+      utf8Sorting: this._utf8Sorting,
+    })
     const _wrapKey = async (material: WebCryptoEncryptionMaterial) => {
       /* The AAD section is uInt16BE(length) + AAD
        * see: https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/message-format.html#header-aad

modules/raw-aes-keyring-node/src/raw_aes_keyring_node.ts

@@ -14,11 +14,7 @@ import {
   NodeAlgorithmSuite,
 } from '@aws-crypto/material-management-node'
 import { randomBytes, createCipheriv, createDecipheriv } from 'crypto'
-import {
-  serializeFactory,
-  concatBuffers,
-  SerializeOptions,
-} from '@aws-crypto/serialize'
+import { serializeFactory, concatBuffers } from '@aws-crypto/serialize'
 import {
   _onEncrypt,
   _onDecrypt,
@@ -78,18 +74,14 @@ export class RawAesKeyringNode extends KeyringNode {
       })
 
     if (utf8Sorting === undefined) {
-      readOnlyProperty(this, '_utf8Sorting', true)
+      readOnlyProperty(this, '_utf8Sorting', false)
     } else {
       readOnlyProperty(this, '_utf8Sorting', utf8Sorting)
     }
-    // default will be true
-    const serializeOptions: SerializeOptions = {
+    // default will be false
+    const { serializeEncryptionContext } = serializeFactory(fromUtf8, {
       utf8Sorting: this._utf8Sorting,
-    }
-    const { serializeEncryptionContext } = serializeFactory(
-      fromUtf8,
-      serializeOptions
-    )
+    })
     const _wrapKey = async (material: NodeEncryptionMaterial) => {
       /* The AAD section is uInt16BE(length) + AAD
        * see: https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/message-format.html#header-aad

modules/serialize/src/serialize_factory.ts

@@ -97,6 +97,15 @@ export function serializeFactory(
     encryptionContext: EncryptionContext
   ): Uint8Array[] {
     // use closure value from the serializeFactory
+    // If the encryption context contains high order
+    // utf8 code points the "old" implementation would sort these values
+    // based on their values, see the false branch of this function.
+    // This led to different sorting if using these high order utf8 code points,
+    // which led to decryption failures from other ESDK language implementations
+    // that correctly sorted the encryption context by sorting based on the utf8
+    // values as opposed to the string value.
+    // See, https://github.com/aws/aws-encryption-sdk-javascript/issues/428
+    // for mote details
     const { utf8Sorting } = sorting
     if (utf8Sorting) {
       return Object.entries(encryptionContext)