Skip to content

Commit bfc58d2

Browse files
DifferentialOrangeDifferentialOrange
committed
key: extract error on read fail
Before this patch, read fail had generated a Go error to return it to a user, even though read errors (for example, caused by bad input or wrong password) were placed in error queue. Since goroutines are not binded to a thread and error queue is per thread, sometimes it had resulted in valid connections failing with error. This patch fixes the issue.
1 parent 1c4bc18 commit bfc58d2

File tree

3 files changed

+261
-7
lines changed

3 files changed

+261
-7
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ Forked from https://github.com/libp2p/openssl (unmaintained) to add:
55
1. Fix build by golang 1.13.
66
2. Fix build on Apple M1.
77
3. Fix static build.
8+
4. Fix error extraction on key reading.
89

910
### License
1011

‎key.go

+17-5
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import "C"
1919

2020
import (
2121
"errors"
22+
"fmt"
2223
"io/ioutil"
2324
"runtime"
2425
"unsafe"
@@ -302,9 +303,11 @@ func LoadPrivateKeyFromPEM(pem_block []byte) (PrivateKey, error) {
302303
}
303304
defer C.BIO_free(bio)
304305

306+
runtime.LockOSThread()
307+
defer runtime.UnlockOSThread()
305308
key := C.PEM_read_bio_PrivateKey(bio, nil, nil, nil)
306309
if key == nil {
307-
return nil, errors.New("failed reading private key")
310+
return nil, fmt.Errorf("failed reading private key: %w", errorFromErrorQueue())
308311
}
309312

310313
p := &pKey{key: key}
@@ -328,9 +331,12 @@ func LoadPrivateKeyFromPEMWithPassword(pem_block []byte, password string) (
328331
defer C.BIO_free(bio)
329332
cs := C.CString(password)
330333
defer C.free(unsafe.Pointer(cs))
334+
335+
runtime.LockOSThread()
336+
defer runtime.UnlockOSThread()
331337
key := C.PEM_read_bio_PrivateKey(bio, nil, nil, unsafe.Pointer(cs))
332338
if key == nil {
333-
return nil, errors.New("failed reading private key")
339+
return nil, fmt.Errorf("failed reading private key: %w", errorFromErrorQueue())
334340
}
335341

336342
p := &pKey{key: key}
@@ -352,9 +358,11 @@ func LoadPrivateKeyFromDER(der_block []byte) (PrivateKey, error) {
352358
}
353359
defer C.BIO_free(bio)
354360

361+
runtime.LockOSThread()
362+
defer runtime.UnlockOSThread()
355363
key := C.d2i_PrivateKey_bio(bio, nil)
356364
if key == nil {
357-
return nil, errors.New("failed reading private key der")
365+
return nil, fmt.Errorf("failed reading private key der: %w", errorFromErrorQueue())
358366
}
359367

360368
p := &pKey{key: key}
@@ -383,9 +391,11 @@ func LoadPublicKeyFromPEM(pem_block []byte) (PublicKey, error) {
383391
}
384392
defer C.BIO_free(bio)
385393

394+
runtime.LockOSThread()
395+
defer runtime.UnlockOSThread()
386396
key := C.PEM_read_bio_PUBKEY(bio, nil, nil, nil)
387397
if key == nil {
388-
return nil, errors.New("failed reading public key")
398+
return nil, fmt.Errorf("failed reading public key: %w", errorFromErrorQueue())
389399
}
390400

391401
p := &pKey{key: key}
@@ -407,9 +417,11 @@ func LoadPublicKeyFromDER(der_block []byte) (PublicKey, error) {
407417
}
408418
defer C.BIO_free(bio)
409419

420+
runtime.LockOSThread()
421+
defer runtime.UnlockOSThread()
410422
key := C.d2i_PUBKEY_bio(bio, nil)
411423
if key == nil {
412-
return nil, errors.New("failed reading public key der")
424+
return nil, fmt.Errorf("failed reading public key der: %w", errorFromErrorQueue())
413425
}
414426

415427
p := &pKey{key: key}

‎ssl_test.go

+243-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
"bytes"
1919
"crypto/rand"
2020
"crypto/tls"
21+
"encoding/base64"
22+
"fmt"
2123
"io"
2224
"io/ioutil"
2325
"net"
@@ -79,6 +81,77 @@ ucCCa4lOGgPtXJ0Qf1c8yq5vh4yqkQjrgUTkr+CFDGR6y4CxmNDQxEMYIajaIiSY
7981
qmgvgyRayemfO2zR0CPgC6wSoGBth+xW6g+WA8y0z76ZSaWpFi8lVM4=
8082
-----END RSA PRIVATE KEY-----
8183
`)
84+
keyDERBase64 = `MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDdf3icNvFsrlrnNLi8SocscqlS
85+
bFq+pEvmhcSoqgDLqebnqu8Ld73HJJ74MGXEgRX8xZT5FinOML31CR6t9E/j3dqV6p+GfdlFLe3I
86+
qtC0/bPVnCDBirBygBI4uCrMq+1VhAxPWclrDo7l9QRYbsExH9lfn+RyvxeNMZiOASasvVZNncY8
87+
E9usBGRdH17EfDL/TPwXqWOLyxSN5o54GTztjjy9w9CGQP7jcCueKYyQJQCtEmnwc6P/q6/EPv5R
88+
6drBkX6loAPtmCUAkHqxkWOJrRq/v7PwzRYhfY+ZpVHGc7WEkDnLzRiUypr1C9oxvLKS10etZEIw
89+
EdKyOkSg2fdPAgMBAAECggEBALCCnpjOaAIVx7csGnNiaOoQzcIzOvVldF7WBuvp3gxu7uV7IFfh
90+
KkkCc/SQjOjVfbIbuiXtdY8s5JPamqpBYVDTQRfrCwlgTL6GZVFeXkd9TcxSSQAzB32Xde3hRaoo
91+
8E8Plce+Y3Z++X1jjfzy9d2x5cYAY0rV4WzRMyMm460PEPKuYlyfGknvL/vvL8dvmtRr0MK+Py2Q
92+
oeAeZ4jBATQeEgWh+P6oJhup5XKNs8BphkyshJdvYeZzJQsoQ4ZFswfbgjMicoDQ3FWR0/2qZXxj
93+
ZsnrT0V+3/LxWmPfWIR2r84VfKht1W+dcydpXIB62hpbLsA5MhDYKfDAZsn8XwECgYEA3+kKctCt
94+
ZER5Uy4WN/4yBiAK1FFpDwQ5W+NhxdeLQ+G04wVMBvt2OqGWCYE4WyZ5dxvxhFmwX6k227XGTlEk
95+
6q7Ip8ryPWtR0Al+jEYeJpQUjb/WL0TO5df8CjTE1GRYzEwq7BgGaa3V8y5Wj7UzxQTAUx0dG2DF
96+
77/545XUnY8CgYEA/T3sYs5hJgAjU47cE39sRU1q7q0DE/lhKEaZfZDHLT6ijEdnVXcPzKjoLSFK
97+
D4f589OaMyAZy9pbor5DIJ0cOOBuTaF91n4F6r3YkCk+sf2AxXsw+HAfODCPMMOKj04XFhje+MFD
98+
bJf72O/P2WS+rY0zd+oWpuwboS37mYJzqkECgYEAjrZvFW0SBuVp2u119exLoAHORTM6XfrYQEv2
99+
Jm5Sckqqy0O2CIFAAvC4u4gkDlzAcH1b+3pa4y3sLC94nLQ1bmtGs0O0EBeWBp32jZunXfll/E74
100+
Shp2MKLwHuUxSxpGSriFZwONGtBUnHG9dE0PGRUFLDRTN/7/Sec3c6os4NsCgYAIJj7+KwALVgPN
101+
A5LneblFPamMRrsLoIHU5vi3hroyJYrbksyrfmpevqzCDwkwGMMdapjSvly2J6+9O/wzB3tKBUbn
102+
bqP7DBEqrbNTaFBhL/Q95qn7xLfsefuRqSlDVVL+3gwG20lNLFLpd0YsC8brFNksKbdS5dQ5yp4H
103+
IaCRQQKBgQCainjmRGL3viDgw+/IumVwTizJvX2J+sePvqA9EGL3gKnyNmixjVO5wIJriU4aA+1c
104+
nRB/VzzKrm+HjKqRCOuBROSv4IUMZHrLgLGY0NDEQxghqNoiJJiqaC+DJFrJ6Z87bNHQI+ALrBKg
105+
YG2H7FbqD5YDzLTPvplJpakWLyVUzg==
106+
`
107+
keyEncryptedBytes = []byte(`-----BEGIN ENCRYPTED PRIVATE KEY-----
108+
MIIFLTBXBgkqhkiG9w0BBQ0wSjApBgkqhkiG9w0BBQwwHAQIEyjG5ZrEc7ACAggA
109+
MAwGCCqGSIb3DQIJBQAwHQYJYIZIAWUDBAEqBBDOuZqPxXCetvSxecEZgZyFBIIE
110+
0PQTvLPEM6mh/yqURhnfqg/sQvKnm9AoaVfeucK9E25wpuAr24mR3/QBmL+cIGQx
111+
oNPohmf0MU8CHBgzg4dNL6cRFohHdzrEemV02hk3NRv4z+UhQQelT/ZwF3PbcegI
112+
Zbj6POzZjoK5NXDDuEqxG2SN59+oEEmF/fJkPuK0iqgVsAEvOFYFb117/IbRMjNJ
113+
vse4ZYmNEfknww4OKPL8D8gBYbtPbsKaQTVcoQuJMiaUCypU5uBucYFIjzY2otkM
114+
mVNL4YaS0YdZpzp6JfNLQF80IILRtW+JpYzBALQJH5pTjXH6mA1/RDJwCGwwZrMu
115+
18UtNre2bMPDdCL+GX8uPG0HuTpBjojELaVZz1aimJN9vad/Q+X6QxiXYRJseTML
116+
IO8nHuEu36HAg7OzOU3umCGdlQ7z3GJ9eP6npE1p44h89zbHOMYcGp/doG2f1fO5
117+
2lAqpfG/fAtefW7yUmSrGXVe0g8L62qoGyv4DJdSPaa0Nc+N/FeKc8e/V+kWOoDm
118+
LY0XIy8TATuiqS9NwaKFSGC/kUoDt0UTPqUGeAjObfabiLOOCsuUJmohF+BxxpO/
119+
xNIcylDUuYDbDFVNSWeDToloVH8i5RZeLy2vskLM4uHrOraaRH9HUnqMQ9jQ7SXh
120+
1/lCmDJgStrjkYL9IhVzXfrmtOZqASwwUiiFiQoJLsnN3ic/6PHx8gu890wpL0Se
121+
jUgLxX21m42tZ1ismGcmzL8U00RAEth+fO+0dLQx1c6yfsSywlb2Fb7kuMW3HU15
122+
tbpA7AfZviqarXLcECFsbzOMt/pfUbMUG3OOJ6q/4gMiAEPi+TIrECFCkjHP0Tgw
123+
aeCC2I3yfboaSNeI6dH422JJwPvfRc2I3MHOHlpXnRCgF3btDKW8vw96b7X/P5uV
124+
9/KpXirP/O3JYWYg/co1KaT6LCtuCfUf8Z9gZYbcwn6Kxh9g5LQPysxMVQVx9R2H
125+
ktjWUWwNUVOPA4GtbiNbQXjAgyyTPxv2wJSJav0yJrJUkqkvz1nrnIyocGk+xiJ8
126+
BAUl/GOGeiS5gskxumJzG6iIv8LRTFKQ85Lp5oD9EwAbxloASjDVwzMSVCqcZQfb
127+
q4VIpbcBUpvyH6tchxQUujmI2ZQ/54C9u100Z4gAVYsLuRaDKcJ1c3kuffLIz/fI
128+
Cfa/kzt+o9YyeCxz2w0aVHsbOk+0P1dL8usBc5b1MB7RaH7So8/7j8mluzyiUNn9
129+
64VUiSNWEEVlDN40Ar+BBBdRUUJJkWgpPuLQpCj6dSPxevAeHjbytjByUSPt+v0d
130+
oGW4XCDw/72IBa/S5kSgE+n5FV3lrxq4DAgEeVVmZkO35cis7mqcgSgxwBzDclku
131+
UZ56N5FOw5WwELz7+zC1fvdJGpKAwr9uOu6mKArvIshCGhRLeOSjBe3biERcrCJt
132+
WrJra8Zt/E7fOFi0KedYQtdu/7oV31NrTFj5+eL+j/D6+tKGA1+goDDLt5xPFbu+
133+
l8yjbswmGrOTCHMrd4SJqmGUMWz1dWMdjIeQrwGok25mIE9BtUGzyQN84oWMe75Z
134+
eD+ArpnO4zJcny47LG9PwtyBVn3GDinB/RGi2qcJPYg7xmV7PDNlFuvExctBEQJD
135+
7onbx6HP5kKOZHTZvkm2viGuZbG2Pgz2kk32CxsxWTUL
136+
-----END ENCRYPTED PRIVATE KEY-----
137+
`)
138+
keyEncryptedPassword = "mysslpassword"
139+
keyPublicBytes = []byte(`-----BEGIN PUBLIC KEY-----
140+
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA3X94nDbxbK5a5zS4vEqH
141+
LHKpUmxavqRL5oXEqKoAy6nm56rvC3e9xySe+DBlxIEV/MWU+RYpzjC99QkerfRP
142+
493aleqfhn3ZRS3tyKrQtP2z1ZwgwYqwcoASOLgqzKvtVYQMT1nJaw6O5fUEWG7B
143+
MR/ZX5/kcr8XjTGYjgEmrL1WTZ3GPBPbrARkXR9exHwy/0z8F6lji8sUjeaOeBk8
144+
7Y48vcPQhkD+43ArnimMkCUArRJp8HOj/6uvxD7+UenawZF+paAD7ZglAJB6sZFj
145+
ia0av7+z8M0WIX2PmaVRxnO1hJA5y80YlMqa9QvaMbyyktdHrWRCMBHSsjpEoNn3
146+
TwIDAQAB
147+
-----END PUBLIC KEY-----
148+
`)
149+
keyPublicDERBase64 = `MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA3X94nDbxbK5a5zS4vEqHLHKpUmxavqRL
150+
5oXEqKoAy6nm56rvC3e9xySe+DBlxIEV/MWU+RYpzjC99QkerfRP493aleqfhn3ZRS3tyKrQtP2z
151+
1ZwgwYqwcoASOLgqzKvtVYQMT1nJaw6O5fUEWG7BMR/ZX5/kcr8XjTGYjgEmrL1WTZ3GPBPbrARk
152+
XR9exHwy/0z8F6lji8sUjeaOeBk87Y48vcPQhkD+43ArnimMkCUArRJp8HOj/6uvxD7+UenawZF+
153+
paAD7ZglAJB6sZFjia0av7+z8M0WIX2PmaVRxnO1hJA5y80YlMqa9QvaMbyyktdHrWRCMBHSsjpE
154+
oNn3TwIDAQAB`
82155
prime256v1KeyBytes = []byte(`-----BEGIN EC PRIVATE KEY-----
83156
MHcCAQEEIB/XL0zZSsAu+IQF1AI/nRneabb2S126WFlvvhzmYr1KoAoGCCqGSM49
84157
AwEHoUQDQgAESSFGWwF6W1hoatKGPPorh4+ipyk0FqpiWdiH+4jIiU39qtOeZGSh
@@ -665,7 +738,7 @@ func TestStdlibLotsOfConns(t *testing.T) {
665738
})
666739
}
667740

