-
Notifications
You must be signed in to change notification settings - Fork 217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OCPBUGS-35231: cert-inspection: parse secret/configmap/files as kubeconfigs #1746
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,40 +6,34 @@ import ( | |
"github.com/openshift/library-go/pkg/certs/cert-inspection/certgraphapi" | ||
certificatesv1 "k8s.io/api/certificates/v1" | ||
corev1 "k8s.io/api/core/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/client-go/rest" | ||
"k8s.io/client-go/tools/clientcmd" | ||
clientcmdapi "k8s.io/client-go/tools/clientcmd/api" | ||
"k8s.io/client-go/util/cert" | ||
) | ||
|
||
func InspectSecret(obj *corev1.Secret) (*certgraphapi.CertKeyPair, error) { | ||
resourceString := fmt.Sprintf("secrets/%s[%s]", obj.Name, obj.Namespace) | ||
func InspectSecret(obj *corev1.Secret) ([]*certgraphapi.CertKeyPair, error) { | ||
tlsCrt, isTLS := obj.Data["tls.crt"] | ||
if !isTLS { | ||
return nil, nil | ||
} | ||
//fmt.Printf("%s - tls (%v)\n", resourceString, obj.CreationTimestamp.UTC()) | ||
if len(tlsCrt) == 0 { | ||
return nil, fmt.Errorf("%s MISSING tls.crt content\n", resourceString) | ||
} | ||
|
||
certificates, err := cert.ParseCertsPEM([]byte(tlsCrt)) | ||
if err != nil { | ||
return nil, err | ||
} | ||
for _, certificate := range certificates { | ||
detail, err := toCertKeyPair(certificate) | ||
if err != nil { | ||
return nil, err | ||
if !isTLS || len(tlsCrt) == 0 { | ||
if detail, err := InspectSecretAsKubeConfig(obj); err == nil { | ||
return detail, nil | ||
} | ||
detail = addSecretLocation(detail, obj.Namespace, obj.Name) | ||
return detail, nil | ||
return nil, nil | ||
} | ||
return nil, fmt.Errorf("didn't see that coming") | ||
return extractCertKeyPairsFromBytes("secret", &obj.ObjectMeta, tlsCrt) | ||
} | ||
|
||
func inspectCSR(resourceString, objName string, certificate []byte) (*certgraphapi.CertKeyPair, error) { | ||
func extractCertKeyPairsFromBytes(resourceType string, obj *metav1.ObjectMeta, certificate []byte) ([]*certgraphapi.CertKeyPair, error) { | ||
resourceString := "" | ||
if obj != nil { | ||
resourceString = fmt.Sprintf("%s/%s[%s]", resourceType, obj.Name, obj.Namespace) | ||
} | ||
if len(certificate) == 0 { | ||
return nil, fmt.Errorf("%s MISSING issued certificate\n", resourceString) | ||
} | ||
|
||
certKeyPairDetails := []*certgraphapi.CertKeyPair{} | ||
certificates, err := cert.ParseCertsPEM([]byte(certificate)) | ||
if err != nil { | ||
return nil, err | ||
|
@@ -49,34 +43,124 @@ func inspectCSR(resourceString, objName string, certificate []byte) (*certgrapha | |
if err != nil { | ||
return nil, err | ||
} | ||
return detail, nil | ||
if resourceType == "secret" && obj != nil { | ||
detail = addSecretLocation(detail, obj.Namespace, obj.Name) | ||
} | ||
certKeyPairDetails = append(certKeyPairDetails, detail) | ||
} | ||
return nil, fmt.Errorf("didn't see that coming") | ||
return certKeyPairDetails, nil | ||
} | ||
|
||
func InspectCSR(obj *certificatesv1.CertificateSigningRequest) (*certgraphapi.CertKeyPair, error) { | ||
resourceString := fmt.Sprintf("csr/%s[%s]", obj.Name, obj.Namespace) | ||
return inspectCSR(resourceString, obj.Name, obj.Status.Certificate) | ||
func InspectCSR(obj *certificatesv1.CertificateSigningRequest) ([]*certgraphapi.CertKeyPair, error) { | ||
return extractCertKeyPairsFromBytes("csr", &obj.ObjectMeta, obj.Status.Certificate) | ||
} | ||
|
||
func InspectConfigMap(obj *corev1.ConfigMap) (*certgraphapi.CertificateAuthorityBundle, error) { | ||
caBundle, ok := obj.Data["ca-bundle.crt"] | ||
if !ok { | ||
return nil, nil | ||
if details, err := InspectConfigMapAsKubeConfig(obj); err == nil { | ||
return details, nil | ||
} | ||
if len(caBundle) == 0 { | ||
|
||
caBundle, ok := obj.Data["ca-bundle.crt"] | ||
if !ok || len(caBundle) == 0 { | ||
return nil, nil | ||
} | ||
|
||
certificates, err := cert.ParseCertsPEM([]byte(caBundle)) | ||
if err != nil { | ||
return nil, err | ||
return nil, nil | ||
} | ||
caBundleDetail, err := toCABundle(certificates) | ||
if err != nil { | ||
return nil, err | ||
} | ||
caBundleDetail = addConfigMapLocation(caBundleDetail, obj.Namespace, obj.Name) | ||
return caBundleDetail, nil | ||
} | ||
|
||
func extractKubeConfigFromConfigMap(obj *corev1.ConfigMap) (*rest.Config, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might be more useful to return the actual kubeconfig type since it can contain multiple stanzas and rest.Config only holds one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea, I'll implement that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would require reworking entire There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
if obj == nil { | ||
return nil, fmt.Errorf("empty object") | ||
} | ||
for _, v := range obj.Data { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this appears to be parsing every key of potentially every configmap to see if it's a kubeconfig? Instead of doing this, could we choose specific keys we find valid? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I can modify this to a hardcoded list of keys, but I'm worried that it might miss some new kubeconfigs. |
||
kubeConfig, err := clientcmd.NewClientConfigFromBytes([]byte(v)) | ||
if err != nil { | ||
continue | ||
} | ||
if clientConfig, err := kubeConfig.ClientConfig(); err == nil { | ||
return clientConfig, nil | ||
} | ||
} | ||
return nil, nil | ||
} | ||
|
||
func extractKubeConfigFromSecret(obj *corev1.Secret) (*clientcmdapi.Config, error) { | ||
if obj == nil { | ||
return nil, fmt.Errorf("empty object") | ||
} | ||
for _, v := range obj.Data { | ||
clientConfig, err := clientcmd.NewClientConfigFromBytes(v) | ||
if err != nil { | ||
continue | ||
} | ||
if kubeConfig, err := clientConfig.RawConfig(); err == nil && kubeConfig.CurrentContext != "" { | ||
return &kubeConfig, nil | ||
} | ||
} | ||
return nil, nil | ||
} | ||
|
||
func GetCAFromKubeConfig(kubeConfig *rest.Config, namespace, name string) (*certgraphapi.CertificateAuthorityBundle, error) { | ||
if kubeConfig == nil { | ||
return nil, fmt.Errorf("empty kubeconfig") | ||
} | ||
certificates, err := cert.ParseCertsPEM(kubeConfig.CAData) | ||
if err != nil { | ||
return nil, nil | ||
} | ||
caBundleDetail, err := toCABundle(certificates) | ||
if err != nil { | ||
return nil, err | ||
} | ||
if len(namespace) > 0 && len(name) > 0 { | ||
caBundleDetail = addConfigMapLocation(caBundleDetail, namespace, name) | ||
} | ||
return caBundleDetail, nil | ||
} | ||
|
||
func GetCertKeyPairsFromKubeConfig(authInfo *clientcmdapi.AuthInfo, obj *metav1.ObjectMeta) ([]*certgraphapi.CertKeyPair, error) { | ||
if authInfo == nil { | ||
return nil, fmt.Errorf("empty authinfo") | ||
} | ||
return extractCertKeyPairsFromBytes("secret", obj, authInfo.ClientCertificateData) | ||
} | ||
|
||
func InspectConfigMapAsKubeConfig(obj *corev1.ConfigMap) (*certgraphapi.CertificateAuthorityBundle, error) { | ||
kubeConfig, err := extractKubeConfigFromConfigMap(obj) | ||
if err != nil { | ||
return nil, err | ||
} | ||
if kubeConfig == nil { | ||
return nil, fmt.Errorf("empty kubeconfig") | ||
} | ||
|
||
return GetCAFromKubeConfig(kubeConfig, obj.Namespace, obj.Name) | ||
} | ||
|
||
func InspectSecretAsKubeConfig(obj *corev1.Secret) ([]*certgraphapi.CertKeyPair, error) { | ||
kubeConfig, err := extractKubeConfigFromSecret(obj) | ||
if err != nil { | ||
return nil, err | ||
} | ||
if kubeConfig == nil { | ||
return nil, fmt.Errorf("empty kubeconfig") | ||
} | ||
certKeyPairInfos := []*certgraphapi.CertKeyPair{} | ||
for _, v := range kubeConfig.AuthInfos { | ||
certKeyPairInfo, err := GetCertKeyPairsFromKubeConfig(v, &obj.ObjectMeta) | ||
if err != nil { | ||
continue | ||
} | ||
certKeyPairInfos = append(certKeyPairInfos, certKeyPairInfo...) | ||
} | ||
return certKeyPairInfos, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be useful to somehow log that this has been attempted and an error occurred?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any error is treated as critical here, so "collect TLS artifacts" test would fail if errors found. Also, any valid certificate would be treated as invalid kubeconfig, so we should continue instead of returning error.