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

Attempting to decrypt doubly-encrypted files triggers panic #1175

Open
dcadolph opened this issue Feb 14, 2023 · 0 comments
Open

Attempting to decrypt doubly-encrypted files triggers panic #1175

dcadolph opened this issue Feb 14, 2023 · 0 comments

Comments

@dcadolph
Copy link

Problem

When attempting to decrypt a file that has been (inadvertently) encrypted twice, cli panics.

Example

Consumer encrypts the following YAML file.

secret: nunya

The result is:

secret: ENC[AES256_GCM,data:N1ewFlk=,iv:RYS/rKFoY0CiaydaBOamiANfIzO/hgy2NQ7fFwqBsEo=,tag:3xHmTrAPDAWCleTvneCkyg==,type:str]
sops:
    kms: []
    gcp_kms: []
    azure_kv: []
    hc_vault: []
    age:
        - recipient: age1dux0j2jdwlu4lcq95llqyy2lu4tjvmuzww5hsxk3k0y7tz8a2s9szcpzja
          enc: |
            -----BEGIN AGE ENCRYPTED FILE-----
            YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSB6ek16S0F5M2lMRDE5RCs2
            ZTNwV3VMNFR0U3dRQnRLWkUzZWkzY0hEeGpRCmF3MjdleE92VmJQZDdmSWdlY3BL
            b0NZWk85YXo5QnJnNXJLZWdOM1Zqa1UKLS0tIFdJb2VuTmowV2cwb2JsZm5sL25Y
            RGYwd0NyaVF2UkFUY3V0ODZ0S0hkYVkKZCpiS13d34WOSMurduNtkySz1OX62rKZ
            DFS043kiu8/2H3cPgl+I0smGd96fKqIQt0eDagUYTDkRsCdh/KdGYA==
            -----END AGE ENCRYPTED FILE-----
    lastmodified: "2023-02-14T03:45:03Z"
    mac: ENC[AES256_GCM,data:xPAtovoNW1if8j/yMDuE2CrmPthZLlv09lcMXdymCkUFfHAB+2HpbQEYUQDpkJArvP/MzFYODF+xIC1Aayj4CLSC55np14opwsi43T4cz+xH/SgeiAORp7V6hyBVibQ/sV75s9NkNCH8lGZzHVaGHPVfIxXdNexLoW+b1hihA6M=,iv:oOdEQhA41YFIufTsKikwueSSN4vFr8pmJeUQUw8DzVA=,tag:vR73I0yazDaIP+9h+fE5Sg==,type:str]
    pgp: []
    unencrypted_suffix: _unencrypted
    version: 3.7.3

The consumer then (oversight) combines a plaintext YAML document with the YAML document they already encrypted, creating a single (amalgamated) YAML file with two separate YAMl documents. They do not include the encrypted YAML document first (i.e. when the cli searches branches[0] for existing metadata, the metadata key "sops" will not be identified and encryption will be performed).

secret: on-the-dl
...
---
secret: ENC[AES256_GCM,data:N1ewFlk=,iv:RYS/rKFoY0CiaydaBOamiANfIzO/hgy2NQ7fFwqBsEo=,tag:3xHmTrAPDAWCleTvneCkyg==,type:str]
sops:
    kms: []
    gcp_kms: []
    azure_kv: []
    hc_vault: []
    age:
        - recipient: age1dux0j2jdwlu4lcq95llqyy2lu4tjvmuzww5hsxk3k0y7tz8a2s9szcpzja
          enc: |
            -----BEGIN AGE ENCRYPTED FILE-----
            YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSB6ek16S0F5M2lMRDE5RCs2
            ZTNwV3VMNFR0U3dRQnRLWkUzZWkzY0hEeGpRCmF3MjdleE92VmJQZDdmSWdlY3BL
            b0NZWk85YXo5QnJnNXJLZWdOM1Zqa1UKLS0tIFdJb2VuTmowV2cwb2JsZm5sL25Y
            RGYwd0NyaVF2UkFUY3V0ODZ0S0hkYVkKZCpiS13d34WOSMurduNtkySz1OX62rKZ
            DFS043kiu8/2H3cPgl+I0smGd96fKqIQt0eDagUYTDkRsCdh/KdGYA==
            -----END AGE ENCRYPTED FILE-----
    lastmodified: "2023-02-14T03:45:03Z"
    mac: ENC[AES256_GCM,data:xPAtovoNW1if8j/yMDuE2CrmPthZLlv09lcMXdymCkUFfHAB+2HpbQEYUQDpkJArvP/MzFYODF+xIC1Aayj4CLSC55np14opwsi43T4cz+xH/SgeiAORp7V6hyBVibQ/sV75s9NkNCH8lGZzHVaGHPVfIxXdNexLoW+b1hihA6M=,iv:oOdEQhA41YFIufTsKikwueSSN4vFr8pmJeUQUw8DzVA=,tag:vR73I0yazDaIP+9h+fE5Sg==,type:str]
    pgp: []
    unencrypted_suffix: _unencrypted
    version: 3.7.3

