Always Encrypted

[fredwes]

Add Always Encrypted support to tedious.js.

Done PR submission steps 1-6

[arthurschreiber]

Oh, wow. This is quite the Christmas present! 🎄 🎁

[arthurschreiber]

So I took a quick look over the changes - there is quite a lot of changes here! How long have you been working on this? 😲

Anyway, it'll take me a bit of time to have this reviewed and give feedback for this, but the changes are looking pretty solid from what I've seen so far. 👍

[fredwes]

We've worked on it for a few months but only recently got approval to submit a PR. Would love to get your feedback! 😄

[IanChokS]

Hi @fredwes,

Thanks for this PR! Just wondering if this PR also allows for keys to be stored in Windows Certificate Store?

[fredwes]

@IanChokS The only key store supported by this PR is Azure Key Vault. Additional key stores can be added by implementing a decryptColumnEncryptionKey method

[codecov[bot]]

Codecov Report

Merging #1020 into master will decrease coverage by 23.27%. The diff coverage is 18.14%.

@@             Coverage Diff             @@
##           master    #1020       +/-   ##
===========================================
- Coverage   80.36%   57.08%   -23.28%     
===========================================
  Files          86       96       +10     
  Lines        4405     5367      +962     
  Branches      778      968      +190     
===========================================
- Hits         3540     3064      -476     
- Misses        612     1942     +1330     
- Partials      253      361      +108

Impacted Files	Coverage Δ
src/tedious.ts	100% <ø> (ø)	:arrow_up:
src/data-type.ts	100% <ø> (ø)	:arrow_up:
src/always-encrypted/cek-entry.ts	0% <0%> (ø)
src/data-types/datetime.ts	50% <0%> (-50%)	:arrow_down:
src/data-types/tinyint.ts	58.06% <0%> (-33.61%)	:arrow_down:
src/data-types/money.ts	46.42% <0%> (-53.58%)	:arrow_down:
src/data-types/nvarchar.ts	69.64% <0%> (-22.67%)	:arrow_down:
src/data-types/smallint.ts	58.06% <0%> (-25.27%)	:arrow_down:
src/data-types/floatn.ts	25% <0%> (-3.58%)	:arrow_down:
src/data-types/datetime2.ts	55.22% <0%> (-30.5%)	:arrow_down:
... and 101 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e94c4d0...66bd734. Read the comment docs.

[arthurschreiber]

@fredwes Thanks for all the work on this so far! I added some comments around some of the code that I reviewed.

[fredwes]

Thanks @arthurschreiber! I fixed the issues you raised so far and fixed the rest of the failing tests. I'm going to continue to add additional test cases as you complete your review

[arthurschreiber]

@fredwes We're currently in the process of changing how requests and their payloads are generated - see https://github.com/tediousjs/tedious/issues/1038.

I believe those changes will nicely play into some of the changes you've done for the encryption support (e.g. adding toBuffer). That will require some updates to the changes you've done here so far, but also should reduce a whole bunch of duplication.

What would be absolutely awesome is if we could break out some of the changes made in here in support for Always Encrypted but which are not really related to encryption support. What I'm mainly thinking about is the collation support for parameters. Extracting that into a separate PR would allow me to review and merge this separately from the larger PR here, while also making both PRs easier to review in the end. If you don't have time for doing this yourself, would you be ok if I do the extraction?

What do you think? 🙇

[fredwes]

@arthurschreiber That's a good idea, I think there are some other things that can be split out into separate PRs as well (and cleaned up). I can do it or if you prefer to do it, that's fine too. I can get started in the next couple days.

[arthurschreiber]

@fredwes If I understand correctly, Always encryoption does not support PLP streamed data (e.g. for nvarchar(max) or varbinary(max)).

Does this mean encrypted data is limited to 4000 characters / 8000 bytes? 🤔

[fredwes]

@arthurschreiber Always Encrypted does support PLP, but for the purpose of toBuffer we don't use PLP terminators, the entire parameter value is used during encryption/decryption

[arthurschreiber]

@fredwes how would a PLP implementation look like? I’m wondering because the authentication tag seems to be stored at the front of the encrypted data, but can only be generated after reading all data, making PLP streaming pretty useless, I’d think. 🤔

[fredwes]

@arthurschreiber There is no implementation necessary from an Always Encrypted perspective. It should be supported implicitly using existing parameter writing and reading logic. You are right, PLP streaming for Always Encrypted is essentially just a compatibility thing, since you need the entire value to do encrypt/decrypt.

