Skip to content

Commit b0b2e77

Browse files
marainomaraino
committed
Avoid doing unauthenticated requests on the SDK
When step-ca runs with mTLS required on some endpoints, the SDK used in autocert will fail to start because the identity certificate is missing. This certificate is only required to retrieve all roots, in most cases there's only one, and the SDK has access to it.
1 parent fbd3fd2 commit b0b2e77

File tree

3 files changed

+107
-6
lines changed

3 files changed

+107
-6
lines changed

Diff for: ‎ca/bootstrap.go

+39-6
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ func BootstrapClient(ctx context.Context, token string, options ...TLSOption) (*
6363
return nil, err
6464
}
6565

66+
version, err := client.Version()
67+
if err != nil {
68+
return nil, err
69+
}
70+
6671
req, pk, err := CreateSignRequest(token)
6772
if err != nil {
6873
return nil, err
@@ -73,8 +78,14 @@ func BootstrapClient(ctx context.Context, token string, options ...TLSOption) (*
7378
return nil, err
7479
}
7580

76-
// Make sure the tlsConfig have all supported roots on RootCAs
77-
options = append(options, AddRootsToRootCAs())
81+
// Make sure the tlsConfig have all supported roots on RootCAs.
82+
//
83+
// The roots request is only supported if identity certificates are not
84+
// required. In all cases the current root is also added after applying all
85+
// options too.
86+
if !version.RequireClientAuthentication {
87+
options = append(options, AddRootsToRootCAs())
88+
}
7889

7990
transport, err := client.Transport(ctx, sign, pk, options...)
8091
if err != nil {
@@ -125,6 +136,11 @@ func BootstrapServer(ctx context.Context, token string, base *http.Server, optio
125136
return nil, err
126137
}
127138

139+
version, err := client.Version()
140+
if err != nil {
141+
return nil, err
142+
}
143+
128144
req, pk, err := CreateSignRequest(token)
129145
if err != nil {
130146
return nil, err
@@ -135,8 +151,14 @@ func BootstrapServer(ctx context.Context, token string, base *http.Server, optio
135151
return nil, err
136152
}
137153

138-
// Make sure the tlsConfig have all supported roots on ClientCAs and RootCAs
139-
options = append(options, AddRootsToCAs())
154+
// Make sure the tlsConfig have all supported roots on RootCAs.
155+
//
156+
// The roots request is only supported if identity certificates are not
157+
// required. In all cases the current root is also added after applying all
158+
// options too.
159+
if !version.RequireClientAuthentication {
160+
options = append(options, AddRootsToCAs())
161+
}
140162

141163
tlsConfig, err := client.GetServerTLSConfig(ctx, sign, pk, options...)
142164
if err != nil {
@@ -177,6 +199,11 @@ func BootstrapListener(ctx context.Context, token string, inner net.Listener, op
177199
return nil, err
178200
}
179201

202+
version, err := client.Version()
203+
if err != nil {
204+
return nil, err
205+
}
206+
180207
req, pk, err := CreateSignRequest(token)
181208
if err != nil {
182209
return nil, err
@@ -187,8 +214,14 @@ func BootstrapListener(ctx context.Context, token string, inner net.Listener, op
187214
return nil, err
188215
}
189216

190-
// Make sure the tlsConfig have all supported roots on ClientCAs and RootCAs
191-
options = append(options, AddRootsToCAs())
217+
// Make sure the tlsConfig have all supported roots on RootCAs.
218+
//
219+
// The roots request is only supported if identity certificates are not
220+
// required. In all cases the current root is also added after applying all
221+
// options too.
222+
if !version.RequireClientAuthentication {
223+
options = append(options, AddRootsToCAs())
224+
}
192225

193226
tlsConfig, err := client.GetServerTLSConfig(ctx, sign, pk, options...)
194227
if err != nil {

Diff for: ‎ca/bootstrap_test.go

+66
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@ import (
88
"net/http"
99
"net/http/httptest"
1010
"reflect"
11+
"strings"
1112
"sync"
1213
"testing"
1314
"time"
1415

1516
"github.com/pkg/errors"
1617
"github.com/smallstep/certificates/api"
1718
"github.com/smallstep/certificates/authority"
19+
"github.com/smallstep/certificates/errs"
1820
"go.step.sm/crypto/jose"
1921
"go.step.sm/crypto/randutil"
2022
)
@@ -74,6 +76,30 @@ func startCAServer(configFile string) (*CA, string, error) {
7476
return ca, caURL, nil
7577
}
7678

79+
func mTLSMiddleware(next http.Handler, nonAuthenticatedPaths ...string) http.Handler {
80+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
81+
if r.URL.Path == "/version" {
82+
api.JSON(w, api.VersionResponse{
83+
Version: "test",
84+
RequireClientAuthentication: true,
85+
})
86+
return
87+
}
88+
89+
for _, s := range nonAuthenticatedPaths {
90+
if strings.HasPrefix(r.URL.Path, s) || strings.HasPrefix(r.URL.Path, "/1.0"+s) {
91+
next.ServeHTTP(w, r)
92+
}
93+
}
94+
isMTLS := r.TLS != nil && len(r.TLS.PeerCertificates) > 0
95+
if !isMTLS {
96+
api.WriteError(w, errs.Unauthorized("missing peer certificate"))
97+
} else {
98+
next.ServeHTTP(w, r)
99+
}
100+
})
101+
}
102+
77103
func generateBootstrapToken(ca, subject, sha string) string {
78104
now := time.Now()
79105
jwk, err := jose.ReadKey("testdata/secrets/ott_mariano_priv.jwk", jose.WithPassword([]byte("password")))
@@ -171,6 +197,15 @@ func TestBootstrapServerWithoutMTLS(t *testing.T) {
171197
token := func() string {
172198
return generateBootstrapToken(srv.URL, "subject", "ef742f95dc0d8aa82d3cca4017af6dac3fce84290344159891952d18c53eefe7")
173199
}
200+
201+
mtlsServer := startCABootstrapServer()
202+
next := mtlsServer.Config.Handler
203+
mtlsServer.Config.Handler = mTLSMiddleware(next, "/root/", "/sign")
204+
defer mtlsServer.Close()
205+
mtlsToken := func() string {
206+
return generateBootstrapToken(mtlsServer.URL, "subject", "ef742f95dc0d8aa82d3cca4017af6dac3fce84290344159891952d18c53eefe7")
207+
}
208+
174209
type args struct {
175210
ctx context.Context
176211
token string
@@ -182,6 +217,7 @@ func TestBootstrapServerWithoutMTLS(t *testing.T) {
182217
wantErr bool
183218
}{
184219
{"ok", args{context.Background(), token(), &http.Server{}}, false},
220+
{"ok mtls", args{context.Background(), mtlsToken(), &http.Server{}}, false},
185221
{"fail", args{context.Background(), "bad-token", &http.Server{}}, true},
186222
{"fail with TLSConfig", args{context.Background(), token(), &http.Server{TLSConfig: &tls.Config{}}}, true},
187223
}
@@ -217,6 +253,15 @@ func TestBootstrapServerWithMTLS(t *testing.T) {
217253
token := func() string {
218254
return generateBootstrapToken(srv.URL, "subject", "ef742f95dc0d8aa82d3cca4017af6dac3fce84290344159891952d18c53eefe7")
219255
}
256+
257+
mtlsServer := startCABootstrapServer()
258+
next := mtlsServer.Config.Handler
259+
mtlsServer.Config.Handler = mTLSMiddleware(next, "/root/", "/sign")
260+
defer mtlsServer.Close()
261+
mtlsToken := func() string {
262+
return generateBootstrapToken(mtlsServer.URL, "subject", "ef742f95dc0d8aa82d3cca4017af6dac3fce84290344159891952d18c53eefe7")
263+
}
264+
220265
type args struct {
221266
ctx context.Context
222267
token string
@@ -228,6 +273,7 @@ func TestBootstrapServerWithMTLS(t *testing.T) {
228273
wantErr bool
229274
}{
230275
{"ok", args{context.Background(), token(), &http.Server{}}, false},
276+
{"ok mtls", args{context.Background(), mtlsToken(), &http.Server{}}, false},
231277
{"fail", args{context.Background(), "bad-token", &http.Server{}}, true},
232278
{"fail with TLSConfig", args{context.Background(), token(), &http.Server{TLSConfig: &tls.Config{}}}, true},
233279
}
@@ -263,6 +309,15 @@ func TestBootstrapClient(t *testing.T) {
263309
token := func() string {
264310
return generateBootstrapToken(srv.URL, "subject", "ef742f95dc0d8aa82d3cca4017af6dac3fce84290344159891952d18c53eefe7")
265311
}
312+
313+
mtlsServer := startCABootstrapServer()
314+
next := mtlsServer.Config.Handler
315+
mtlsServer.Config.Handler = mTLSMiddleware(next, "/root/", "/sign")
316+
defer mtlsServer.Close()
317+
mtlsToken := func() string {
318+
return generateBootstrapToken(mtlsServer.URL, "subject", "ef742f95dc0d8aa82d3cca4017af6dac3fce84290344159891952d18c53eefe7")
319+
}
320+
266321
type args struct {
267322
ctx context.Context
268323
token string
@@ -273,6 +328,7 @@ func TestBootstrapClient(t *testing.T) {
273328
wantErr bool
274329
}{
275330
{"ok", args{context.Background(), token()}, false},
331+
{"ok mtls", args{context.Background(), mtlsToken()}, false},
276332
{"fail", args{context.Background(), "bad-token"}, true},
277333
}
278334
for _, tt := range tests {
@@ -541,6 +597,15 @@ func TestBootstrapListener(t *testing.T) {
541597
token := func() string {
542598
return generateBootstrapToken(srv.URL, "127.0.0.1", "ef742f95dc0d8aa82d3cca4017af6dac3fce84290344159891952d18c53eefe7")
543599
}
600+
601+
mtlsServer := startCABootstrapServer()
602+
next := mtlsServer.Config.Handler
603+
mtlsServer.Config.Handler = mTLSMiddleware(next, "/root/", "/sign")
604+
defer mtlsServer.Close()
605+
mtlsToken := func() string {
606+
return generateBootstrapToken(mtlsServer.URL, "subject", "ef742f95dc0d8aa82d3cca4017af6dac3fce84290344159891952d18c53eefe7")
607+
}
608+
544609
type args struct {
545610
token string
546611
}
@@ -550,6 +615,7 @@ func TestBootstrapListener(t *testing.T) {
550615
wantErr bool
551616
}{
552617
{"ok", args{token()}, false},
618+
{"ok mtls", args{mtlsToken()}, false},
553619
{"fail", args{"bad-token"}, true},
554620
}
555621
for _, tt := range tests {

Diff for: ‎ca/tls_options.go

+2
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ func AddRootCA(cert *x509.Certificate) TLSOption {
115115
if ctx.Config.RootCAs == nil {
116116
ctx.Config.RootCAs = x509.NewCertPool()
117117
}
118+
ctx.hasRootCA = true
118119
ctx.Config.RootCAs.AddCert(cert)
119120
ctx.mutableConfig.AddImmutableRootCACert(cert)
120121
return nil
@@ -129,6 +130,7 @@ func AddClientCA(cert *x509.Certificate) TLSOption {
129130
if ctx.Config.ClientCAs == nil {
130131
ctx.Config.ClientCAs = x509.NewCertPool()
131132
}
133+
ctx.hasClientCA = true
132134
ctx.Config.ClientCAs.AddCert(cert)
133135
ctx.mutableConfig.AddImmutableClientCACert(cert)
134136
return nil

0 commit comments

Comments
 (0)