They encrypt the file (no error, no panic) and the contents are modified to:

secret: ENC[AES256_GCM,data:92cLigbVk+lR,iv:Xf07RCH+lAcPnDWHUhd3yDpo4MXY7PAt59gzwPxW7BE=,tag:5Ow7M9Ypt1IonchUMHzSaA==,type:str]
sops:
    kms: []
    gcp_kms: []
    azure_kv: []
    hc_vault: []
    age:
        - recipient: age1dux0j2jdwlu4lcq95llqyy2lu4tjvmuzww5hsxk3k0y7tz8a2s9szcpzja
          enc: |
            -----BEGIN AGE ENCRYPTED FILE-----
            YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSAvWGlCaEwrYWhVRmpVQWlI
            MGFCbjJNeFI2RERLVHNCYUJsZk5palRnVmxNCjJnb1ZOQ0p3SFg5bkdYeHpVSFBv
            QlFXWTBvbHg5N2svaWFwSHBxY1V4ekEKLS0tIDZEMjlsTVdLVkJ2MGNJWlppYkwx
            dkN5QWY1Vm1uTG5DVnRhY0o2NTc2Sm8KwpFJhKZ8lWnwSDVqkxfbWIug4x6q4QXt
            9NPbOhKDy1JHJrCYM8STu6Q07Loidn+0IvpXoW0YN4ZFw7otQPoBzA==
            -----END AGE ENCRYPTED FILE-----
    lastmodified: "2023-02-14T03:48:58Z"
    mac: ENC[AES256_GCM,data:YdMHezKnNRKZ4NvzDoEahjr2QOE0ykxIPcqe0ekxEpFDm1yZ4NqKh2TTTQ/vQU8+Z6ITYLFA2tDzl95x3DXaMJOkK7q9nCFZ78ZJaO0IM0DS823IyvoBnmTWVKBbSA0sd4pT6OAvSNc1nv1GqC6vQlR6l3ZQBqnqEK1UYuz7mL0=,iv:lavCCRattBBd0jp0O7PYn0OGsJVvH/aKoYd0cEy0s64=,tag:2ixQG/tVzV59L+z57+C1uw==,type:str]
    pgp: []
    unencrypted_suffix: _unencrypted
    version: 3.7.3
