fix overlapping client CA and requestheader CA validation with proper certificate checking

fix error message

fix lint
This commit is contained in:
Wenxue Zhao 2025-04-22 21:58:28 -04:00
parent da05e3ccc7
commit d9fb647515
4 changed files with 85 additions and 0 deletions

View file

@ -18,6 +18,7 @@ package options
import (
"context"
"crypto/x509"
"errors"
"fmt"
"net/url"
@ -50,6 +51,7 @@ import (
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
v1listers "k8s.io/client-go/listers/core/v1"
certutil "k8s.io/client-go/util/cert"
"k8s.io/client-go/util/keyutil"
cliflag "k8s.io/component-base/cli/flag"
"k8s.io/klog/v2"
@ -307,11 +309,39 @@ func (o *BuiltInAuthenticationOptions) Validate() []error {
if o.RequestHeader != nil {
allErrors = append(allErrors, o.RequestHeader.Validate()...)
if o.ClientCert != nil &&
len(o.ClientCert.ClientCA) > 0 &&
len(o.RequestHeader.ClientCAFile) > 0 &&
len(o.RequestHeader.AllowedNames) == 0 {
clientCACerts, err1 := certutil.CertsFromFile(o.ClientCert.ClientCA)
requestHeaderCACerts, err2 := certutil.CertsFromFile(o.RequestHeader.ClientCAFile)
if err1 == nil && err2 == nil {
if certificatesOverlap(clientCACerts, requestHeaderCACerts) {
allErrors = append(allErrors,
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"))
}
}
}
}
return allErrors
}
// certificatesOverlap returns true when there's at least one identical
// certificate in the two certificate bundles
func certificatesOverlap(a, b []*x509.Certificate) bool {
for _, ca := range a {
for _, cb := range b {
if ca.Equal(cb) {
return true
}
}
}
return false
}
// AddFlags returns flags of authentication for a API Server
func (o *BuiltInAuthenticationOptions) AddFlags(fs *pflag.FlagSet) {
if o == nil {

View file

@ -24,6 +24,7 @@ import (
"encoding/pem"
"fmt"
"os"
"path/filepath"
"reflect"
goruntime "runtime"
"strings"
@ -58,6 +59,8 @@ import (
func TestAuthenticationValidate(t *testing.T) {
testCases := []struct {
name string
clientCert *apiserveroptions.ClientCertAuthenticationOptions
requestHeader *apiserveroptions.RequestHeaderAuthenticationOptions
testAnonymous *AnonymousAuthenticationOptions
testOIDC *OIDCAuthenticationOptions
testSA *ServiceAccountAuthenticationOptions
@ -256,11 +259,43 @@ func TestAuthenticationValidate(t *testing.T) {
// feature-gate, otherwise this feature-gate cannot be disabled.
emulationVersion: version.MustParse("1.33"),
},
{
name: "overlapping CA without allowed-names",
clientCert: &apiserveroptions.ClientCertAuthenticationOptions{ClientCA: filepath.Join("testdata", "client-ca.pem")},
requestHeader: &apiserveroptions.RequestHeaderAuthenticationOptions{
ClientCAFile: filepath.Join("testdata", "client-ca.pem"),
AllowedNames: []string{},
UsernameHeaders: []string{"X-Remote-User"},
},
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",
},
{
name: "overlapping CA with allowed-names",
clientCert: &apiserveroptions.ClientCertAuthenticationOptions{ClientCA: filepath.Join("testdata", "client-ca.pem")},
requestHeader: &apiserveroptions.RequestHeaderAuthenticationOptions{
ClientCAFile: filepath.Join("testdata", "client-ca.pem"),
AllowedNames: []string{"Client-CA"},
UsernameHeaders: []string{"X-Remote-User"},
},
expectErr: "",
},
{
name: "different CAs without allowed-names",
clientCert: &apiserveroptions.ClientCertAuthenticationOptions{ClientCA: filepath.Join("testdata", "client-ca.pem")},
requestHeader: &apiserveroptions.RequestHeaderAuthenticationOptions{
ClientCAFile: filepath.Join("testdata", "server-ca.pem"),
AllowedNames: []string{},
UsernameHeaders: []string{"X-Remote-User"},
},
expectErr: "",
},
}
for _, testcase := range testCases {
t.Run(testcase.name, func(t *testing.T) {
options := NewBuiltInAuthenticationOptions()
options.ClientCert = testcase.clientCert
options.RequestHeader = testcase.requestHeader
options.Anonymous = testcase.testAnonymous
options.OIDC = testcase.testOIDC
options.ServiceAccounts = testcase.testSA

View file

@ -0,0 +1,10 @@
-----BEGIN CERTIFICATE-----
MIIBbTCCARSgAwIBAgIUDMmq4/Gw2N1o5TWBLWsm65RiVkIwCgYIKoZIzj0EAwIw
FDESMBAGA1UEAxMJQ2xpZW50LUNBMCAXDTIxMDUyMjIzNTIwMFoYDzIxMjEwNDI4
MjM1MjAwWjAUMRIwEAYDVQQDEwlDbGllbnQtQ0EwWTATBgcqhkjOPQIBBggqhkjO
PQMBBwNCAAQDhCqK/KktlUtqgVgBLRRbJ/I1FJdG2AxYRpu+ygc7fUJFrZlLebyC
U5DbVovKxV0Xq8A/fU71+rKy4YybRQjvo0IwQDAOBgNVHQ8BAf8EBAMCAQYwDwYD
VR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUaDl2pG6N7NoORQjpHprKDSOL8+0wCgYI
KoZIzj0EAwIDRwAwRAIgbS1tdj6El37kUwF9yZDXKfjLUlRBBLmIYhP0mdui6/AC
IB4F/weuM/6IjCdcPJRxvdC7qjCdV0xnFqvQ+BhuUGSF
-----END CERTIFICATE-----

View file

@ -0,0 +1,10 @@
-----BEGIN CERTIFICATE-----
MIIBbzCCARSgAwIBAgIUf0aG2C1P7KaDGobg9oeN3uhQlu4wCgYIKoZIzj0EAwIw
FDESMBAGA1UEAxMJU2VydmVyLUNBMCAXDTIxMDUyMjIzNTIwMFoYDzIxMjEwNDI4
MjM1MjAwWjAUMRIwEAYDVQQDEwlTZXJ2ZXItQ0EwWTATBgcqhkjOPQIBBggqhkjO
PQMBBwNCAAQ/DG/wiOR9TkCK9wrQiK6scv0foSIaH7NnRLyv48FbQNcU9dwCNBy0
TyBUe7d+n3TLUlVOuJrGuJT/Hwtuunxko0IwQDAOBgNVHQ8BAf8EBAMCAQYwDwYD
VR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUjcdIlU1vGLSUWBcSqCEJTgqlSacwCgYI
KoZIzj0EAwIDSQAwRgIhAIujFeJKprddp+9aCZZUv05jCS5JiopW2bn/FJJRQ6OK
AiEA1NS6trAbfgk6vYS2D2vamuF4XC9LggyxbcoaMf+GAn4=
-----END CERTIFICATE-----