668-
func TestOpenSSLLotsOfConns(t *testing.T) {
741+
func getCtx(t *testing.T) *Ctx {
669742
ctx, err := NewCtx()
670743
if err != nil {
671744
t.Fatal(err)
@@ -684,7 +757,12 @@ func TestOpenSSLLotsOfConns(t *testing.T) {
684757
if err = ctx.UseCertificate(cert); err != nil {
685758
t.Fatal(err)
686759
}
687-
if err = ctx.SetCipherList("AES128-SHA"); err != nil {
760+
return ctx
761+
}
762+
763+
func TestOpenSSLLotsOfConns(t *testing.T) {
764+
ctx := getCtx(t)
765+
if err := ctx.SetCipherList("AES128-SHA"); err != nil {
688766
t.Fatal(err)
689767
}
690768
LotsOfConns(t, 1024*64, 10, 100, 0*time.Second,
@@ -694,3 +772,166 @@ func TestOpenSSLLotsOfConns(t *testing.T) {
694772
return Client(c, ctx)
695773
})
696774
}
775+
776+
func getCtxWithPrivateKeyAfterFail(t *testing.T,
777+
getPrivateKeyAfterFail func(t *testing.T) PrivateKey) *Ctx {
778+
ctx, err := NewCtx()
779+
if err != nil {
780+
t.Fatal(err)
781+
}
782+
783+
key := getPrivateKeyAfterFail(t)
784+
785+
if err = ctx.UsePrivateKey(key); err != nil {
786+
t.Fatal(err)
787+
}
788+
789+
cert, err := LoadCertificateFromPEM(certBytes)
790+
if err != nil {
791+
t.Fatal(err)
792+
}
793+
794+
if err = ctx.UseCertificate(cert); err != nil {
795+
t.Fatal(err)
796+
}
797+
798+
return ctx
799+
800+
}
801+
802+
func getPrivatePEMKeyAfterFail(t *testing.T) PrivateKey {
803+
_, err := LoadPrivateKeyFromPEM([]byte("badbadkey"))
804+
if err == nil {
805+
t.Fatal("Expected error, got none")
806+
}
807+
808+
key, err := LoadPrivateKeyFromPEM(keyBytes)
809+
if err != nil {
810+
t.Fatal(err)
811+
}
812+
813+
return key
814+
}
815+
816+
func getPrivateEncryptedPEMKeyAfterFail(t *testing.T) PrivateKey {
817+
badPassword := fmt.Sprintf("wrong_%s", keyEncryptedPassword)
818+
_, err := LoadPrivateKeyFromPEMWithPassword(keyEncryptedBytes, badPassword)
819+
if err == nil {
820+
t.Fatal("Expected error, got none")
821+
}
822+
823+
key, err := LoadPrivateKeyFromPEMWithPassword(keyEncryptedBytes, keyEncryptedPassword)
824+
if err != nil {
825+
t.Fatal(err)
826+
}
827+
828+
return key
829+
}
830+
831+
func getPrivateDERKeyAfterFail(t *testing.T) PrivateKey {
832+
keyDERBytes, err := base64.StdEncoding.DecodeString(keyDERBase64)
833+
if err != nil {
834+
t.Fatal(err)
835+
}
836+
837+
_, err = LoadPrivateKeyFromDER([]byte("badbadkey"))
838+
if err == nil {
839+
t.Fatal("Expected error, got none")
840+
}
841+
842+
key, err := LoadPrivateKeyFromDER(keyDERBytes)
843+
if err != nil {
844+
t.Fatal(err)
845+
}
846+
847+
return key
848+
}
849+
850+
func getCtxWithPublicKeyAfterFail(t *testing.T,
851+
getPublicKeyAfterFail func(t *testing.T) PublicKey) *Ctx {
852+
ctx, err := NewCtx()
853+
if err != nil {
854+
t.Fatal(err)
855+
}
856+
857+
cert, err := LoadCertificateFromPEM(certBytes)
858+
if err != nil {
859+
t.Fatal(err)
860+
}
861+
862+
key := getPublicKeyAfterFail(t)
863+
864+
if err = cert.SetPubKey(key); err != nil {
865+
t.Fatal(err)
866+
}
867+
868+
if err = ctx.UseCertificate(cert); err != nil {
869+
t.Fatal(err)
870+
}
871+
872+
return ctx
873+
}
874+
875+
func getPublicPEMKeyAfterFail(t *testing.T) PublicKey {
876+
_, err := LoadPublicKeyFromPEM([]byte("badbadkey"))
877+
if err == nil {
878+
t.Fatal("Expected error, got none")
879+
}
880+
881+
key, err := LoadPublicKeyFromPEM(keyPublicBytes)
882+
if err != nil {
883+
t.Fatal(err)
884+
}
885+
886+
return key
887+
}
888+
889+
func getPublicDERKeyAfterFail(t *testing.T) PublicKey {
890+
keyPublicDERBytes, err := base64.StdEncoding.DecodeString(keyPublicDERBase64)
891+
if err != nil {
892+
t.Fatal(err)
893+
}
894+
895+
_, err = LoadPublicKeyFromDER([]byte("badbadkey"))
896+
if err == nil {
897+
t.Fatal("Expected error, got none")
898+
}
899+
900+
key, err := LoadPublicKeyFromDER(keyPublicDERBytes)
901+
if err != nil {
902+
t.Fatal(err)
903+
}
904+
905+
return key
906+
}
907+
908+
var lotsOfConnsWithFailCases = map[string]func(t *testing.T) *Ctx{
909+
"PrivatePEM": func(t *testing.T) *Ctx {
910+
return getCtxWithPrivateKeyAfterFail(t, getPrivatePEMKeyAfterFail)
911+
},
912+
"PrivateEncryptedPEM": func(t *testing.T) *Ctx {
913+
return getCtxWithPrivateKeyAfterFail(t, getPrivateEncryptedPEMKeyAfterFail)
914+
},
915+
"PrivateDER": func(t *testing.T) *Ctx {
916+
return getCtxWithPrivateKeyAfterFail(t, getPrivateDERKeyAfterFail)
917+
},
918+
"PublicPEM": func(t *testing.T) *Ctx {
919+
return getCtxWithPublicKeyAfterFail(t, getPublicPEMKeyAfterFail)
920+
},
921+
"PublicDER": func(t *testing.T) *Ctx {
922+
return getCtxWithPublicKeyAfterFail(t, getPublicDERKeyAfterFail)
923+
},
924+
}
925+
926+
func TestOpenSSLLotsOfConnsWithFail(t *testing.T) {
927+
for name, getClientCtx := range lotsOfConnsWithFailCases {
928+
t.Run(name, func(t *testing.T) {
929+
LotsOfConns(t, 1024*64, 10, 100, 0*time.Second,
930+
func(l net.Listener) net.Listener {
931+
return NewListener(l, getCtx(t))
932+
}, func(c net.Conn) (net.Conn, error) {
933+
return Client(c, getClientCtx(t))
934+
})
935+
})
936+
}
937+
}

0 commit comments

Comments
 (0)