Skip to content

Commit

Permalink
autorotate DP secrets if data has changed
Browse files Browse the repository at this point in the history
  • Loading branch information
johannes94 committed Jul 2, 2024
1 parent f07bb47 commit d47303e
Show file tree
Hide file tree
Showing 12 changed files with 65 additions and 23 deletions.
6 changes: 3 additions & 3 deletions .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,14 @@
"filename": "internal/dinosaur/pkg/presenters/managedcentral.go",
"hashed_secret": "f4ac636d63edfd5477df8f25e4f4794c73e91d51",
"is_verified": false,
"line_number": 208
"line_number": 209
},
{
"type": "Secret Keyword",
"filename": "internal/dinosaur/pkg/presenters/managedcentral.go",
"hashed_secret": "e26735ec1cbf8ad15cb7d1eea4893035f61297aa",
"is_verified": false,
"line_number": 214
"line_number": 215
}
],
"internal/dinosaur/pkg/services/dinosaurservice_moq.go": [
Expand Down Expand Up @@ -473,5 +473,5 @@
}
]
},
"generated_at": "2024-06-17T20:09:11Z"
"generated_at": "2024-07-02T18:42:50Z"
}
43 changes: 29 additions & 14 deletions fleetshard/pkg/central/reconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package reconciler
import (
"bytes"
"context"
"crypto/sha256"
"encoding/base64"
"encoding/json"
"fmt"
Expand Down Expand Up @@ -824,14 +825,18 @@ func (r *CentralReconciler) collectReconciliationStatus(ctx context.Context, rem
}

// Only report secrets if Central is ready, to ensure we're not trying to get secrets before they are created.
// Only report secrets if not all secrets are already stored to ensure we don't overwrite initial secrets with corrupted secrets
// from the cluster state.
if isRemoteCentralReady(remoteCentral) && !r.areSecretsStored(remoteCentral.Metadata.SecretsStored) {
secrets, err := r.collectSecretsEncrypted(ctx, remoteCentral)
if isRemoteCentralReady(remoteCentral) {
secrets, secretSum, err := r.collectSecretsEncrypted(ctx, remoteCentral)
if err != nil {
return nil, err
}
status.Secrets = secrets // pragma: allowlist secret

// Only report secrets if data hash differs to make sure we don't produce huge amount of data
// if no update is required on the fleet-manager DB
if secretSum != remoteCentral.Metadata.SecretDataSum { // pragma: allowlist secret
status.Secrets = secrets // pragma: allowlist secret
status.SecretDataSum = secretSum // pragma: allowlist secret
}
}

return status, nil
Expand Down Expand Up @@ -875,18 +880,18 @@ func (r *CentralReconciler) collectSecrets(ctx context.Context, remoteCentral *p
return secrets, nil
}

func (r *CentralReconciler) collectSecretsEncrypted(ctx context.Context, remoteCentral *private.ManagedCentral) (map[string]string, error) {
func (r *CentralReconciler) collectSecretsEncrypted(ctx context.Context, remoteCentral *private.ManagedCentral) (map[string]string, string, error) {
secrets, err := r.collectSecrets(ctx, remoteCentral)
if err != nil {
return nil, err
return nil, "", err
}

encryptedSecrets, err := r.encryptSecrets(secrets)
encryptedSecrets, secretDataSha, err := r.encryptSecrets(secrets)
if err != nil {
return nil, fmt.Errorf("encrypting secrets for namespace: %s: %w", remoteCentral.Metadata.Namespace, err)
return nil, "", fmt.Errorf("encrypting secrets for namespace: %s: %w", remoteCentral.Metadata.Namespace, err)
}

return encryptedSecrets, nil
return encryptedSecrets, secretDataSha, nil
}

func (r *CentralReconciler) decryptSecrets(secrets map[string]string) (map[string]*corev1.Secret, error) {
Expand Down Expand Up @@ -914,24 +919,34 @@ func (r *CentralReconciler) decryptSecrets(secrets map[string]string) (map[strin
return decryptedSecrets, nil
}

func (r *CentralReconciler) encryptSecrets(secrets map[string]*corev1.Secret) (map[string]string, error) {
// encryptSecrets return the encrypted secrets and a sha256 sum of secret data to check if secrets
// need update later on
func (r *CentralReconciler) encryptSecrets(secrets map[string]*corev1.Secret) (map[string]string, string, error) {
encryptedSecrets := map[string]string{}

allSecretData := []byte{}
for key, secret := range secrets { // pragma: allowlist secret
secretBytes, err := json.Marshal(secret)
if err != nil {
return nil, fmt.Errorf("error marshaling secret for encryption: %s: %w", key, err)
return nil, "", fmt.Errorf("error marshaling secret for encryption: %s: %w", key, err)
}

for _, data := range secret.Data {
allSecretData = append(allSecretData, data...)
}

encryptedBytes, err := r.secretCipher.Encrypt(secretBytes)
if err != nil {
return nil, fmt.Errorf("encrypting secret: %s: %w", key, err)
return nil, "", fmt.Errorf("encrypting secret: %s: %w", key, err)
}

encryptedSecrets[key] = base64.StdEncoding.EncodeToString(encryptedBytes)
}

return encryptedSecrets, nil
secretSum := sha256.Sum256(allSecretData)
secretShaStr := base64.StdEncoding.EncodeToString(secretSum[:])

return encryptedSecrets, secretShaStr, nil

}

Expand Down
10 changes: 7 additions & 3 deletions internal/dinosaur/pkg/api/dbapi/central_request_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,13 @@ type CentralRequest struct {
// We store this in the database to ensure that old centrals whose namespace contained "owner-<central-id>" information will continue to work.

// Secrets stores the encrypted secrets reported for a central tenant
Secrets api.JSON `json:"secrets"`
Namespace string `json:"namespace"`
RoutesCreationID string `json:"routes_creation_id"`
Secrets api.JSON `json:"secrets"`
// SecretDataSum is the b64 encoded hash of plain text data of the stored secrets.
// It used used for equality checks of secrets in the dataplane cluster with the secrets stored in DB
SecretDataSum string `json:"secrets_data_sum"`

Namespace string `json:"namespace"`
RoutesCreationID string `json:"routes_creation_id"`
// DeletionTimestamp stores the timestamp of the DELETE api call for the resource.
DeletionTimestamp sql.NullTime `json:"deletionTimestamp"`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ type DataPlaneCentralStatus struct {
// Going to ignore the rest of fields (like capacity and versions) for now, until when they are needed
Routes []DataPlaneCentralRoute
Secrets map[string]string
SecretDataSum string
CentralVersion string
CentralOperatorVersion string
}
Expand Down
5 changes: 5 additions & 0 deletions internal/dinosaur/pkg/api/private/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,9 @@ components:
type: string
description: Map of Secrets created for a Central
type: object
secretDataSum:
description: Hash of plain text secret data used for equality check
type: string
type: object
DataPlaneCentralStatusUpdateRequest:
additionalProperties:
Expand Down Expand Up @@ -444,6 +447,8 @@ components:
additionalProperties:
type: string
type: object
secretDataSum:
type: string
expired-at:
format: date-time
nullable: true
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ func ConvertDataPlaneDinosaurStatus(status map[string]private.DataPlaneCentralSt
CentralClusterID: k,
Conditions: c,
Routes: routes,
Secrets: v.Secrets, // pragma: allowlist secret
Secrets: v.Secrets, // pragma: allowlist secret
SecretDataSum: v.SecretDataSum, // pragma: allowlist secret
})
}

Expand Down
1 change: 1 addition & 0 deletions internal/dinosaur/pkg/presenters/managedcentral.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ func (c *ManagedCentralPresenter) presentManagedCentral(gitopsConfig gitops.Conf
},
Internal: from.Internal,
SecretsStored: getSecretNames(from), // pragma: allowlist secret
SecretDataSum: from.SecretDataSum, // pragma: allowlist secret
ExpiredAt: dbapi.NullTimeToTimePtr(from.ExpiredAt),
},
Spec: private.ManagedCentralAllOfSpec{
Expand Down
7 changes: 7 additions & 0 deletions internal/dinosaur/pkg/services/data_plane_dinosaur.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,18 @@ func (d *dataPlaneCentralService) addSecretsToRequest(centralRequest *dbapi.Cent
logger.Logger.V(10).Infof("skip persisting secrets for Central %s, report is empty or nil", centralRequest.ID)
return nil
}

if centralStatus.SecretDataSum == "" {
// TODO: change this to send a bad request later, once we are sure no FS version without SecretDataSum feature is running
logger.Logger.V(10).Warningf("persisting secret but no secret data sum. this might be a request of a outdated fleetshard version")
}

logger.Logger.Infof("store secret information for central %s", centralRequest.ID)

if err := centralRequest.SetSecrets(centralStatus.Secrets); err != nil {
return serviceError.NewWithCause(serviceError.ErrorGeneral, err, "failed to set secrets for central %s", centralRequest.ID)
}
centralRequest.SecretDataSum = centralStatus.SecretDataSum // pragma: allowlist secret

return nil
}
Expand Down
4 changes: 2 additions & 2 deletions internal/dinosaur/pkg/services/dinosaur.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,11 @@ func (k *dinosaurService) RotateCentralRHSSOClient(ctx context.Context, centralR

func (k *dinosaurService) ResetCentralSecretBackup(ctx context.Context, centralRequest *dbapi.CentralRequest) *errors.ServiceError {
centralRequest.Secrets = nil // pragma: allowlist secret

centralRequest.SecretDataSum = ""
logStateChange("reset secrets", centralRequest.ID, nil)

dbConn := k.connectionFactory.New()
if err := dbConn.Model(centralRequest).Select("secrets").Updates(centralRequest).Error; err != nil {
if err := dbConn.Model(centralRequest).Select("secrets", "secrets_data_hash").Updates(centralRequest).Error; err != nil {
return errors.NewWithCause(errors.ErrorGeneral, err, "Unable to reset secrets for central request")
}

Expand Down
5 changes: 5 additions & 0 deletions openapi/fleet-manager-private.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ components:
type: object
additionalProperties:
type: string
secretDataSum:
type: string
expired-at:
type: string
format: date-time
Expand Down Expand Up @@ -459,6 +461,9 @@ components:
type: object
additionalProperties:
type: string
secretDataSum:
description: "Hash of plain text secret data used for equality check"
type: string

example:
$ref: "#/components/examples/DataPlaneCentralStatusRequestExample"
Expand Down

0 comments on commit d47303e

Please sign in to comment.