Skip to content

Commit 505937f

Browse files
michelle-clayton-workMichelle Clayton
michelle-clayton-work
and
Michelle Clayton
authored
Revert "Merge pull request #142 from microsoft/dev/mclayton/update-addCleanup" (#146)
This reverts commit b3072ea, reversing changes made to 7f762f5. Co-authored-by: Michelle Clayton <mclayton@microsoft.com>
1 parent c6c5c73 commit 505937f

File tree

12 files changed

+90
-140
lines changed

12 files changed

+90
-140
lines changed

‎cng/aes.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,14 @@ func NewAESCipher(key []byte) (cipher.Block, error) {
3030
return nil, err
3131
}
3232
c := &aesCipher{kh: kh, key: bytes.Clone(key)}
33-
addCleanupKey(c, kh)
33+
runtime.SetFinalizer(c, (*aesCipher).finalize)
3434
return c, nil
3535
}
3636

37+
func (c *aesCipher) finalize() {
38+
bcrypt.DestroyKey(c.kh)
39+
}
40+
3741
func (c *aesCipher) BlockSize() int { return aesBlockSize }
3842

3943
// validateAndClipInputs checks that dst and src meet the [cipher.Block]
@@ -161,11 +165,15 @@ func newCBC(encrypt bool, alg string, key, iv []byte) *cbcCipher {
161165
panic(err)
162166
}
163167
x := &cbcCipher{kh: kh, encrypt: encrypt, blockSize: blockSize}
164-
addCleanupKey(x, kh)
168+
runtime.SetFinalizer(x, (*cbcCipher).finalize)
165169
x.SetIV(iv)
166170
return x
167171
}
168172

173+
func (x *cbcCipher) finalize() {
174+
bcrypt.DestroyKey(x.kh)
175+
}
176+
169177
func (x *cbcCipher) BlockSize() int { return x.blockSize }
170178

171179
func (x *cbcCipher) CryptBlocks(dst, src []byte) {
@@ -239,13 +247,17 @@ type aesGCM struct {
239247
maskInitialized bool
240248
}
241249

250+
func (g *aesGCM) finalize() {
251+
bcrypt.DestroyKey(g.kh)
252+
}
253+
242254
func newGCM(key []byte, tls cipherGCMTLS) (*aesGCM, error) {
243255
kh, err := newCipherHandle(bcrypt.AES_ALGORITHM, bcrypt.CHAIN_MODE_GCM, key)
244256
if err != nil {
245257
return nil, err
246258
}
247259
g := &aesGCM{kh: kh, tls: tls}
248-
addCleanupKey(g, kh)
260+
runtime.SetFinalizer(g, (*aesGCM).finalize)
249261
return g, nil
250262
}
251263

‎cng/chacha20poly1305.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,16 @@ func NewChaCha20Poly1305(key []byte) (cipher.AEAD, error) {
4040
return nil, err
4141
}
4242
c := &chacha20poly1305{kh: kh}
43-
addCleanupKey(c, kh)
43+
runtime.SetFinalizer(c, (*chacha20poly1305).finalize)
4444
return c, nil
4545
}
4646

47+
func (c *chacha20poly1305) finalize() {
48+
if c.kh != 0 {
49+
bcrypt.DestroyKey(c.kh)
50+
}
51+
}
52+
4753
func (c *chacha20poly1305) NonceSize() int {
4854
return chacha20Poly1305NonceSize
4955
}

‎cng/des.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func NewDESCipher(key []byte) (cipher.Block, error) {
2929
return nil, err
3030
}
3131
c := &desCipher{kh: kh, alg: bcrypt.DES_ALGORITHM, key: bytes.Clone(key)}
32-
addCleanupKey(c, kh)
32+
runtime.SetFinalizer(c, (*desCipher).finalize)
3333
return c, nil
3434
}
3535

@@ -39,10 +39,14 @@ func NewTripleDESCipher(key []byte) (cipher.Block, error) {
3939
return nil, err
4040
}
4141
c := &desCipher{kh: kh, alg: bcrypt.DES3_ALGORITHM, key: bytes.Clone(key)}
42-
addCleanupKey(c, kh)
42+
runtime.SetFinalizer(c, (*desCipher).finalize)
4343
return c, nil
4444
}
4545

