getsops / getsops/sops

Attempting to decrypt doubly-encrypted files triggers panic

Open
#1,175 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
23.1k
Forks
1.1k
Avg merge
1d 11h
Merged PRs (30d)
9

Description

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/cli.v1@v1.20.0/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/cli.v1@v1.20.0/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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in stores/yaml/store.go at LoadEncryptedFile and trace the ensureNoMetadata call that only examines branches[0]. Reproduce the issue with the multi-document YAML example, then add coverage showing that metadata in any branch prevents double encryption and that decryption exits gracefully instead of panicking.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cli, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.