Skip to content

Commit 9747a9f

Browse files
MEN-3730 Switch all client crypto operations over to OpenSSL.
All server communication goes through openssl: * custom dial function is provided * new verification errors are handled Unit tests fixes: * new certs * new call startTestHTTPS, to avoid build-in localhostCert from httptest * new tests: * TestClientAuthDepthZeroSelfSignedCert X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT * TestClientAuthEndEntityKeyTooSmall X509_V_ERR_EE_KEY_TOO_SMALL * test host added to /etc/hosts in .gitlab-ci.yml * added host verification error test * added no verify test ChangeLog:Switch to OpenSSL for all server communication. Signed-off-by: Peter Grzybowski <peter@northern.tech>
1 parent eaa7f1a commit 9747a9f

13 files changed

Lines changed: 1182 additions & 115 deletions

client/client.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"strings"
2727
"time"
2828

29+
openssl "github.com/Linutronix/golang-openssl"
2930
"github.com/pkg/errors"
3031
log "github.com/sirupsen/logrus"
3132
"golang.org/x/net/http2"
@@ -280,6 +281,49 @@ func newHttpClient() *http.Client {
280281
return &http.Client{}
281282
}
282283

284+
func dialOpenSSL(conf Config, network string, addr string) (net.Conn, error) {
285+
contextSSL, err := openssl.NewCtx()
286+
// probably should consider reusing the context, but then we
287+
// have to propagate it with every request.
288+
err = contextSSL.LoadVerifyLocations(conf.ServerCert, "")
289+
if err != nil {
290+
return nil, err
291+
}
292+
flags := openssl.DialFlags(0)
293+
294+
if conf.NoVerify {
295+
flags = openssl.InsecureSkipHostVerification
296+
}
297+
298+
conn, err := openssl.Dial("tcp", addr, contextSSL, flags)
299+
if err != nil {
300+
return nil, err
301+
}
302+
303+
v := conn.VerifyResult()
304+
if v != openssl.Ok {
305+
if v == openssl.CertHasExpired {
306+
return nil, errors.Errorf("certificate has expired, "+
307+
"openssl verify rc: %d server cert file: %s", v, conf.ServerCert)
308+
}
309+
if v == openssl.DepthZeroSelfSignedCert {
310+
return nil, errors.Errorf("depth zero self-signed certificate, "+
311+
"openssl verify rc: %d server cert file: %s", v, conf.ServerCert)
312+
}
313+
if v == 0x42 { //X509_V_ERR_EE_KEY_TOO_SMALL
314+
return nil, errors.Errorf("end entity key too short, "+
315+
"openssl verify rc: %d server cert file: %s", v, conf.ServerCert)
316+
}
317+
if v == 0x14 { //X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY
318+
return nil, errors.Errorf("certificate signed by unknown authority, "+
319+
"openssl verify rc: %d server cert file: %s", v, conf.ServerCert)
320+
}
321+
return nil, errors.Errorf("not a valid certificate, "+
322+
"openssl verify rc: %d server cert file: %s", v, conf.ServerCert)
323+
}
324+
return conn, err
325+
}
326+
283327
func newHttpsClient(conf Config) (*http.Client, error) {
284328
client := newHttpClient()
285329

@@ -295,6 +339,9 @@ func newHttpsClient(conf Config) (*http.Client, error) {
295339
transport := http.Transport{
296340
TLSClientConfig: &tlsc,
297341
Proxy: http.ProxyFromEnvironment,
342+
DialTLS: func(network string, addr string) (net.Conn, error) {
343+
return dialOpenSSL(conf, network, addr)
344+
},
298345
}
299346

300347
client.Transport = &transport

client/client_auth_test.go

Lines changed: 148 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"fmt"
2020
"io/ioutil"
2121
"net/http"
22-
"net/http/httptest"
2322
"testing"
2423

2524
"github.com/stretchr/testify/assert"
@@ -88,13 +87,15 @@ func TestClientAuth(t *testing.T) {
8887
http.Header{},
8988
}
9089

91-
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
90+
ts := startTestHTTPS(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
9291
responder.headers = r.Header
9392
w.WriteHeader(responder.httpStatus)
9493
w.Header().Set("Content-Type", "application/json")
9594

9695
fmt.Fprint(w, responder.data)
97-
}))
96+
}),
97+
localhostCert,
98+
localhostKey)
9899
defer ts.Close()
99100

100101
ac, err := NewApiClient(
@@ -122,8 +123,10 @@ func TestClientAuth(t *testing.T) {
122123
}
123124

124125
func TestClientAuthExpiredCert(t *testing.T) {
125-
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
126-
}))
126+
ts := startTestHTTPS(
127+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}),
128+
localhostCertExpired,
129+
localhostKeyExpired)
127130
defer ts.Close()
128131