46+
func (c *desCipher) finalize() {
47+
bcrypt.DestroyKey(c.kh)
48+
}
49+
4650
func (c *desCipher) BlockSize() int { return desBlockSize }
4751

4852
func (c *desCipher) Encrypt(dst, src []byte) {

‎cng/dsa.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ type PrivateKeyDSA struct {
9696
hkey bcrypt.KEY_HANDLE
9797
}
9898

99+
func (k *PrivateKeyDSA) finalize() {
100+
bcrypt.DestroyKey(k.hkey)
101+
}
102+
99103
// PublicKeyDSA represents a DSA public key.
100104
type PublicKeyDSA struct {
101105
DSAParameters
@@ -104,6 +108,10 @@ type PublicKeyDSA struct {
104108
hkey bcrypt.KEY_HANDLE
105109
}
106110

111+
func (k *PublicKeyDSA) finalize() {
112+
bcrypt.DestroyKey(k.hkey)
113+
}
114+
107115
// GenerateKeyDSA generates a new private DSA key using the given parameters.
108116
func GenerateKeyDSA(params DSAParameters) (x, y BigInt, err error) {
109117
h, err := loadDSA()
@@ -147,7 +155,7 @@ func NewPrivateKeyDSA(params DSAParameters, X, Y BigInt) (*PrivateKeyDSA, error)
147155
return nil, err
148156
}
149157
k := &PrivateKeyDSA{params, X, Y, hkey}
150-
addCleanupKey(k, hkey)
158+
runtime.SetFinalizer(k, (*PrivateKeyDSA).finalize)
151159
return k, nil
152160
}
153161

@@ -166,7 +174,7 @@ func NewPublicKeyDSA(params DSAParameters, Y BigInt) (*PublicKeyDSA, error) {
166174
return nil, err
167175
}
168176
k := &PublicKeyDSA{params, Y, hkey}
169-
addCleanupKey(k, hkey)
177+
runtime.SetFinalizer(k, (*PublicKeyDSA).finalize)
170178
return k, nil
171179
}
172180

‎cng/ecdh.go‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,21 @@ type PublicKeyECDH struct {
5858
priv *PrivateKeyECDH
5959
}
6060

61+
func (k *PublicKeyECDH) finalize() {
62+
if k.priv == nil {
63+
bcrypt.DestroyKey(k.hkey)
64+
}
65+
}
66+
6167
type PrivateKeyECDH struct {
6268
hkey bcrypt.KEY_HANDLE
6369
isNIST bool
6470
}
6571

72+
func (k *PrivateKeyECDH) finalize() {
73+
bcrypt.DestroyKey(k.hkey)
74+
}
75+
6676
func ECDH(priv *PrivateKeyECDH, pub *PublicKeyECDH) ([]byte, error) {
6777
// First establish the shared secret.
6878
var secret bcrypt.SECRET_HANDLE
@@ -128,7 +138,7 @@ func GenerateKeyECDH(curve string) (*PrivateKeyECDH, []byte, error) {
128138
bytes = bytes[hdr.KeySize*2:]
129139

130140
k := &PrivateKeyECDH{hkey, isNIST(curve)}
131-
addCleanupKey(k, hkey)
141+
runtime.SetFinalizer(k, (*PrivateKeyECDH).finalize)
132142
return k, bytes, nil
133143
}
134144

@@ -163,7 +173,7 @@ func NewPublicKeyECDH(curve string, bytes []byte) (*PublicKeyECDH, error) {
163173
return nil, err
164174
}
165175
k := &PublicKeyECDH{hkey, append([]byte(nil), bytes...), nil}
166-
addCleanupKey(k, hkey)
176+
runtime.SetFinalizer(k, (*PublicKeyECDH).finalize)
167177
return k, nil
168178
}
169179

@@ -192,7 +202,7 @@ func NewPrivateKeyECDH(curve string, key []byte) (*PrivateKeyECDH, error) {
192202
return nil, err
193203
}
194204
k := &PrivateKeyECDH{hkey, nist}
195-
addCleanupKey(k, hkey)
205+
runtime.SetFinalizer(k, (*PrivateKeyECDH).finalize)
196206
return k, nil
197207
}
198208

