feat: add Always Encrypted support #637

denysvitali · 2021-02-09T16:32:41Z

This commit adds partial support for the Microsoft SQL "Always Encrypted" feature (basically, E2E encryption).

The current implementation is to be consider a preview since it might not be perfectly implemented.

Supported features:

PFX "keystore"
Seamless encryption

Missing features:

Support for Private Keys that are not RSA
Encryption support (only Decryption is possible at the moment)

The most probably needs to be improved a bit, but so far it's working for some of the use cases that I needed it for.

Feel free to test the feature and open an issue if you find any problem: my goal is to have enough testers to spot eventual bugs.

Fixes #265

This commit adds partial support for the Microsoft SQL "Always Encrypted" feature (basically, E2E encryption). The current implementation is to be consider a __preview__ since it might not be perfectly implemented. Supported features: - PFX "keystore" - Seamless encryption Missing features: - Support for Private Keys that are not RSA - Encryption support (only Decryption is possible at the moment) The most probably needs to be improved a bit, but so far it's working for some of the use cases that I needed it for. Feel free to test the feature and open an issue if you find any problem: my goal is to have enough testers to spot eventual bugs. fix: lint issues

fix(alwaysEncrypted): fix NBCRow feat(go.mod): update mssql-always-encrypted to v0.1.3

kardianos

This is unlikely to go further unless you bring the encryption into this package. I would recommend opening a new PR for that if you are interested in that.

I would also want to start with encryption parameters in the Connector.

kardianos · 2021-07-08T14:01:57Z

README.md