[Sathishabm210]

@fredwes Here is my code to retrieve the always encrypted column data using tedious-master, still I am getting encrypted data only. Will this support to get decrypted data from the always encrypted column table? Below is my code:

var Connection = require('tedious-master').Connection;
var ColumnEncryptionAzureKeyVaultProvider = require('tediousmaster').ColumnEncryptionAzureKeyVaultProvider;
var Request = require('tedious-master').Request;

var config = {  
    server: '',  //update me
    authentication: {
        type: 'default',
        options: {
            userName: '', //update me
            password: ''  //update me
        }
    },
    options: {
        // If you are on Microsoft Azure, you need encryption:
        encrypt: true,
        database: ''  //update me
    }
};

var akvProvider = new ColumnEncryptionAzureKeyVaultProvider("", "", "");
config.options.encryptionKeyStoreProviders = [{
  key: akvProvider.name,
  value: akvProvider
}]

var connection = new Connection(config);
connection.on('connect', function(err) {  
    // If no error, then good to proceed.  
    console.log("Connected");  
    executeStatement();  
});  

function executeStatement() {  
    request = new Request("select * from alwaysEncryptColumnTable ", function(err) {  
    if (err) {  
        console.log(err);}  
    });  
    var result = "";  
    request.on('row', function(columns) {  
        columns.forEach(function(column) {  
          if (column.value === null) {  
            console.log('NULL');  
          } else {  
            result+= column.value + " ";  
          }  
        });  
        console.log(result);  
        result ="";  
    });  
    request.on('done', function(rowCount, more) {  
    console.log(rowCount + ' rows returned');  
    });  
    connection.execSql(request);  
}  

[fredwes]

@Sathishabm210 are you using tedious master branch or this PR branch? Always Encrypted feature isn’t merged yet to master.

If you are using this PR branch then I see that you are missing the Always Encrypted connection option. Add this property to your connection options: columnEncryptionSetting: true

[jocund]

@fredwes Just tried always encrypted based connection using AKV referencing the example you provided. Was able to push/pull non-encrypted columns, but for encrypted one it says the following for me (node:16044) UnhandledPromiseRejectionWarning: Error: Cannot use a non-RSA key: RSA-HSM.

Does this imply that we cannot set a key vault with RSA-HSM if we need to use your existing code? Any thing we can do? Please let me know

[Sathishabm210]

@Sathishabm210 are you using tedious master branch or this PR branch? Always Encrypted feature isn’t merged yet to master.

If you are using this PR branch then I see that you are missing the Always Encrypted connection option. Add this property to your connection options: columnEncryptionSetting: true

@fredwes I am using this PR branch only. After setting this option its throwing the below error

Error: Cannot use a non-RSA key: RSA-HSM.
    at ColumnEncryptionAzureKeyVaultProvider.getAKVKeySize (C:\projects\azureKeyvault.js\node_modules\tedious-master\lib\always-encrypted\keystore-provider-azure-key-vault.js:296:13)
    at C:\projects\azureKeyvault.js\node_modules\tedious-master\lib\always-encrypted\keystore-provider-azure-key-vault.js:51:36
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (C:\projects\azureKeyvault.js\node_modules\tedious-master\lib\always-encrypted\keystore-provider-azure-key-vault.js:16:103)
    at _next (C:\projects\azureKeyvault.js\node_modules\tedious-master\lib\always-encrypted\keystore-provider-azure-key-vault.js:18:194)
    at processTicksAndRejections (internal/process/task_queues.js:94:5)
(node:5300) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch
block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:5300) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

[fredwes]

@Sathishabm210 @jocund Looks like there is a bug with my code. RSA-HSM should be allowed. This PR has gotten out of date with the master branch. I will make sure to apply a fix for this in one of my upcoming PRs.

[jocund]

@fredwes Thanks for confirming. When can we expect a PR update to fix this? Also, in the mean time can you suggest any work around that we could do on this PR to help move forward?

[jocund]

@fredwes Commented this line in your lib folder file 'keystore-provider-azure-key-vault' that checks 'RSA' to allow 'RSA-HSM' based keys. It worked to help execute successful pull queries

// if (!kty || 'RSA'.localeCompare(kty, 'en') !== 0) { // throw new Error(Cannot use a non-RSA key: ${kty}.);

However, when inserting data into encrypted column, I've landed with a 'collation_name' issue as below. Do you know what we can do to resolve it?