@@ -210,9 +220,8 @@ func (k *PrivateKeyECDH) PublicKey() (*PublicKeyECDH, error) {
210220
// Only include X.
211221
bytes = data[:hdr.KeySize]
212222
}
213-
// No cleanup needed: pub.priv prevents k from being garbage collected,
214-
// so k's cleanup will destroy the shared hkey when both are unreachable.
215223
pub := &PublicKeyECDH{k.hkey, bytes, k}
224+
runtime.SetFinalizer(pub, (*PublicKeyECDH).finalize)
216225
return pub, nil
217226
}
218227

‎cng/ecdh_test.go‎

Lines changed: 0 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -158,107 +158,3 @@ func hexDecode(t *testing.T, s string) []byte {
158158
}
159159
return b
160160
}
161-
162-
func BenchmarkGenerateKeyECDH(b *testing.B) {
163-
for _, curve := range []string{"P-256", "P-384", "P-521", "X25519"} {
164-
b.Run(curve, func(b *testing.B) {
165-
b.ReportAllocs()
166-
b.ResetTimer()
167-
for i := 0; i < b.N; i++ {
168-
_, _, err := cng.GenerateKeyECDH(curve)
169-
if err != nil {
170-
b.Fatal(err)
171-
}
172-
}
173-
})
174-
}
175-
}
176-
177-
func BenchmarkECDH(b *testing.B) {
178-
for _, curve := range []string{"P-256", "P-384", "P-521", "X25519"} {
179-
b.Run(curve, func(b *testing.B) {
180-
aliceKey, _, err := cng.GenerateKeyECDH(curve)
181-
if err != nil {
182-
b.Fatal(err)
183-
}
184-
bobKey, _, err := cng.GenerateKeyECDH(curve)
185-
if err != nil {
186-
b.Fatal(err)
187-
}
188-
bobPub, err := bobKey.PublicKey()
189-
if err != nil {
190-
b.Fatal(err)
191-
}
192-
b.ReportAllocs()
193-
b.ResetTimer()
194-
for i := 0; i < b.N; i++ {
195-
_, err := cng.ECDH(aliceKey, bobPub)
196-
if err != nil {
197-
b.Fatal(err)
198-
}
199-
}
200-
})
201-
}
202-
}
203-
204-
func BenchmarkNewPrivateKeyECDH(b *testing.B) {
205-
for _, curve := range []string{"P-256", "P-384", "P-521", "X25519"} {
206-
b.Run(curve, func(b *testing.B) {
207-
_, privBytes, err := cng.GenerateKeyECDH(curve)
208-
if err != nil {
209-
b.Fatal(err)
210-
}
211-
b.ReportAllocs()
212-
b.ResetTimer()
213-
for i := 0; i < b.N; i++ {
214-
_, err := cng.NewPrivateKeyECDH(curve, privBytes)
215-
if err != nil {
216-
b.Fatal(err)
217-
}
218-
}
219-
})
220-
}
221-
}
222-
223-
func BenchmarkNewPublicKeyECDH(b *testing.B) {
224-
for _, curve := range []string{"P-256", "P-384", "P-521", "X25519"} {
225-
b.Run(curve, func(b *testing.B) {
226-
key, _, err := cng.GenerateKeyECDH(curve)
227-
if err != nil {
228-
b.Fatal(err)
229-
}
230-
pub, err := key.PublicKey()
231-
if err != nil {
232-
b.Fatal(err)
233-
}
234-
pubBytes := pub.Bytes()
235-
b.ReportAllocs()
236-
b.ResetTimer()
237-
for i := 0; i < b.N; i++ {
238-
_, err := cng.NewPublicKeyECDH(curve, pubBytes)
239-
if err != nil {
240-
b.Fatal(err)
241-
}
242-
}
243-
})
244-
}
245-
}
246-
247-
func BenchmarkPublicKeyECDH(b *testing.B) {
248-
for _, curve := range []string{"P-256", "P-384", "P-521", "X25519"} {
249-
b.Run(curve, func(b *testing.B) {
250-
key, _, err := cng.GenerateKeyECDH(curve)
251-
if err != nil {
252-
b.Fatal(err)
253-
}
254-
b.ReportAllocs()
255-
b.ResetTimer()
256-
for i := 0; i < b.N; i++ {
257-
_, err := key.PublicKey()
258-
if err != nil {
259-
b.Fatal(err)
260-
}
261-
}
262-
})
263-
}
264-
}