129132
ac, err := NewApiClient(
@@ -145,8 +148,10 @@ func TestClientAuthExpiredCert(t *testing.T) {
145148
}
146149

147150
func TestClientAuthUnknownAuthorityCert(t *testing.T) {
148-
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
149-
}))
151+
ts := startTestHTTPS(
152+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}),
153+
localhostCertUnknown,
154+
localhostKeyUnknown)
150155
defer ts.Close()
151156

152157
ac, err := NewApiClient(
@@ -164,12 +169,70 @@ func TestClientAuthUnknownAuthorityCert(t *testing.T) {
164169
rsp, err := client.Request(ac, ts.URL, msger)
165170
assert.Error(t, err)
166171
assert.Contains(t, err.Error(), "certificate signed by unknown authority")
172+
// see https://github.com/openssl/openssl/blob/OpenSSL_1_1_1-stable/crypto/x509/x509_vfy.c#L3268
173+
// for self-signed openssl always returns self-signed error; either
174+
// X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT or X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN
175+
// and never X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT or X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY
176+
assert.Nil(t, rsp)
177+
}
178+
179+
//X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT
180+
func TestClientAuthDepthZeroSelfSignedCert(t *testing.T) {
181+
ts := startTestHTTPS(
182+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}),
183+
localhostCert,
184+
localhostKey)
185+
defer ts.Close()
186+
187+
ac, err := NewApiClient(
188+
Config{"server.zero.depth.self.signed.crt", true, false},
189+
)
190+
assert.NotNil(t, ac)
191+
assert.NoError(t, err)
192+
193+
client := NewAuth()
194+
assert.NotNil(t, client)
195+
196+
msger := &testAuthDataMessenger{
197+
reqData: []byte("foobar"),
198+
}
199+
rsp, err := client.Request(ac, ts.URL, msger)
200+
assert.Error(t, err)
201+
assert.Contains(t, err.Error(), "depth zero self-signed certificate")
202+
assert.Nil(t, rsp)
203+
}
204+
205+
//X509_V_ERR_EE_KEY_TOO_SMALL
206+
func TestClientAuthEndEntityKeyTooSmall(t *testing.T) {
207+
ts := startTestHTTPS(
208+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}),
209+
localhostCertShortEEKey,
210+
localhostKeyShortEEKey)
211+
defer ts.Close()
212+
213+
ac, err := NewApiClient(
214+
Config{"server.crt", true, false},
215+
)
216+
assert.NotNil(t, ac)
217+
assert.NoError(t, err)
218+
219+
client := NewAuth()
220+
assert.NotNil(t, client)
221+
222+
msger := &testAuthDataMessenger{
223+
reqData: []byte("foobar"),
224+
}
225+
rsp, err := client.Request(ac, ts.URL, msger)
226+
assert.Error(t, err)
227+
assert.Contains(t, err.Error(), "end entity key too short")
167228
assert.Nil(t, rsp)
168229
}
169230

170231
func TestClientAuthNoCert(t *testing.T) {
171-
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
172-
}))
232+
ts := startTestHTTPS(
233+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}),
234+
localhostCert,
235+
localhostKey)
173236
defer ts.Close()
174237

