Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 84488c9

Browse filesBrowse files
committed
fix overlapping client CA and requestheader CA validation with proper certificate checking
fix error message
1 parent 9a8c2a9 commit 84488c9
Copy full SHA for 84488c9

File tree

Expand file treeCollapse file tree

4 files changed

+87
-0
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+87
-0
lines changed

‎pkg/kubeapiserver/options/authentication.go

Copy file name to clipboardExpand all lines: pkg/kubeapiserver/options/authentication.go
+30Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package options
1818

1919
import (
2020
"context"
21+
"crypto/x509"
2122
"errors"
2223
"fmt"
2324
"net/url"
@@ -50,6 +51,7 @@ import (
5051
"k8s.io/client-go/informers"
5152
"k8s.io/client-go/kubernetes"
5253
v1listers "k8s.io/client-go/listers/core/v1"
54+
certutil "k8s.io/client-go/util/cert"
5355
"k8s.io/client-go/util/keyutil"
5456
cliflag "k8s.io/component-base/cli/flag"
5557
"k8s.io/klog/v2"
@@ -307,11 +309,39 @@ func (o *BuiltInAuthenticationOptions) Validate() []error {
307309

308310
if o.RequestHeader != nil {
309311
allErrors = append(allErrors, o.RequestHeader.Validate()...)
312+
313+
if o.ClientCert != nil &&
314+
len(o.ClientCert.ClientCA) > 0 &&
315+
len(o.RequestHeader.ClientCAFile) > 0 &&
316+
len(o.RequestHeader.AllowedNames) == 0 {
317+
clientCACerts, err1 := certutil.CertsFromFile(o.ClientCert.ClientCA)
318+
requestHeaderCACerts, err2 := certutil.CertsFromFile(o.RequestHeader.ClientCAFile)
319+
if err1 == nil && err2 == nil {
320+
if certificatesOverlap(clientCACerts, requestHeaderCACerts) {
321+
allErrors = append(allErrors,
322+
fmt.Errorf("--requestheader-client-ca-file and --client-ca-file contain overlapping certificates; --requestheader-allowed-names must be specified to ensure regular client certificates cannot set authenticating proxy headers for arbitrary users"))
323+
}
324+
}
325+
}
326+
310327
}
311328

312329
return allErrors
313330
}
314331

332+
// certificatesOverlap returns true when there's at least one identical
333+
// certificate in the two certificate bundles
334+
func certificatesOverlap(a, b []*x509.Certificate) bool {
335+
for _, ca := range a {
336+
for _, cb := range b {
337+
if ca.Equal(cb) {
338+
return true
339+
}
340+
}
341+
}
342+
return false
343+
}
344+
315345
// AddFlags returns flags of authentication for a API Server
316346
func (o *BuiltInAuthenticationOptions) AddFlags(fs *pflag.FlagSet) {
317347
if o == nil {

‎pkg/kubeapiserver/options/authentication_test.go

Copy file name to clipboardExpand all lines: pkg/kubeapiserver/options/authentication_test.go
+37Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"encoding/pem"
2525
"fmt"
2626
"os"
27+
"path/filepath"
2728
"reflect"
2829
"strings"
2930
"syscall"
@@ -55,8 +56,12 @@ import (
5556
)
5657

5758
func TestAuthenticationValidate(t *testing.T) {
59+
testDataDir := filepath.Join("testdata")
60+
5861
testCases := []struct {
5962
name string
63+
clientCert *apiserveroptions.ClientCertAuthenticationOptions
64+
requestHeader *apiserveroptions.RequestHeaderAuthenticationOptions
6065
testAnonymous *AnonymousAuthenticationOptions
6166
testOIDC *OIDCAuthenticationOptions
6267
testSA *ServiceAccountAuthenticationOptions
@@ -251,11 +256,43 @@ func TestAuthenticationValidate(t *testing.T) {
251256
FlagsSet: true,
252257
},
253258
},
259+
{
260+
name: "overlapping CA without allowed-names",
261+
clientCert: &apiserveroptions.ClientCertAuthenticationOptions{ClientCA: filepath.Join(testDataDir, "client-ca.pem")},
262+
requestHeader: &apiserveroptions.RequestHeaderAuthenticationOptions{
263+
ClientCAFile: filepath.Join(testDataDir, "client-ca.pem"),
264+
AllowedNames: []string{},
265+
UsernameHeaders: []string{"X-Remote-User"},
266+
},
267+
expectErr: "--requestheader-client-ca-file and --client-ca-file contain overlapping certificates; --requestheader-allowed-names must be specified to ensure regular client certificates cannot set authenticating proxy headers for arbitrary users",
268+
},
269+
{
270+
name: "overlapping CA with allowed-names",
271+
clientCert: &apiserveroptions.ClientCertAuthenticationOptions{ClientCA: filepath.Join(testDataDir, "client-ca.pem")},
272+
requestHeader: &apiserveroptions.RequestHeaderAuthenticationOptions{
273+
ClientCAFile: filepath.Join(testDataDir, "client-ca.pem"),
274+
AllowedNames: []string{"Client-CA"},
275+
UsernameHeaders: []string{"X-Remote-User"},
276+
},
277+
expectErr: "",
278+
},
279+
{
280+
name: "different CAs without allowed-names",
281+
clientCert: &apiserveroptions.ClientCertAuthenticationOptions{ClientCA: filepath.Join(testDataDir, "client-ca.pem")},
282+
requestHeader: &apiserveroptions.RequestHeaderAuthenticationOptions{
283+
ClientCAFile: filepath.Join(testDataDir, "server-ca.pem"),
284+
AllowedNames: []string{},
285+
UsernameHeaders: []string{"X-Remote-User"},
286+
},
287+
expectErr: "",
288+
},
254289
}
255290

256291
for _, testcase := range testCases {
257292
t.Run(testcase.name, func(t *testing.T) {
258293
options := NewBuiltInAuthenticationOptions()
294+
options.ClientCert = testcase.clientCert
295+
options.RequestHeader = testcase.requestHeader
259296
options.Anonymous = testcase.testAnonymous
260297
options.OIDC = testcase.testOIDC
261298
options.ServiceAccounts = testcase.testSA
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIBbTCCARSgAwIBAgIUDMmq4/Gw2N1o5TWBLWsm65RiVkIwCgYIKoZIzj0EAwIw
3+
FDESMBAGA1UEAxMJQ2xpZW50LUNBMCAXDTIxMDUyMjIzNTIwMFoYDzIxMjEwNDI4
4+
MjM1MjAwWjAUMRIwEAYDVQQDEwlDbGllbnQtQ0EwWTATBgcqhkjOPQIBBggqhkjO
5+
PQMBBwNCAAQDhCqK/KktlUtqgVgBLRRbJ/I1FJdG2AxYRpu+ygc7fUJFrZlLebyC
6+
U5DbVovKxV0Xq8A/fU71+rKy4YybRQjvo0IwQDAOBgNVHQ8BAf8EBAMCAQYwDwYD
7+
VR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUaDl2pG6N7NoORQjpHprKDSOL8+0wCgYI
8+
KoZIzj0EAwIDRwAwRAIgbS1tdj6El37kUwF9yZDXKfjLUlRBBLmIYhP0mdui6/AC
9+
IB4F/weuM/6IjCdcPJRxvdC7qjCdV0xnFqvQ+BhuUGSF
10+
-----END CERTIFICATE-----
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIBbzCCARSgAwIBAgIUf0aG2C1P7KaDGobg9oeN3uhQlu4wCgYIKoZIzj0EAwIw
3+
FDESMBAGA1UEAxMJU2VydmVyLUNBMCAXDTIxMDUyMjIzNTIwMFoYDzIxMjEwNDI4
4+
MjM1MjAwWjAUMRIwEAYDVQQDEwlTZXJ2ZXItQ0EwWTATBgcqhkjOPQIBBggqhkjO
5+
PQMBBwNCAAQ/DG/wiOR9TkCK9wrQiK6scv0foSIaH7NnRLyv48FbQNcU9dwCNBy0
6+
TyBUe7d+n3TLUlVOuJrGuJT/Hwtuunxko0IwQDAOBgNVHQ8BAf8EBAMCAQYwDwYD
7+
VR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUjcdIlU1vGLSUWBcSqCEJTgqlSacwCgYI
8+
KoZIzj0EAwIDSQAwRgIhAIujFeJKprddp+9aCZZUv05jCS5JiopW2bn/FJJRQ6OK
9+
AiEA1NS6trAbfgk6vYS2D2vamuF4XC9LggyxbcoaMf+GAn4=
10+
-----END CERTIFICATE-----

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.