From 56ab827985a5c71248175d47e49cd0068ce80385 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Sat, 16 Jul 2022 16:40:25 +0200 Subject: [PATCH 1/3] reconciler/apiexport: avoid seconds long rsa4096 computation --- pkg/crypto/rand.go | 50 +++++++++++++++++++++++++ pkg/reconciler/apis/apiexport/crypto.go | 22 +++++------ 2 files changed, 59 insertions(+), 13 deletions(-) create mode 100644 pkg/crypto/rand.go diff --git a/pkg/crypto/rand.go b/pkg/crypto/rand.go new file mode 100644 index 00000000000..d5c8638aca3 --- /dev/null +++ b/pkg/crypto/rand.go @@ -0,0 +1,50 @@ +/* +Copyright 2022 The KCP Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package crypto + +import ( + "encoding/base64" + "math/rand" +) + +// RandomBits returns a random byte slice with at least the requested bits of entropy. +// Callers should avoid using a value less than 256 unless they have a very good reason. +func RandomBits(bits int) []byte { + size := bits / 8 + if bits%8 != 0 { + size++ + } + b := make([]byte, size) + if _, err := rand.Read(b); err != nil { + panic(err) // rand should never fail + } + return b +} + +// RandomBitsString returns a random string with at least the requested bits of entropy. +// It uses RawURLEncoding to ensure we do not get / characters or trailing ='s. +func RandomBitsString(bits int) string { + return base64.RawURLEncoding.EncodeToString(RandomBits(bits)) +} + +// Random256BitsString is a convenience function for calling RandomBitsString(256). +// Callers that need a random string should use this function unless they have a +// very good reason to need a different amount of entropy. +func Random256BitsString() string { + // 32 bytes (256 bits) = 43 base64-encoded characters + return RandomBitsString(256) +} diff --git a/pkg/reconciler/apis/apiexport/crypto.go b/pkg/reconciler/apis/apiexport/crypto.go index 54c66dd8a3a..45b6d08d6f3 100644 --- a/pkg/reconciler/apis/apiexport/crypto.go +++ b/pkg/reconciler/apis/apiexport/crypto.go @@ -17,27 +17,23 @@ limitations under the License. package apiexport import ( - cryptorand "crypto/rand" - "crypto/rsa" "crypto/sha256" "fmt" + "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/util/keyutil" + "k8s.io/klog/v2" apisv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1" + "github.com/kcp-dev/kcp/pkg/crypto" ) func GenerateIdentitySecret(ns string, apiExportName string) (*corev1.Secret, error) { - privateKey, err := rsa.GenerateKey(cryptorand.Reader, 4096) - if err != nil { - return nil, fmt.Errorf("error generating private key: %w", err) - } - - encoded, err := keyutil.MarshalPrivateKeyToPEM(privateKey) - if err != nil { - return nil, fmt.Errorf("error encoding private key: %w", err) + start := time.Now() + key := crypto.Random256BitsString() + if dur := time.Since(start); dur > time.Millisecond*100 { + klog.Warningf("identity key generation took long: %s", dur) } secret := &corev1.Secret{ @@ -45,8 +41,8 @@ func GenerateIdentitySecret(ns string, apiExportName string) (*corev1.Secret, er Namespace: ns, Name: apiExportName, }, - Data: map[string][]byte{ - apisv1alpha1.SecretKeyAPIExportIdentity: encoded, + StringData: map[string]string{ + apisv1alpha1.SecretKeyAPIExportIdentity: key, }, } From 057a50a85defd87bc8449a581fb01b26deb94dbc Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Sat, 16 Jul 2022 16:42:40 +0200 Subject: [PATCH 2/3] reconcilers: improve log strings --- pkg/reconciler/apis/apibinding/apibinding_reconcile.go | 2 +- pkg/reconciler/apis/apiexport/apiexport_reconcile.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/apis/apibinding/apibinding_reconcile.go b/pkg/reconciler/apis/apibinding/apibinding_reconcile.go index 7126a6be451..1d214a4ebf0 100644 --- a/pkg/reconciler/apis/apibinding/apibinding_reconcile.go +++ b/pkg/reconciler/apis/apibinding/apibinding_reconcile.go @@ -268,7 +268,7 @@ func (c *controller) reconcileBinding(ctx context.Context, apiBinding *apisv1alp if existingCRD == nil { // Create flow - klog.V(2).Infof("Creating CRD %s|%s for APIBinding %s|%s", ShadowWorkspaceName, crd.Name, logicalcluster.From(apiBinding), apiBinding.Name) + klog.V(2).Infof("Creating CRD %s|%s for APIBinding %s|%s resource %s.%s", ShadowWorkspaceName, crd.Name, logicalcluster.From(apiBinding), apiBinding.Name, crd.Spec.Names.Plural, crd.Spec.Group) if _, err := c.createCRD(ctx, ShadowWorkspaceName, crd); err != nil { schemaClusterName := logicalcluster.From(schema) if apierrors.IsInvalid(err) { diff --git a/pkg/reconciler/apis/apiexport/apiexport_reconcile.go b/pkg/reconciler/apis/apiexport/apiexport_reconcile.go index 0f36ee549d6..344e3fd6ec4 100644 --- a/pkg/reconciler/apis/apiexport/apiexport_reconcile.go +++ b/pkg/reconciler/apis/apiexport/apiexport_reconcile.go @@ -127,7 +127,7 @@ func (c *controller) createIdentitySecret(ctx context.Context, clusterName logic return err } - klog.V(2).Infof("Creating identity secret %s/%s for APIExport %s|%s", clusterName, secret.Name, clusterName, apiExportName) + klog.V(2).Infof("Creating identity secret %s|%s/%s for APIExport %s|%s", clusterName, c.secretNamespace, secret.Name, clusterName, apiExportName) if err := c.createSecret(ctx, clusterName, secret); err != nil { return err } From 46e8f92f8b1058ddd9f19b38679eb7a53427e805 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Sat, 16 Jul 2022 16:46:11 +0200 Subject: [PATCH 3/3] reconcilers/workload/apiexport: rename to unique filenames --- .../{apiexport_controller.go => workload_apiexport_controller.go} | 0 .../{apiexport_indexes.go => workload_apiexport_indexes.go} | 0 .../{apiexport_reconcile.go => workload_apiexport_reconcile.go} | 0 ...ort_reconcile_test.go => workload_apiexport_reconcile_test.go} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename pkg/reconciler/workload/apiexport/{apiexport_controller.go => workload_apiexport_controller.go} (100%) rename pkg/reconciler/workload/apiexport/{apiexport_indexes.go => workload_apiexport_indexes.go} (100%) rename pkg/reconciler/workload/apiexport/{apiexport_reconcile.go => workload_apiexport_reconcile.go} (100%) rename pkg/reconciler/workload/apiexport/{apiexport_reconcile_test.go => workload_apiexport_reconcile_test.go} (100%) diff --git a/pkg/reconciler/workload/apiexport/apiexport_controller.go b/pkg/reconciler/workload/apiexport/workload_apiexport_controller.go similarity index 100% rename from pkg/reconciler/workload/apiexport/apiexport_controller.go rename to pkg/reconciler/workload/apiexport/workload_apiexport_controller.go diff --git a/pkg/reconciler/workload/apiexport/apiexport_indexes.go b/pkg/reconciler/workload/apiexport/workload_apiexport_indexes.go similarity index 100% rename from pkg/reconciler/workload/apiexport/apiexport_indexes.go rename to pkg/reconciler/workload/apiexport/workload_apiexport_indexes.go diff --git a/pkg/reconciler/workload/apiexport/apiexport_reconcile.go b/pkg/reconciler/workload/apiexport/workload_apiexport_reconcile.go similarity index 100% rename from pkg/reconciler/workload/apiexport/apiexport_reconcile.go rename to pkg/reconciler/workload/apiexport/workload_apiexport_reconcile.go diff --git a/pkg/reconciler/workload/apiexport/apiexport_reconcile_test.go b/pkg/reconciler/workload/apiexport/workload_apiexport_reconcile_test.go similarity index 100% rename from pkg/reconciler/workload/apiexport/apiexport_reconcile_test.go rename to pkg/reconciler/workload/apiexport/workload_apiexport_reconcile_test.go