'Operand type clash: char(6) encrypted with (encryption_type = 'DETERMINISTIC', encryption_algorithm_name = 'AEAD_AES_256_CBC_HMAC_SHA_256', column_encryption_key_name = 'Our_key_Name', column_encryption_key_database_name = 'Our_DB_Name') collation_name = ''

is incompatible with char(6) encrypted with ( encryption_type = 'DETERMINISTIC', encryption_algorithm_name = 'AEAD_AES_256_CBC_HMAC_SHA_256', column_encryption_key_name = 'Our_key_Name', column_encryption_key_database_name = 'Our_DB_Name') collation_name = 'SQL_Latin1_General_CP1_CI_AS'',

[fredwes]

@fredwes Commented this line in your lib folder file 'keystore-provider-azure-key-vault' that checks 'RSA' to allow 'RSA-HSM' based keys. It worked to help execute successful pull queries

// if (!kty || 'RSA'.localeCompare(kty, 'en') !== 0) { // throw new Error(Cannot use a non-RSA key: ${kty}.);

However, when inserting data into encrypted column, I've landed with a 'collation_name' issue as below. Do you know what we can do to resolve it?

'Operand type clash: char(6) encrypted with (encryption_type = 'DETERMINISTIC', encryption_algorithm_name = 'AEAD_AES_256_CBC_HMAC_SHA_256', column_encryption_key_name = 'Our_key_Name', column_encryption_key_database_name = 'Our_DB_Name') collation_name = ''

is incompatible with char(6) encrypted with ( encryption_type = 'DETERMINISTIC', encryption_algorithm_name = 'AEAD_AES_256_CBC_HMAC_SHA_256', column_encryption_key_name = 'Our_key_Name', column_encryption_key_database_name = 'Our_DB_Name') collation_name = 'SQL_Latin1_General_CP1_CI_AS'',