---
secret: ENC[AES256_GCM,data:j+Vjb6g/3yrfxtmiq9CxI7sJxJybLPQEXdP6CPArkGRjp1tNkG6kWA/e1hd+EsBu9MAU+ygDDVWVAInRBd1yASf18FeiL/A3TxRlIU7ghPHJ1VTiXeMRtCxLcWfzXjuIOtMldD4cy53Bquq4vmUJfGOW3g==,iv:4pyoB6eM3oqvtNBEecg3Y8bTD2ID46+SfwUhyGpz7Tg=,tag:8mbP0zhXzHbuZQ3T83lHCw==,type:str]
sops:
    kms: []
    gcp_kms: []
    azure_kv: []
    hc_vault: []
    age:
        - recipient: ENC[AES256_GCM,data:ihKLIkn9DvNCbaQHiVZcTpfQe6tAqpDTK+fnNhYimaGdCDuzg1qKWfbOgLSrc1OCx2MKo0GtII5VyVatjEI=,iv:D89P/llZBiVvJUZeeZx7UROSDEuhcwAKA5ZxmD78zT8=,tag:W3ypn1PxZJM0kITJdfAtjw==,type:str]
          enc: ENC[AES256_GCM,data:u5OvY8xONd0rhsEInSMcQSd1jUlAiAl8TmYVQpMnWAR1I+jz1eSYWWmuKPbROX5xrwt2D+dX+Tc2jONXfw8fjGgS5EnjIjSlFDFsAr2qPwEVNv6eps7ownRNno5Efx6SOpjWGYm6RNpRH11X3rMwp95jq6qQ1gnUoOyZ//g7tqH2W+eMyIiLxlc9yfSqb92rOhJ//hRNtR+XNhb7GhjNQuIIDiw3OL41VFbGgmxs7pY1t5cJAfF13x++I8mLJaWpz3qdPIhUe8TMqKHBp9j+yCcpD0QK4H8ZpAl2solshOV4fOQbEtzifpmBk1e+gHtgsZBSc7ZFyM7s1D7sVURnjbYFN0Wk7sUhP+gvQVabhSfPtGa8CyAmEIn3w/HlpRpz1RZ4/S7DSQPM1hVFJZ1Y8UJU2UUs+aSn5/Pdkco3eI69El9XEbmoTh4sxrSsGOOXOB3S3dijvOMSlLUpoaAcwjK1Dh1sS6rIoOVq13nfsm67blQmuh5se7aMGUIggo6/DA==,iv:k8iBzxB1WNPu0px2skNY3MOYguQ71OqlxTOWqYyI8pg=,tag:6rh0Ut/+CChPumWjeQ4N/g==,type:str]
    lastmodified: ENC[AES256_GCM,data:ILpHYx+3/FI1X6bYma3aqEPV3J4=,iv:YdZha6yh/QeF+oxvIlwRw79FqymEpmhII58jVUlz6Pk=,tag:Ta+XP2C+jIHlQNNqdi7umw==,type:str]
    mac: ENC[AES256_GCM,data:p0pwC50cws0a1XnVoB3pIiBZt2x6HOPvd6DpI9CnIE37W/Sr019whCjvFJv+tZHSiGuX0mlV3k+e9HoQOWyu5qy18i+VJhXXE5+O5+cZR/ZTmp8YNlp4Cb0v/PXxKaHC5HyDOIE8ITNG5dqHXoandMw3ibfvPWQC3cXt7NszPDsqCs380Mv0v6/62zbmS280tuTafW2fH+A7M3SshXhTUOau6Y19sreCy/bZjt3vNUttJ3ug3f25ZYFOro6TMl3POz58CGUuNK1dWWyA88VP9UhfBnXRwGg2BoW5csNN/qoghZFnOCaq3BvgE+G3iE9ZRTx6NEvyOQC5muRYHZjyLYwGV/sp0OvwwvAaBYU40VBlUE8aUky7,iv:X5Ry2ALezU6B54zk0mYZs6dvagd9IdpXBPAjTAM5O9w=,tag:uM0a5xetefDADGHdyXab8g==,type:str]
    pgp: []
    unencrypted_suffix: ENC[AES256_GCM,data:0m7lmstx7x46HW4I,iv:I/JU308bWNkq53GKPt3T7Enrq7sdsVhLBMbITq5kaHU=,tag:nmPz8C5nzftY7PXe8pF4vw==,type:str]
    version: ENC[AES256_GCM,data:BI+ODC8=,iv:UXyh2KaMockgWAgCerKY7ynRvNOBEEFl7UPOSR1Gf1g=,tag:x/QvqwcuG0gU7+61P6IxnQ==,type:str]
sops:
    kms: []
    gcp_kms: []
    azure_kv: []
    hc_vault: []
    age:
        - recipient: age1dux0j2jdwlu4lcq95llqyy2lu4tjvmuzww5hsxk3k0y7tz8a2s9szcpzja
          enc: |
            -----BEGIN AGE ENCRYPTED FILE-----
            YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSAvWGlCaEwrYWhVRmpVQWlI
            MGFCbjJNeFI2RERLVHNCYUJsZk5palRnVmxNCjJnb1ZOQ0p3SFg5bkdYeHpVSFBv
            QlFXWTBvbHg5N2svaWFwSHBxY1V4ekEKLS0tIDZEMjlsTVdLVkJ2MGNJWlppYkwx
            dkN5QWY1Vm1uTG5DVnRhY0o2NTc2Sm8KwpFJhKZ8lWnwSDVqkxfbWIug4x6q4QXt
            9NPbOhKDy1JHJrCYM8STu6Q07Loidn+0IvpXoW0YN4ZFw7otQPoBzA==
            -----END AGE ENCRYPTED FILE-----
    lastmodified: "2023-02-14T03:48:58Z"
    mac: ENC[AES256_GCM,data:YdMHezKnNRKZ4NvzDoEahjr2QOE0ykxIPcqe0ekxEpFDm1yZ4NqKh2TTTQ/vQU8+Z6ITYLFA2tDzl95x3DXaMJOkK7q9nCFZ78ZJaO0IM0DS823IyvoBnmTWVKBbSA0sd4pT6OAvSNc1nv1GqC6vQlR6l3ZQBqnqEK1UYuz7mL0=,iv:lavCCRattBBd0jp0O7PYn0OGsJVvH/aKoYd0cEy0s64=,tag:2ixQG/tVzV59L+z57+C1uw==,type:str]
    pgp: []
    unencrypted_suffix: _unencrypted
    version: 3.7.3

When the consumer attempts to decrypt this file, sops will panic:

panic: runtime error: slice bounds out of range [3:2]

goroutine 1 [running]:
go.mozilla.org/sops/v3/stores/yaml.(*Store).LoadEncryptedFile(0x10187a498, {0x1400001b300, 0x1103, _})
	/private/tmp/sops-20221023-4343-apisg3/sops-3.7.3/stores/yaml/store.go:282 +0x7b0
go.mozilla.org/sops/v3/cmd/sops/common.LoadEncryptedFile({0x129248d58, 0x10187a498}, {0x1400011ee70, 0x25})
	/private/tmp/sops-20221023-4343-apisg3/sops-3.7.3/cmd/sops/common/common.go:138 +0x110
go.mozilla.org/sops/v3/cmd/sops/common.LoadEncryptedFileWithBugFixes({{0x1012037a0, 0x1400069d7d0}, {0x129248d18, 0x10187a498}, {0x1400011ee70, 0x25}, 0x0, {0x14000121810, 0x1, 0x1}})
	/private/tmp/sops-20221023-4343-apisg3/sops-3.7.3/cmd/sops/common/common.go:232 +0x68
main.decrypt({{0x1012037a0, 0x1400069d7d0}, {0x129248d18, 0x10187a498}, {0x129248d18, 0x10187a498}, {0x1400011ee70, 0x25}, 0x0, {0x0, ...}, ...})
	/private/tmp/sops-20221023-4343-apisg3/sops-3.7.3/cmd/sops/decrypt.go:23 +0xe4
main.main.func8(0x14000312160)
	/private/tmp/sops-20221023-4343-apisg3/sops-3.7.3/cmd/sops/main.go:811 +0x918
gopkg.in/urfave/cli%2ev1.HandleAction({0x1010721a0?, 0x1011f7348?}, 0x14000332000?)
	/Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/urfave/[email protected]/app.go:490 +0x6c
gopkg.in/urfave/cli%2ev1.(*App).Run(0x14000103a00, {0x14000194180, 0x3, 0x3})
	/Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/urfave/[email protected]/app.go:264 +0x518
main.main()
	/private/tmp/sops-20221023-4343-apisg3/sops-3.7.3/cmd/sops/main.go:989 +0x2bec

Proposed Solution

In the code, rather than iterating over only the first branch, looking for the "sops" metadata key in the file:

if err := ensureNoMetadata(opts, branches[0]); err != nil {
   return nil, common.NewExitError(err, codes.FileAlreadyEncrypted)
}

Code should iterate over all branches:

if err := ensureNoMetadata(opts, branches); err != nil {
   return nil, common.NewExitError(err, codes.FileAlreadyEncrypted)
}

This way, a file that is "partially encrypted" (encrypted in its entirety and then later combined with additional documents or objects) will not be encrypted a second time, resulting in data loss (since the file can now no longer be decrypted) and a panic on the cli's side when an attempt is made to decrypt the file.

Reasoning

CLI should only panic if there is a developer error, not a consumer oversight. If the panic can be prevented when an attempt is made to "doubly encrypt" a file, that would be ideal and allow the cli to exit gracefully. If the proposed solution is not ideal, for reasons of efficiency or otherwise, some mechanism should be employed to prevent a panic when an attempt to decrypt a "doubly-encrypted" file is made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant