Skip to content
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

[release-4.5] Bug 1835090: Collect certificates #70

Merged
merged 4 commits into from
Feb 19, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
/*.pprof
main
.vscode/
2 changes: 1 addition & 1 deletion pkg/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func (s *Support) Run(ctx context.Context, controller *controllercmd.ControllerC

// the gatherers periodically check the state of the cluster and report any
// config to the recorder
configPeriodic := clusterconfig.New(gatherConfigClient, gatherKubeClient.CoreV1(), metricsClient)
configPeriodic := clusterconfig.New(gatherConfigClient, gatherKubeClient.CoreV1(), gatherKubeClient.CertificatesV1beta1(), metricsClient)
periodic := periodic.New(configObserver, recorder, map[string]gather.Interface{
"config": configPeriodic,
})
Expand Down
33 changes: 25 additions & 8 deletions pkg/gather/clusterconfig/clusterconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import (
"sync"
"time"

configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/client-go/config/clientset/versioned/scheme"
configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
_ "k8s.io/client-go/kubernetes"
certificatesv1beta1 "k8s.io/client-go/kubernetes/typed/certificates/v1beta1"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -20,10 +26,6 @@ import (
"k8s.io/client-go/rest"
"k8s.io/klog"

configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/client-go/config/clientset/versioned/scheme"
configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"

"github.com/openshift/insights-operator/pkg/record"
)

Expand All @@ -40,15 +42,16 @@ type Gatherer struct {
client configv1client.ConfigV1Interface
coreClient corev1client.CoreV1Interface
metricsClient rest.Interface

lock sync.Mutex
lastVersion *configv1.ClusterVersion
certClient certificatesv1beta1.CertificatesV1beta1Interface
lock sync.Mutex
lastVersion *configv1.ClusterVersion
}

func New(client configv1client.ConfigV1Interface, coreClient corev1client.CoreV1Interface, metricsClient rest.Interface) *Gatherer {
func New(client configv1client.ConfigV1Interface, coreClient corev1client.CoreV1Interface, certClient certificatesv1beta1.CertificatesV1beta1Interface, metricsClient rest.Interface) *Gatherer {
return &Gatherer{
client: client,
coreClient: coreClient,
certClient: certClient,
metricsClient: metricsClient,
}
}
Expand Down Expand Up @@ -223,6 +226,20 @@ func (i *Gatherer) Gather(ctx context.Context, recorder record.Interface) error
}
return []record.Record{{Name: "config/proxy", Item: ProxyAnonymizer{config}}}, nil
},
func() ([]record.Record, []error) {
requests, err := i.certClient.CertificateSigningRequests().List(metav1.ListOptions{})
if errors.IsNotFound(err) {
return nil, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is used in previous gathering functions, but is the nil handled properly later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fortunately returning nil in a slice will return empty slice and wont fail. It looks even empty Record wont fail as its just a structure. I have just verified both.

}
if err != nil {
return nil, []error{err}
}
records := make([]record.Record, len(requests.Items))
for i, sr := range requests.Items {
records[i] = record.Record{Name: fmt.Sprintf("config/certificatesigningrequests/%s", sr.Name), Item: CSRAnonymizer{&sr}}
}
return records, nil
},
)
}

Expand Down
185 changes: 185 additions & 0 deletions pkg/gather/clusterconfig/csr.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
package clusterconfig

import (
"context"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"fmt"
"time"

"github.com/openshift/insights-operator/pkg/utils"
"k8s.io/api/certificates/v1beta1"
certificatesv1b1api "k8s.io/api/certificates/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/json"
)

type CSRAnonymizer struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick - all public data types should have proper docstring

*certificatesv1b1api.CertificateSigningRequest
}

func (a CSRAnonymizer) Marshal(_ context.Context) ([]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick - all public methods should have proper docstring

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these comments are still pending, would be nice to add some docs in the whole file.

res, err := anonymizeCsr(a.CertificateSigningRequest)
if err != nil {
return nil, err
}

return json.Marshal(res)
}

func anonymizeCsrRequest(r *certificatesv1b1api.CertificateSigningRequest, c *CSRAnonymizedFeatures) {
if r == nil || c == nil {
return
}
c.Spec = &StateFeatures{}
c.Spec.Username = r.Spec.Username
c.Spec.Groups = r.Spec.Groups
c.Spec.Usages = r.Spec.Usages

// CSR in a PEM
// parse only first PEM block
block, _ := pem.Decode(r.Spec.Request)
if block == nil {
// unable to decode CSR: missing block
return
}
csr, err := x509.ParseCertificateRequest(block.Bytes)
if err != nil {
return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we need to log these issues?

}

err = csr.CheckSignature()
if err != nil {
return
}
c.Spec.Request = &CsrFeatures{}
c.Spec.Request.ValidSignature = err == nil
c.Spec.Request.Subject = anonymizePkxName(csr.Subject)

c.Spec.Request.SignatureAlgorithm = csr.SignatureAlgorithm.String()
c.Spec.Request.PublicKeyAlgorithm = csr.PublicKeyAlgorithm.String()
c.Spec.Request.DNSNames = utils.Map(csr.DNSNames, anonymizeURL)
c.Spec.Request.EmailAddresses = utils.Map(csr.EmailAddresses, anonymizeURL)
ipsl := []string{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we know len(csr.IPAddresses), you can allocate an array, but it's up to you

for _, ip := range csr.IPAddresses {
ipsl = append(ipsl, ip.String())
}
c.Spec.Request.IPAddresses = utils.Map(ipsl, anonymizeURL)
urlsl := []string{}
for _, u := range csr.URIs {
urlsl = append(urlsl, u.String())
}
c.Spec.Request.URIs = utils.Map(urlsl, anonymizeURL)
}

func anonymizePkxName(s pkix.Name) (a pkix.Name) {
its := func(n *pkix.Name) []interface{} {
return []interface{}{
&n.CommonName,
&n.Locality,
&n.Province,
&n.StreetAddress,
&n.PostalCode,
&n.Country,
&n.Organization,
&n.OrganizationalUnit,
&n.SerialNumber,
}
}

src := its(&s)
dst := its(&a)
for i := range src {
switch s := src[i].(type) {
case *string:
*(dst[i].(*string)) = anonymizeString(*s)
case *[]string:
*(dst[i].(*[]string)) = utils.Map(*s, anonymizeString)
default:
panic(fmt.Sprintf("unknown type %T", s))
}
}
return
}

// returns true if certificate is valid
func anonymizeCsrCert(r *certificatesv1b1api.CertificateSigningRequest, c *CSRAnonymizedFeatures) {
if r == nil || c == nil {
return
}
c.Status = &StatusFeatures{}
c.Status.Conditions = r.Status.Conditions
// Certificate PEM
// parse only first PEM block
block, _ := pem.Decode(r.Status.Certificate)
if block == nil {
// unable to decode CSR: missing block
return
}
cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
return
}
c.Status.Cert = &CertFeatures{}
c.Status.Cert.Issuer = anonymizePkxName(cert.Issuer)
c.Status.Cert.Subject = anonymizePkxName(cert.Subject)
c.Status.Cert.NotBefore = cert.NotBefore.Format(time.RFC3339)
c.Status.Cert.NotAfter = cert.NotAfter.Format(time.RFC3339)
}

func addMeta(r *certificatesv1b1api.CertificateSigningRequest, c *CSRAnonymizedFeatures) {
if r == nil || c == nil {
return
}
c.TypeMeta = r.TypeMeta
c.ObjectMeta = r.ObjectMeta
}

func anonymizeCsr(r *certificatesv1b1api.CertificateSigningRequest) (*CSRAnonymizedFeatures, error) {
c := &CSRAnonymizedFeatures{}
addMeta(r, c)
anonymizeCsrRequest(r, c)
anonymizeCsrCert(r, c)
return c, nil
}

type CSRAnonymizedFeatures struct {
TypeMeta metav1.TypeMeta
ObjectMeta metav1.ObjectMeta
Spec *StateFeatures
Status *StatusFeatures
}

type StateFeatures struct {
UID string
Username string
Groups []string
Usages []v1beta1.KeyUsage

Request *CsrFeatures
}

type StatusFeatures struct {
Conditions []v1beta1.CertificateSigningRequestCondition
Cert *CertFeatures
}

type CsrFeatures struct {
ValidSignature bool
SignatureAlgorithm string
PublicKeyAlgorithm string
DNSNames []string
EmailAddresses []string
IPAddresses []string
URIs []string
Subject pkix.Name
}

type CertFeatures struct {
Verified bool
Issuer pkix.Name
Subject pkix.Name
NotBefore string
NotAfter string
}
67 changes: 67 additions & 0 deletions pkg/gather/clusterconfig/csr_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package clusterconfig

import (
"encoding/json"
"io/ioutil"
"os"
"reflect"
"testing"

certificatesv1b1api "k8s.io/api/certificates/v1beta1"
)

func TestCSRs(t *testing.T) {
var files = []struct {
dataFile string
expFile string
}{
{"testdata/csr_appr.json", "testdata/csr_appr_anon.json"},
{"testdata/csr_unappr.json", "testdata/csr_unappr_anon.json"},
}

for _, tt := range files {
t.Run(tt.dataFile, func(t *testing.T) {

r := &certificatesv1b1api.CertificateSigningRequest{}

f, err := os.Open(tt.dataFile)
if err != nil {
t.Fatal("test failed to unmarshal csr data", err)
}
defer f.Close()
bts, err := ioutil.ReadAll(f)
if err != nil {
t.Fatal("error reading test data file", err)
}
err = json.Unmarshal([]byte(bts), r)
if err != nil {
t.Fatal("test failed to unmarshal csr data", err)
}
exp := &CSRAnonymizedFeatures{}

f, err = os.Open(tt.expFile)
if err != nil {
t.Fatal("test failed to unmarshal csr anonymized data", err)
}
defer f.Close()
bts, err = ioutil.ReadAll(f)
if err != nil {
t.Fatal("error reading test data file", err)
}
err = json.Unmarshal([]byte(bts), exp)
if err != nil {
t.Fatal("test failed to unmarshal anonymized csr data", err)
}

a, err := anonymizeCsr(r)
ss, _ := json.Marshal(a)
_ = ss
if err != nil {
t.Fatal("should not fail", err)
}
if !reflect.DeepEqual(exp, a) {
t.Fatal("Expected", exp, "but got", a)
}
})
}
}
38 changes: 38 additions & 0 deletions pkg/gather/clusterconfig/testdata/csr_appr.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"metadata": {
"name": "my-svc.my-namespace",
"selfLink": "/apis/certificates.k8s.io/v1beta1/certificatesigningrequests/my-svc.my-namespace",
"uid": "16a753e6-3e81-11ea-8a8c-52fdfc072182",
"resourceVersion": "187306",
"creationTimestamp": "2020-01-24T08:11:13Z"
},
"spec": {
"request": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURSBSRVFVRVNULS0tLS0KTUlJRWxEQ0NBbndDQVFBd1R6RUxNQWtHQTFVRUJoTUNXRmd4RlRBVEJnTlZCQWNNREVSbFptRjFiSFFnUTJsMAplVEVjTUJvR0ExVUVDZ3dUUkdWbVlYVnNkQ0JEYjIxd1lXNTVJRXgwWkRFTE1Ba0dBMVVFQXd3Q1FVRXdnZ0lpCk1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQ0R3QXdnZ0lLQW9JQ0FRQ2tSVFZQUlFGcTRxV2JINGFKc0gvUzFhUGwKTTdCYmhHUGcxbW1iVVJVMmtiTWFBR0J2djRRYkhxK0pySGl3aERyTVRFalZCTU1SQWVFTi85ZWdYQzZvc3NkbAowRFY5RzQzaTl3K1RxMmRmOGVpRXVHZ09TbVorRHl0MnBxTTV6aDFObml1Mm0xUGF2Y2gva2ViSCs4MXhpbXJxCkNNdTlxNVdmNXNlNlUxWUZVK2lvdjJpSGJxMWM1cjhNbzFTL040Z1JENWpMMXQzK0lkdmViRzZQUWJhUDVZNVAKMlNxaHdVV1Y3eUpVcE1hcWd1YkRwSlFJSGxqSy9JVm9SMWpDRjU3VkwxeTlsVFVsQUhuZHUyQkRJY3h6ZE5tVwphK2NQOWM2MGtXVHVKc09MRW05eTd5Y3lVcmpFOVlodHNyS0ovbDlRUThCRnA0WlliTWNNQkI1ak90dUVSbTNqCjN6cXN6WFZGWmdiOXFPYlFwWDhHK1V4bDNjZi9GNSt0d2Z0UEFzVHlGcHR3b29YOVhnOUpVUytLWElMaVUydTcKT0RVUHlEM2NwZ0pmajVhd1NydEw2VzBmQ2NubFdEUUpBdlBKaHc4QmsyRzhKWlA2ZVl0eUlxRElSdFhaNFgySwp2M2k3eVF6YlN6WkF4VHRGdXVxTU5QWWhpNDNVa0hmVXpMOWdJN1pBZ1JBNGpTa0pHTmNMNDVOdWNRSmlmcnAzCkdIVlBGZDdUOGpPNUVsbnZyQlVHb2FKQ1NUVXlNU2p1Tjg2RHpKc1ZmNXd2OUovTytyYTRMdnYvQ3R5OXNTOGgKWmpmVlVkdDBDV0V0OGRZV1B5NjVLcGdlOEZtaGg5dGVHY0dXQkJkSWFzZ2pqYlZ1ZnFoZEM3QXVBWGhZMG00TwppUXdScStiVVgvYXVkclhmVVFJREFRQUJvQUF3RFFZSktvWklodmNOQVFFTEJRQURnZ0lCQUQzT2VEeW1TWmwxCk90RG9lS05PQTRMUXBUSEtkMC9jK3BDTUY3OWdxRnl5LzMxYjlRMm9sTzM1dmNkZ2ZVTkxWU0xnWkc2aWFEcUsKTEIva2tzeDgwc0RxS2V2bUtSK2JSRjBGeWxGQ1pBUjVHZ3pJRWhLZXJpQU1ud252Tkl5cTBrTU40Qk4rSGUyVwphcGE2MUZnV2tGejVnMW9UUnh0Y1JKNTF3dFJrMjdZVGNBRlU5ZmFEc2pJY0xqNk5XUVNpN3JNeUxtZTh4eHNYCjlFOVVFUXJ5bTF0cG9wdlc1N1g0R3U4dFNCWkZ6QXJEU2VLaGN3QW8zNzBGY3ZUQytNSzNicVNzRlZWU0p1L2UKNGxlanRXSmpCUkwyRnFtenR3U0g0WW8xRmhRVHphaU94c25MNmVoMDdBRm5Mb3NYYUM4dDlqTTA0enFUQyt6Kwo5eXZ0bU1UMWhoZW9RUDIvYTV6Zjg4TmpvN3ZWdytMVmhnV2tsbnNOMDJoVzhYSXp5Y2xySU5XUXVpanhvRklTCnAyaUxreldGTzlDQWcyTHhHem1talhGQmtZdXFaWEpYMG1jOFZNejNoVktDUVVSTFZDWUhKU3lHaWVxYnlkVGcKcWxkazJRampXNXo0WGh2UzA0ZmhqaTd5NXRNR1pYbW1lNDhzWXFNRWptczVVL29LSkhoWHhzaGtDZnhkT0VTSAoyN3RjM2RZSGU3bDdnZFdjRGJlODNKSE5UdHFwM2tPaUQ3cUo1Q1MydGxZTEhxRmNyTmFWTTViYTN5eWllMG1OCktnWjc4U095YUc0Sm01UWZ5Q0V2SWt2REpTak9EVXQvQmtkdnF5clBBN2RpSW9NYldpVkpBTEVNUmFGeEo2bkcKcmFOWmdlc3dSNzNXM2pGdHQ4OW9Mdmc3NUpTNU43bFcKLS0tLS1FTkQgQ0VSVElGSUNBVEUgUkVRVUVTVC0tLS0tCg==",
"usages": [
"digital signature",
"key encipherment",
"server auth"
],
"username": "kube:admin",
"groups": [
"system:cluster-admins",
"system:authenticated"
],
"extra": {
"scopes.authorization.openshift.io": [
"user:full"
]
}
},
"status": {
"conditions": [
{
"type": "Approved",
"reason": "KubectlApprove",
"message": "This CSR was approved by kubectl certificate approve.",
"lastUpdateTime": "2020-01-24T13:22:22Z"
}
],
"certificate": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVWekNDQXorZ0F3SUJBZ0lVSUluNElDMEd2enRXcFBSYkJlaXBCSVRQSTFJd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0pqRWtNQ0lHQTFVRUF3d2JhM1ZpWlMxamMzSXRjMmxuYm1WeVgwQXhOVGM1TnpZNE1UUXhNQjRYRFRJdwpNREV5TkRFek1UY3dNRm9YRFRJd01ESXlNakE0TWpNek9Gb3dUekVMTUFrR0ExVUVCaE1DV0ZneEZUQVRCZ05WCkJBY1RERVJsWm1GMWJIUWdRMmwwZVRFY01Cb0dBMVVFQ2hNVFJHVm1ZWFZzZENCRGIyMXdZVzU1SUV4MFpERUwKTUFrR0ExVUVBeE1DUVVFd2dnSWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUNEd0F3Z2dJS0FvSUNBUUNrUlRWUApSUUZxNHFXYkg0YUpzSC9TMWFQbE03QmJoR1BnMW1tYlVSVTJrYk1hQUdCdnY0UWJIcStKckhpd2hEck1URWpWCkJNTVJBZUVOLzllZ1hDNm9zc2RsMERWOUc0M2k5dytUcTJkZjhlaUV1R2dPU21aK0R5dDJwcU01emgxTm5pdTIKbTFQYXZjaC9rZWJIKzgxeGltcnFDTXU5cTVXZjVzZTZVMVlGVStpb3YyaUhicTFjNXI4TW8xUy9ONGdSRDVqTAoxdDMrSWR2ZWJHNlBRYmFQNVk1UDJTcWh3VVdWN3lKVXBNYXFndWJEcEpRSUhsaksvSVZvUjFqQ0Y1N1ZMMXk5CmxUVWxBSG5kdTJCREljeHpkTm1XYStjUDljNjBrV1R1SnNPTEVtOXk3eWN5VXJqRTlZaHRzcktKL2w5UVE4QkYKcDRaWWJNY01CQjVqT3R1RVJtM2ozenFzelhWRlpnYjlxT2JRcFg4RytVeGwzY2YvRjUrdHdmdFBBc1R5RnB0dwpvb1g5WGc5SlVTK0tYSUxpVTJ1N09EVVB5RDNjcGdKZmo1YXdTcnRMNlcwZkNjbmxXRFFKQXZQSmh3OEJrMkc4CkpaUDZlWXR5SXFESVJ0WFo0WDJLdjNpN3lRemJTelpBeFR0RnV1cU1OUFloaTQzVWtIZlV6TDlnSTdaQWdSQTQKalNrSkdOY0w0NU51Y1FKaWZycDNHSFZQRmQ3VDhqTzVFbG52ckJVR29hSkNTVFV5TVNqdU44NkR6SnNWZjV3dgo5Si9PK3JhNEx2di9DdHk5c1M4aFpqZlZVZHQwQ1dFdDhkWVdQeTY1S3BnZThGbWhoOXRlR2NHV0JCZElhc2dqCmpiVnVmcWhkQzdBdUFYaFkwbTRPaVF3UnErYlVYL2F1ZHJYZlVRSURBUUFCbzFRd1VqQU9CZ05WSFE4QkFmOEUKQkFNQ0JhQXdFd1lEVlIwbEJBd3dDZ1lJS3dZQkJRVUhBd0V3REFZRFZSMFRBUUgvQkFJd0FEQWRCZ05WSFE0RQpGZ1FVTzcwTVlQY0dnWGZ0bnpxQThwTDE3TG1IV1Jvd0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQkFDM3U1MGtsCk5ESU1vZUNXQzdHRXRkaTcrNVBQL1A3R1ROTkxpR1hFZTdJL2lzNHpvQ0RlTzBPMEEzSUx6R2FlbHhCUmRsYzMKNFVMN3NDSjRhai9nOUtUakdNNzJGQllxVkNIUHBlKzhrZmdKQmdZcVV6d05vc2NaUXJ6YThaNXpXWFMwdnRLcgptbWRqZmZlT2Y2MFpPU3hjM0JtdWw5WU9aYmpFMG1sWGVHYzBSMm9jNS9jOXBTRXZ6QW5ocHZjMENGK29OblhICmtmRVBYZjFELzE0aUpnN1AyTG1PVkpQcWsrZVZ6eDgvVk45SmJTczN0TmtMTDBib2dudUR3RGF6NFZsNFpLankKVTAzNFdvY3prdmZxSmlUdmhENTV0eFRENDhsaGhqYUlyQ0V5eXR5TjllUlZmVDBiSkJ6OGxBRXRDVmorOEZMbApVcHRqVjZQUEFxdDBiSEU9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"
}
}