From f48e5715188f9071ec0e2c6a8b3d57eaffcda769 Mon Sep 17 00:00:00 2001 From: Leo Anderson Date: Fri, 12 Jan 2024 14:35:12 +0000 Subject: [PATCH] AS-92: fix issues that prevent turbostream from working This commit ports over two fixes that seemingly have lost all upstream traction: 1. Add support for RSA-PSS signature verification 2. Add an `unsafe.Pointer` cast to allow compilation on Go > 1.17 These code changes are both mostly ported, there is little "original" code. https://github.com/github/certstore/pull/20/files#diff-e46a14c55fd3c2c2f7ade1a63b9f14f591829e183d307f7d2ee54cb7cc4c4073R395 https://github.com/github/smimesign/pull/114 --- certstore/certstore_windows.go | 95 ++++++++++++++++++++++++++++------ go.mod | 8 ++- 2 files changed, 86 insertions(+), 17 deletions(-) diff --git a/certstore/certstore_windows.go b/certstore/certstore_windows.go index 86c0b1c..7d59e3e 100644 --- a/certstore/certstore_windows.go +++ b/certstore/certstore_windows.go @@ -64,10 +64,11 @@ const ( // API will be used. // // Possible values are: -// 0x00000000 — — Only use CryptoAPI. -// 0x00010000 — CRYPT_ACQUIRE_ALLOW_NCRYPT_KEY_FLAG — Prefer CryptoAPI. -// 0x00020000 — CRYPT_ACQUIRE_PREFER_NCRYPT_KEY_FLAG — Prefer CNG. -// 0x00040000 — CRYPT_ACQUIRE_ONLY_NCRYPT_KEY_FLAG — Only uyse CNG. +// +// 0x00000000 — — Only use CryptoAPI. +// 0x00010000 — CRYPT_ACQUIRE_ALLOW_NCRYPT_KEY_FLAG — Prefer CryptoAPI. +// 0x00020000 — CRYPT_ACQUIRE_PREFER_NCRYPT_KEY_FLAG — Prefer CNG. +// 0x00040000 — CRYPT_ACQUIRE_ONLY_NCRYPT_KEY_FLAG — Only uyse CNG. var winAPIFlag C.DWORD = C.CRYPT_ACQUIRE_PREFER_NCRYPT_KEY_FLAG // winStore is a wrapper around a C.HCERTSTORE. @@ -361,14 +362,40 @@ func (wpk *winPrivateKey) Sign(rand io.Reader, digest []byte, opts crypto.Signer if wpk.capiProv != 0 { return wpk.capiSignHash(opts.HashFunc(), digest) } else if wpk.cngHandle != 0 { - return wpk.cngSignHash(opts.HashFunc(), digest) + return wpk.cngSignHash(opts, digest) } else { return nil, errors.New("bad private key") } } // cngSignHash signs a digest using the CNG APIs. -func (wpk *winPrivateKey) cngSignHash(hash crypto.Hash, digest []byte) ([]byte, error) { +func (wpk *winPrivateKey) cngSignHash(opts crypto.SignerOpts, digest []byte) ([]byte, error) { + + /* + Cydar upstream change: add support for RSA-PSS signature scheme. + Ref: AS-92. + + A garbage SaltLength value is given when using RSA-PSS and results in + an Nginx error during Client cert exchange: + + ``` + *835 SSL_do_handshake() failed (SSL: error:0407E086:rsa + routines:RSA_verify_PKCS1_PSS_mgf1:last octet invalid + error:1417B07B:SSL routines:tls_process_cert_verify:bad signature) + while SSL handshaking. + ``` + + This is because the `pPaddingInfo` parameter for `NCryptSignHash` would + previously always be given a `BCRYPT_PKCS1_PADDING_INFO` struct, which, + when the pointer to it is transferred to C-land and interpreted, would + have an invalid `cbSalt` value, since that's for PKCS1 and not PSS. + + This code changes is mostly ported from an existing PR that was opened + to the original (now archived) `certstore` repo, but never merged: + https://github.com/github/certstore/pull/20/files + */ + + hash := opts.HashFunc() if len(digest) != hash.Size() { return nil, errors.New("bad digest for hash") } @@ -384,24 +411,53 @@ func (wpk *winPrivateKey) cngSignHash(hash crypto.Hash, digest []byte) ([]byte, sigLen = C.DWORD(0) ) - // setup pkcs1v1.5 padding for RSA + // setup pkcs1v1.5/pss padding for RSA if _, isRSA := wpk.publicKey.(*rsa.PublicKey); isRSA { - flags |= C.BCRYPT_PAD_PKCS1 - padInfo := C.BCRYPT_PKCS1_PADDING_INFO{} - padPtr = unsafe.Pointer(&padInfo) + var pszAlgId C.LPCWSTR + // get the current hash algorithm identifier: switch hash { case crypto.SHA1: - padInfo.pszAlgId = BCRYPT_SHA1_ALGORITHM + pszAlgId = BCRYPT_SHA1_ALGORITHM case crypto.SHA256: - padInfo.pszAlgId = BCRYPT_SHA256_ALGORITHM + pszAlgId = BCRYPT_SHA256_ALGORITHM case crypto.SHA384: - padInfo.pszAlgId = BCRYPT_SHA384_ALGORITHM + pszAlgId = BCRYPT_SHA384_ALGORITHM case crypto.SHA512: - padInfo.pszAlgId = BCRYPT_SHA512_ALGORITHM + pszAlgId = BCRYPT_SHA512_ALGORITHM default: return nil, ErrUnsupportedHash } + + // is this using PSS? + if pssOpts, ok := opts.(*rsa.PSSOptions); ok { + // start logic to create a valid BCRYPT_PSS_PADDING_INFO block: + + // first need to figure out what goes into the cbSalt field: + // by default, use the SaltLength provided by the `PSSOptions` + saltLength := pssOpts.SaltLength + switch saltLength { + case rsa.PSSSaltLengthAuto: + return nil, ErrUnsupportedHash + case rsa.PSSSaltLengthEqualsHash: + // in this particular case, take the length of the hash itself: + saltLength = hash.Size() + } + // set the PSS flag to true + flags = C.BCRYPT_PAD_PSS + // point `padPtr` to a PSS structure + padPtr = unsafe.Pointer(&C.BCRYPT_PSS_PADDING_INFO{ + pszAlgId: pszAlgId, + cbSalt: C.ulong(saltLength), + }) + } else { + // otherwise, not using PSS, so configure as PKCS1: + flags = C.BCRYPT_PAD_PKCS1 + // point `padPtr` to a PKCS1 structure + padPtr = unsafe.Pointer(&C.BCRYPT_PKCS1_PADDING_INFO{ + pszAlgId: pszAlgId, + }) + } } // get signature length @@ -637,7 +693,16 @@ func (c errCode) Error() string { if cmsg == nil { return fmt.Sprintf("Error %X", int(c)) } - defer C.LocalFree(C.HLOCAL(cmsg)) + /* + Cydar upstream change: fix compilation on recent Go compiler versions. + + Wrap `cmsg` in an `unsafe.Pointer` before converting to a `C.HLOCAL`: + + This fixes an open issue in the (newer) `certstore` repo, that had an + open pull request, but seems to have lost all traction: + https://github.com/github/smimesign/pull/114 + */ + defer C.LocalFree(C.HLOCAL(unsafe.Pointer(cmsg))) gomsg := C.GoString(cmsg) diff --git a/go.mod b/go.mod index ba48589..ab467bc 100644 --- a/go.mod +++ b/go.mod @@ -1,13 +1,17 @@ module github.com/github/smimesign -go 1.12 +go 1.17 require ( github.com/certifi/gocertifi v0.0.0-20180118203423-deb3ae2ef261 - github.com/davecgh/go-spew v1.1.1 // indirect github.com/pborman/getopt v0.0.0-20180811024354-2b5b3bfb099b github.com/pkg/errors v0.8.1 github.com/stretchr/testify v1.3.0 golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 ) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect +)