Secrets don't get re-encrypted and stays persisted as a plain text

[php-coder]

When user starts to use secrets encryption, the existing secrets should be encrypted after running oadm migrate storage --include=secrets --confirm. But it doesn't happen and secrets stay persisted in the plain text.

Version

oc v3.6.0-alpha.2+b28a75e-471-dirty
kubernetes v1.6.1+5115d708d7
features: Basic-Auth

Steps To Reproduce

1. start OpenShift without secrets encryption enabled (for example, sudo $(which openshift) start --public-master=127.0.0.1 )
2. create a secret (oc create secret generic foo --from-literal=password=test123)
3. stop OpenShift
4. create an encryption config:

kind: EncryptionConfig
apiVersion: v1
resources:
  - resources:
    - secrets
    providers:
    - aesgcm:
        keys:
        - name: key1
          secret: c2vjcmv0iglzihnly3vyzq==
    - identity: {}

5. generate master config (sudo $(which openshift) start master --public-master=127.0.0.1 --write-config=$PWD/openshift.local.config/master)
6. edit master config and enable secrets encryption (sudoedit openshift.local.config/master/master-config.yaml)

kubernetesMasterConfig:
  apiServerArguments:
    experimental-encryption-provider-config:
    - /path/to/encryption.config

7. start OpenShift with secrets encryption enabled (sudo $(which openshift) start --public-master=127.0.0.1 --master-config=$PWD/openshift.local.config/master/master-config.yaml --node-config=$PWD/openshift.local.config/<node>/node-config.yaml)
8. check that the secret persisted in the plain text (the ETCDCTL_API=3 etcdctl --cacert=$PWD/openshift.local.config/master/ca.crt --key=$PWD/openshift.local.config/master/master.etcd-client.key --cert=$PWD/openshift.local.config/master/master.etcd-client.crt --endpoints 'https://127.0.0.1:4001' get /kubernetes.io/secrets/default/foo -w fields | grep Value command should output something like this: "Value" : "k8s\x00\n\f\n\x02v1\x12\x06Secret\x12\x91\x01\nr\n\x03foo\x12\x00\x1a\adefault\"&/api/v1/namespaces/default/secrets/foo*$6689f2de-566f-11e7-8bdd-507b9d2b16b92\x008\x00B\f\b㞩\xca\x05\x10\xfd\xf6\xce\xd1\x03z\x00\x12\x13\n\bpassword\x12\atest123\x1a\x06Opaque\x1a\x00\"\x00")
9. migrate secrets (oadm migrate storage --include=secrets --confirm)
10. check that the secret persisted in the encrypted form

Current Result

Step 8: oadm migrate storage outputs summary: total=168 errors=0 ignored=0 unchanged=168 migrated=0
Step 9: the secret persisted in the plain text (output is similar to the Step 7)

Expected Result

Step 8: oadm migrate storage should print summary: total=168 errors=0 ignored=0 unchanged=0 migrated=168
Step 9: the secret should be encrypted and data should starts from the "k8s:enc:aesgcm:v1:key1:" prefix, for example: "Value" : "k8s:enc:aesgcm:v1:key1:n\x14YG(\x008i\xee\u007f\xce\xd0...

Additional Information

IMPORTANT: I've tested with #14836 and #14838 being applied.

My debug observations:

• Inside of UpdateResource() the flag wasCreated is false
• Inside of GuaranteedUpdate() we're exiting very early, just before transforming data to storage. At this point origState.stale flag is false but it should be true
• origState with a wrong stale field has been returned from getStateFromObject function

@smarterclayton @liggitt @simo5 I couldn't provide a fix at this moment because I don't have enough time and I'm not familiar with rest/etcd code for doing an update. Next week I'll be on PTO and if this blocked issue will be fixed by someone else, I wouldn't mind.

[php-coder]

[security] [SCCFSI] Encrypt secrets at REST in etcd

[smarterclayton]

Yeah, the isStale should be true. This might be the other change I realized I needed to make to GuaranteedSTorage for the etcd self link change.

…

On Fri, Jun 23, 2017 at 12:29 PM, Vyacheslav Semushin < ***@***.***> wrote: <https://camo.githubusercontent.com/193766a3b9959c5f4ed5cd8cf3251d015e839d51/68747470733a2f2f6769746875622e7472656c6c6f2e73657276696365732f696d616765732f6d696e692d7472656c6c6f2d69636f6e2e706e67> [security] [SCCFSI] Encrypt secrets at REST in etcd <https://trello.com/c/yhuQZ3NP/905-security-sccfsi-encrypt-secrets-at-rest-in-etcd> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14864 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p7zE2LPQpwzDfnE2uBeNIy4SqstHks5sG-f1gaJpZM4ODyC6> .

[php-coder]

It turned out that it has been fixed by #15001

@smarterclayton Thanks for fixing this but if you mentioned me at that issue then I wouldn't try to fix it today... :(

[smarterclayton]

Heh, forgot you had an open one On Jul 10, 2017, at 2:03 PM, Vyacheslav Semushin <[email protected]> wrote: It turned out that it has been fixed by #15001 <#15001> @smarterclayton <https://github.com/smarterclayton> Thanks for fixing this but if you mentioned me at that issue then I wouldn't try to fix it today... :( — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14864 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p1X7wqW4o2MCcV4BkioZjMth4mD7ks5sMmdSgaJpZM4ODyC6> .