+
+db, err := sql.Open("sqlserver", u.String())
+if err != nil {
+    logrus.Fatalf("unable to open db: %v", err)


In this example, use return fmt.Errorf("unable to open db: %w", err) throughout.

kardianos · 2021-07-08T14:02:05Z

README.md

+}
+rows, err := db.Query("SELECT id, ssn FROM [dbo].[cid]")
+if err != nil {
+    logrus.Fatalf("unable to perform query: %v", err)


Same as above.

kardianos · 2021-07-08T14:02:36Z

README.md

+if err != nil {
+    logrus.Fatalf("unable to open db: %v", err)
+}
+rows, err := db.Query("SELECT id, ssn FROM [dbo].[cid]")


Remove [ and ].

kardianos · 2021-07-08T14:04:43Z

README.md

+    logrus.Fatalf("unable to perform query: %v", err)
+}
+
+for ; rows.Next(); {


In Go, there are three for loop variants: 0 argument (loop forever), 1 argument (test condition), and 3 argument.

You want the 1 argument version here: for rows.Next() {

kardianos · 2021-07-08T14:05:13Z

README.md

+    }
+    err = rows.Scan(&dest.Id, &dest.SSN)
+    if err != nil {
+        logrus.Fatalf("unable to scan into struct: %v", err)


Same as above.

kardianos · 2021-07-08T14:06:29Z

always_encrypted_test.go

+	defer rows.Close()
+
+	if err != nil {
+		t.Fatalf("unable to query db: %s", err)


The verb should be %w, not %s.

kardianos · 2021-07-08T14:07:18Z

always_encrypted_test.go

+import (
+	"fmt"
+	"testing"
+	"github.com/stretchr/testify/assert"


Remove. We don't need an assert package.

kardianos · 2021-07-08T14:10:05Z

cek.go

@@ -0,0 +1,29 @@
+package mssql
+
+type cekTable struct {


what is cek mean? A comment or full name would be good.

Column Encryption Key, this is used everywhere in Microsoft's documentation. Not sure if it's a good idea to make it longer here since cek is mentioned everywhere in the codebase now

Okay, then above type cekTable struct {, put

// Column Encryption Key table that ...

kardianos · 2021-07-08T14:10:19Z

cek.go

+
+type cekTableEntry struct {
+	databaseID int
+	keyId      int


Why should this be exported?

Oh nevermind, you were referring to the ID part 😅

kardianos · 2021-07-08T14:15:08Z

go.mod

 	github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe
+	github.com/stretchr/testify v1.7.0
+	github.com/swisscom/mssql-always-encrypted v0.1.3


This is a non-starter.

If you want to bring this into this repo for review, we can do that. But we won't be adding an external repo like this.

Makes sense, but I wanted to keep the two things separated to not clog too much this repo, especially since one package is feature complete (encryption /+ decryption) but here we only support encryption for now.

It would belong in an internal/ package, but not a different repo.

denysvitali · 2021-07-08T16:27:39Z

Thanks for the review! I'll try to find the time to improve the PR

lokmannicholas · 2022-09-16T17:03:00Z

any idea how to Insert or update the encrypted data?

shueybubbles · 2023-04-04T16:51:52Z

@denysvitali are you interested in re-homing this PR in our fork? https://github.com/microsoft/go-mssqldb

denysvitali · 2023-04-04T16:53:58Z

Sure!

shueybubbles · 2023-05-18T17:57:16Z

token.go

+	}
+
+	// TODO: Support other private keys
+	rootKey, err := cekv.Decrypt(s.alwaysEncryptedSettings.pKey.(*rsa.PrivateKey))


as we consider porting this over to the microsoft fork - how can we factor out the key providers into separate packages and make the core agnostic to which key provider is chosen?

IE, we want to have an app to be able to import whichever key provider package they want, and we would implement the AKV provider package and perhaps the local PFX file provider.
Then the connection string can specify the name of the key provider along with its authentication parameters. Each provider would be responsible for parsing its auth parameters from the connection string.

shueybubbles · 2023-06-09T22:36:36Z

I've started a more extensive AE implementation in the Microsoft fork and welcome feedback. We're starting with decryption using local certs or Azure Key Vault then expanding to encryption. microsoft#116

shueybubbles · 2023-06-14T15:15:18Z

token.go

+					panic(err)
+				}
+
+				pk, cert, err := pkcs12.Decode(pfxBytes, s.alwaysEncryptedSettings.ksSecret)


cs12.Decode(pfxByte

@denysvitali Hi Denys. I am experimenting with supporting the Windows certificate store to try to get certificate bytes and private key bytes to feed into your existing decryption code.
Unfortunately the encoded cert bytes referenced in a windows CERT_CONTEXT structure are not the same as the contents of the pfx file so pkcs12.Decode fails:

panic: pkcs12: error reading P12 data: asn1: structure error: tags don't match (2 vs {class:0 tag:16 length:559 isCompound:true}) {optional:false explicit:false application:false private:false defaultValue:<nil> tag:<nil> stringType:0 timeType:0 set:false omitEmpty:false} int @4 [recovered]

Do you happen to have some insight on the best way to get the raw bytes of the private key from the Windows cryptapi ?

I can get the x509Certificate thusly:

func certContextToX509(certHandle *windows.CertContext) (*x509.Certificate, error) { var der []byte slice := (*reflect.SliceHeader)(unsafe.Pointer(&der)) slice.Data = uintptr(unsafe.Pointer(certHandle.EncodedCert)) slice.Len = int(certHandle.Length) slice.Cap = int(certHandle.Length) return x509.ParseCertificate(der) }

I'm not a Windows user, but I recall Microsoft actually preventing in some cases to export the private key (I think there is a flag somewhere).

Maybe this is the case for you (or you get an encrypted version of it) and thus you cannot parse it :(

oh ncryptexportkey looks promising...

shueybubbles · 2023-06-16T15:04:53Z

@denysvitali @kardianos Does go-mssqldb have an equivalent function for converting the []namedValue slice to a comma delimited string like .Net at https://github.com/dotnet/SqlClient/blob/2b31810ce69b88d707450e2059ee8fbde63f774f/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs#L6150 ?
For encrypting parameters I need to build a query to sp_describe_parameter_encryption which has the list of named parameters and their types like this.

exec sp_describe_parameter_encryption N'DECLARE @c1 AS NCHAR (10) = @p3ee82b8b457c44498ce61053237ba60e;
DECLARE @c2 AS INT = @p643017f69684494b9143f6de7c9078b1;
INSERT  INTO Table_1
VALUES (@c1, @c2);
',N'@p3ee82b8b457c44498ce61053237ba60e nchar(10),@p643017f69684494b9143f6de7c9078b1 int'

shueybubbles · 2023-07-11T22:26:13Z

@denysvitali I've got a PR for both encryption and decryption in the Microsoft fork at microsoft#116.
It handles both pfx files and the windows certificate store, though I haven't yet written test automation for the E2E of the pfx file-based operations.
I welcome folks like you to grab that branch and try out your scenarios and to give input on the PR.

denysvitali force-pushed the feature/always-encrypted branch from fcc2ec6 to 30c8baa Compare February 9, 2021 16:44

Denys Vitali added 3 commits February 11, 2021 14:33

fix(alwaysEncrypted): padding issue

7f9a2bc

fix(alwaysEncrypted): different types problems

6414034

fix(alwaysEncrypted): fix NBCRow feat(go.mod): update mssql-always-encrypted to v0.1.3

fix(alwaysEncrypted): handle nil values

a4486a1

kardianos requested changes Jul 8, 2021

View reviewed changes

owen-transcarent mentioned this pull request Jul 16, 2022

pkcs12: unknown digest algorithm: 2.16.840.1.101.3.4.2.1 swisscom/mssql-always-encrypted#1

Open

shueybubbles mentioned this pull request Nov 15, 2022

Always Encrypted - get it working microsoft/go-mssqldb#56

Closed

shueybubbles reviewed May 18, 2023

View reviewed changes

shueybubbles reviewed Jun 14, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add Always Encrypted support #637

feat: add Always Encrypted support #637

denysvitali commented Feb 9, 2021

kardianos left a comment

kardianos Jul 8, 2021

kardianos Jul 8, 2021

kardianos Jul 8, 2021

kardianos Jul 8, 2021

kardianos Jul 8, 2021

kardianos Jul 8, 2021

kardianos Jul 8, 2021

kardianos Jul 8, 2021

denysvitali Jul 8, 2021

kardianos Jul 8, 2021

kardianos Jul 8, 2021

denysvitali Jul 8, 2021

denysvitali Jul 8, 2021

kardianos Jul 8, 2021

denysvitali Jul 8, 2021

kardianos Jul 8, 2021

denysvitali commented Jul 8, 2021

lokmannicholas commented Sep 16, 2022

shueybubbles commented Apr 4, 2023

denysvitali commented Apr 4, 2023

shueybubbles May 18, 2023

shueybubbles commented Jun 9, 2023

shueybubbles Jun 14, 2023

denysvitali Jun 14, 2023

shueybubbles Jun 14, 2023

shueybubbles commented Jun 16, 2023

shueybubbles commented Jul 11, 2023

feat: add Always Encrypted support #637

Are you sure you want to change the base?

feat: add Always Encrypted support #637

Conversation

denysvitali commented Feb 9, 2021

kardianos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denysvitali commented Jul 8, 2021

lokmannicholas commented Sep 16, 2022

shueybubbles commented Apr 4, 2023

denysvitali commented Apr 4, 2023

Choose a reason for hiding this comment

shueybubbles commented Jun 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shueybubbles commented Jun 16, 2023

shueybubbles commented Jul 11, 2023