‎cng/ecdsa.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,14 @@ func NewPublicKeyECDSA(curve string, X, Y BigInt) (*PublicKeyECDSA, error) {
9090
return nil, err
9191
}
9292
k := &PublicKeyECDSA{hkey}
93-
addCleanupKey(k, hkey)
93+
runtime.SetFinalizer(k, (*PublicKeyECDSA).finalize)
9494
return k, nil
9595
}
9696

97+
func (k *PublicKeyECDSA) finalize() {
98+
bcrypt.DestroyKey(k.hkey)
99+
}
100+
97101
type PrivateKeyECDSA struct {
98102
hkey bcrypt.KEY_HANDLE
99103
}
@@ -108,10 +112,14 @@ func NewPrivateKeyECDSA(curve string, X, Y, D BigInt) (*PrivateKeyECDSA, error)
108112
return nil, err
109113
}
110114
k := &PrivateKeyECDSA{hkey}
111-
addCleanupKey(k, hkey)
115+
runtime.SetFinalizer(k, (*PrivateKeyECDSA).finalize)
112116
return k, nil
113117
}
114118

119+
func (k *PrivateKeyECDSA) finalize() {
120+
bcrypt.DestroyKey(k.hkey)
121+
}
122+
115123
// SignECDSA signs a hash (which should be the result of hashing a larger message),
116124
// using the private key, priv.
117125
//

‎cng/hash.go‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,6 @@ import (
2020
// maxHashSize is the size of SHA512 and SHA3_512, the largest hashes we support.
2121
const maxHashSize = 64
2222

23-
// addCleanupHash attaches a cleanup function to ptr that will destroy ctx.
24-
func addCleanupHash[T any](ptr *T, ctx bcrypt.HASH_HANDLE) {
25-
runtime.AddCleanup(ptr, func(ctx bcrypt.HASH_HANDLE) {
26-
bcrypt.DestroyHash(ctx)
27-
}, ctx)
28-
}
29-
3023
// SupportsHash returns true if a hash.Hash implementation is supported for h.
3124
func SupportsHash(h crypto.Hash) bool {
3225
switch h {
@@ -195,6 +188,10 @@ func newHash(id string) *Hash {
195188
return &Hash{alg: mustLoadHash(id, bcrypt.ALG_NONE_FLAG)}
196189
}
197190

191+
func (h *Hash) finalize() {
192+
bcrypt.DestroyHash(h.ctx)
193+
}
194+
198195
func (h *Hash) init() {
199196
defer runtime.KeepAlive(h)
200197
if h.ctx != 0 {
@@ -204,15 +201,15 @@ func (h *Hash) init() {
204201
if err != nil {
205202
panic(err)
206203
}
207-
addCleanupHash(h, h.ctx)
204+
runtime.SetFinalizer(h, (*Hash).finalize)
208205
}
209206

210207
func (h *Hash) Clone() (HashCloner, error) {
211208
defer runtime.KeepAlive(h)
212209
h2 := &Hash{alg: h.alg, key: bytes.Clone(h.key)}
213210
if h.ctx != 0 {
214211
hashClone(h.ctx, &h2.ctx)
215-
addCleanupHash(h2, h2.ctx)
212+
runtime.SetFinalizer(h2, (*Hash).finalize)
216213
}
217214
return h2, nil
218215
}

‎cng/keys.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,11 @@ package cng
88

99
import (
1010
"errors"
11-
"runtime"
1211
"unsafe"
1312

1413
"github.com/microsoft/go-crypto-winnative/internal/bcrypt"
1514
)
1615

17-
// addCleanupKey attaches a cleanup function to ptr that will destroy kh.
18-
func addCleanupKey[T any](ptr *T, kh bcrypt.KEY_HANDLE) {
19-
runtime.AddCleanup(ptr, func(kh bcrypt.KEY_HANDLE) {
20-
bcrypt.DestroyKey(kh)
21-
}, kh)
22-
}
23-
2416
const (
2517
sizeOfECCBlobHeader = uint32(unsafe.Sizeof(bcrypt.ECCKEY_BLOB{}))
2618
sizeOfRSABlobHeader = uint32(unsafe.Sizeof(bcrypt.RSAKEY_BLOB{}))

0 commit comments

Comments
 (0)