@jocund That change should fix your issue with RSA-HSM. With regard to the error you are getting, that is an error from SQL server itself. You must use a BIN2 collation with deterministic encryption (see here: https://docs.microsoft.com/en-us/sql/relational-databases/security/encryption/always-encrypted-database-engine?view=sql-server-ver15#selecting--deterministic-or-randomized-encryption).

[jocund]

@fredwes Thanks. We are using BIN2 with deterministic, but we noticed that the operand type clash error is occurring only for "Char" and "VarChar" data types on SQL. When we change these to "NChar" and "NVarChar" on SQL columns and use those types on our code (using tedious), our insert is successful. Not sure why these specific data types would cause an issue. I did notice that your code has "Collation" logic in Char.js and not in NChar.js - Do you think that's playing a role?

char.js writeTypeInfo: function writeTypeInfo(buffer, parameter) { buffer.writeUInt8(this.id); buffer.writeUInt16LE(parameter.length); const collation = Buffer.alloc(5);

if (parameter.collation != null) {
  const _parameter$collation = parameter.collation,
        lcid = _parameter$collation.lcid,
        flags = _parameter$collation.flags,
        version = _parameter$collation.version,
        sortId = _parameter$collation.sortId;
  collation.writeUInt8(lcid & 0xFF, 0);
  collation.writeUInt8(lcid >> 8 & 0xFF, 1); // byte index 2 contains data for both lcid and flags

  collation.writeUInt8(lcid >> 16 & 0x0F | (flags & 0x0F) << 4, 2); // byte index 3 contains data for both flags and version

  collation.writeUInt8(flags & 0xF0 | version & 0x0F, 3);
  collation.writeUInt8(sortId & 0xFF, 4);
}

buffer.writeBuffer(collation);

},

nchar.js

writeTypeInfo: function writeTypeInfo(buffer, parameter) { buffer.writeUInt8(this.id); buffer.writeUInt16LE(parameter.length * 2); buffer.writeBuffer(Buffer.from([0x00, 0x00, 0x00, 0x00, 0x00])); },

[fredwes]

@jocund I think the issue is that you will need to specify the collation in the parameter options.

request.addParameter('name', TYPES.Char, 'value', { collation: { sortId: 52, lcid: 0, flags: 0, version: 0 } });

[jocund]

@fredwes When we try to pull more than 8-10 Azure Always encrypted records from DB using your code, we're seeing that there is some issue with "unknown type error". See logs below. Would you know how to fix it? Note, when we only pull less that 8 records, the pull is successful

(node:10452) UnhandledPromiseRejectionWarning: Error: Unknown type: 2 at C:\Users\Azure_Server\tedious-master\lib\token\stream-parser.js:181:35 at C:\Users\Azure_Server\tedious-master\lib\tracking-buffer\readable-tracking-buffer.js:67:7 at ReadableTrackingBuffer.awaitData (C:\Users\Azure_Server\tedious-master\lib\tracking-buffer\readable-tracking-buffer.js:49:7) at ReadableTrackingBuffer.readUInt8 (C:\Users\Azure_Server\tedious-master\lib\tracking-buffer\readable-tracking-buffer.js:64:10) at C:\Users\Azure_Server\tedious-master\lib\token\stream-parser.js:221:49 at Parser.awaitData (C:\Users\Azure_Server\tedious-master\lib\token\stream-parser.js:208:7) at Parser.readUInt8 (C:\Users\Azure_Server\tedious-master\lib\token\stream-parser.js:221:10) at C:\Users\Azure_Server\tedious-master\lib\token\stream-parser.js:174:17 at new Promise () at C:\Users\Azure_Server\tedious-master\lib\token\stream-parser.js:173:15

(node:10452) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag --unhandled-rejections=strict (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)

[jocund]

@fredwes Grateful for your urgent assistance for above question. Can you please advice? I'm running the code on Azure app service linux env

[akshayganeshen]

@jocund Can you share the schema and/or typical records that can reproduce the issue?

[jocund]

@akshayganeshen @fredwes Below is high level example table that is resulting in unknown type error. Would you know the cause?

CREATE TABLE [db_dbo].[customer]( [id] [uniqueidentifier] NOT NULL, [addresscity] varchar NULL, [addressline1] nvarchar COLLATE Latin1_General_BIN2 ENCRYPTED WITH (COLUMN_ENCRYPTION_KEY = [<Your_Key>], ENCRYPTION_TYPE = Deterministic, ALGORITHM = 'AEAD_AES_256_CBC_HMAC_SHA_256') NULL, [addressstate] varchar NULL, [addresszip] varchar NULL, [First_name] nvarchar COLLATE Latin1_General_BIN2 ENCRYPTED WITH (COLUMN_ENCRYPTION_KEY = [<Your_Key>], ENCRYPTION_TYPE = Deterministic, ALGORITHM = 'AEAD_AES_256_CBC_HMAC_SHA_256') NULL, [language] varchar NULL, [Last_name] nvarchar COLLATE Latin1_General_BIN2 ENCRYPTED WITH (COLUMN_ENCRYPTION_KEY = [<Your_Key>], ENCRYPTION_TYPE = Deterministic, ALGORITHM = 'AEAD_AES_256_CBC_HMAC_SHA_256') NULL, [notes] varchar NULL, [phonenumber] nvarchar COLLATE Latin1_General_BIN2 ENCRYPTED WITH (COLUMN_ENCRYPTION_KEY = [<Your_Key>], ENCRYPTION_TYPE = Deterministic, ALGORITHM = 'AEAD_AES_256_CBC_HMAC_SHA_256') NULL, [companyname] nvarchar COLLATE Latin1_General_BIN2 ENCRYPTED WITH (COLUMN_ENCRYPTION_KEY = [<Your_Key>], ENCRYPTION_TYPE = Deterministic, ALGORITHM = 'AEAD_AES_256_CBC_HMAC_SHA_256') NULL, [Submitted_by] nvarchar COLLATE Latin1_General_BIN2 ENCRYPTED WITH (COLUMN_ENCRYPTION_KEY = [<Your_Key>], ENCRYPTION_TYPE = Deterministic, ALGORITHM = 'AEAD_AES_256_CBC_HMAC_SHA_256') NULL, [createdon] [uniqueidentifier] NOT NULL, )

[jocund]

@fredwes @akshayganeshen Is this a defect? Please let me know

[jonmifsud]

@fredwes this looks like some pretty solid work.

We're helping out a client who has their app primarily in .net and we've done some work in Node. I'm trying to get this set up with Azure Key Vault and I think I'm missing some pieces.

I've managed to get the standard example encryption work just fine. However, I'm not sure what I'm missing to use the ColumnEncryptionAzureKeyVaultProvider, particularly when creating the table with fields.

Would you still create the Master and Column encryption keys within the SQL query? I'm guessing at the very least the master key is obtained from the KeyVault however reading through the code I couldn't see which part/function I need to run to do this cleanly.

Or is there a separate create table syntax when using Azure Key Vault