175238
ac, err := NewApiClient(
@@ -178,3 +241,79 @@ func TestClientAuthNoCert(t *testing.T) {
178241
assert.NotNil(t, ac)
179242
assert.NoError(t, err)
180243
}
244+
245+
func TestClientAuthHostValidationNocheck(t *testing.T) {
246+
ts := startTestHTTPS(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}),
247+
localhostCert,
248+
localhostKey)
249+
defer ts.Close()
250+
251+
ac, err := NewApiClient(
252+
Config{"server.crt", true, true},
253+
)
254+
assert.NotNil(t, ac)
255+
assert.NoError(t, err)
256+
257+
client := NewAuth()
258+
assert.NotNil(t, client)
259+
260+
msger := &testAuthDataMessenger{
261+
reqData: []byte("foobar"),
262+
}
263+
rsp, err := client.Request(ac, ts.URL, msger)
264+
265+
assert.NoError(t, err)
266+
assert.NotNil(t, rsp)
267+
}
268+
269+
func TestClientAuthHostValidationError(t *testing.T) {
270+
ts := startTestHTTPS(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}),
271+
localhostCertWrongHost,
272+
localhostKeyWrongHost)
273+
defer ts.Close()
274+
275+
ac, err := NewApiClient(
276+
Config{"server.unknown-authority.crt", true, false},
277+
)
278+
assert.NotNil(t, ac)
279+
assert.NoError(t, err)
280+
281+
client := NewAuth()
282+
assert.NotNil(t, client)
283+
284+
msger := &testAuthDataMessenger{
285+
reqData: []byte("foobar"),
286+
}
287+
rsp, err := client.Request(ac, ts.URL, msger)
288+
289+
assert.Error(t, err)
290+
assert.Contains(t, err.Error(), "Host validation error")
291+
assert.Nil(t, rsp)
292+
}
293+
294+
// this tests for the error that is handled by default 'not a valid certificate'
295+
// via X509_V_ERR_CA_KEY_TOO_SMALL
296+
func TestClientAuthNotValidCertificate(t *testing.T) {
297+
ts := startTestHTTPS(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}),
298+
localhostCertCAKeyTooShort,
299+
localhostKeyCAKeyTooShort)
300+
defer ts.Close()
301+
302+
ac, err := NewApiClient(
303+
Config{"server.ca.key.too.small.crt", true, false},
304+
)
305+
assert.NotNil(t, ac)
306+
assert.NoError(t, err)
307+
308+
client := NewAuth()
309+
assert.NotNil(t, client)
310+
311+
msger := &testAuthDataMessenger{
312+
reqData: []byte("foobar"),
313+
}
314+
rsp, err := client.Request(ac, ts.URL, msger)
315+
316+
assert.Error(t, err)
317+
assert.Contains(t, err.Error(), "not a valid certificate")
318+
assert.Nil(t, rsp)
319+
}

client/client_inventory_test.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017 Northern.tech AS
1+
// Copyright 2020 Northern.tech AS
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@ package client
1616
import (
1717
"io/ioutil"
1818
"net/http"
19-
"net/http/httptest"
2019
"testing"
2120

2221
"github.com/pkg/errors"
@@ -35,12 +34,15 @@ func TestInventoryClient(t *testing.T) {
3534
}
3635

3736
// Test server that always responds with 200 code, and specific payload
38-
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
39-
w.WriteHeader(responder.httpStatus)
37+
ts := startTestHTTPS(
38+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
39+
w.WriteHeader(responder.httpStatus)
4040

41-
responder.recdata, _ = ioutil.ReadAll(r.Body)
42-
responder.path = r.URL.Path
43-
}))
41+
responder.recdata, _ = ioutil.ReadAll(r.Body)
42+
responder.path = r.URL.Path
43+
}),
44+
localhostCert,
45+
localhostKey)
4446
defer ts.Close()
4547

4648
ac, err := NewApiClient(

client/client_log_test.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017 Northern.tech AS
1+
// Copyright 2020 Northern.tech AS
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@ package client
1616
import (
1717
"io/ioutil"
1818
"net/http"
19-
"net/http/httptest"
2019
"testing"
2120

2221
"github.com/pkg/errors"
@@ -35,12 +34,15 @@ func TestLogUploadClient(t *testing.T) {
3534
}
3635

3736
// Test server that always responds with 200 code, and specific payload
38-
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
39-
w.WriteHeader(responder.httpStatus)
37+
ts := startTestHTTPS(
38+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
39+
w.WriteHeader(responder.httpStatus)
4040

41-
responder.recdata, _ = ioutil.ReadAll(r.Body)
42-
responder.path = r.URL.Path
43-
}))
41+
responder.recdata, _ = ioutil.ReadAll(r.Body)
42+
responder.path = r.URL.Path
43+
}),
44+
localhostCert,
45+
localhostKey)
4446
defer ts.Close()
4547

4648
ac, err := NewApiClient(

client/client_status_test.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2018 Northern.tech AS
1+
// Copyright 2020 Northern.tech AS
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@ package client
1616
import (
1717
"io/ioutil"
1818
"net/http"
19-
"net/http/httptest"
2019
"testing"
2120

2221
"github.com/pkg/errors"
@@ -35,12 +34,15 @@ func TestStatusClient(t *testing.T) {
3534
}
3635

3736
// Test server that always responds with 200 code, and specific payload
38-
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
39-
w.WriteHeader(responder.httpStatus)
37+
ts := startTestHTTPS(
38+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
39+
w.WriteHeader(responder.httpStatus)
4040

41-
responder.recdata, _ = ioutil.ReadAll(r.Body)
42-
responder.path = r.URL.Path
43-
}))
41+
responder.recdata, _ = ioutil.ReadAll(r.Body)
42+
responder.path = r.URL.Path
43+
}),
44+
localhostCert,
45+
localhostKey)
4446
defer ts.Close()
4547

4648
ac, err := NewApiClient(

0 commit comments

Comments
 (0)