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

Transit Encryption - batch decrypting an empty string returns a badly formatted response #6140

Closed
DaveWM opened this issue Jan 31, 2019 · 7 comments · Fixed by #9991
Closed
Assignees
Labels
bug Used to indicate a potential bug secret/transit

Comments

@DaveWM
Copy link

DaveWM commented Jan 31, 2019

Describe the bug
Using the transit http api, when you encrypt an empty string using batch encryption (i.e. by sending it in the format "batch_results": [{"plaintext": ""}]) then decrypt it again, you get a response like:

{
  "request_id": "b350dac3-ce5e-5364-ed96-7aa519dea2a1",
  "lease_id": "",
  "renewable": false,
  "lease_duration": 0,
  "data": {
    "batch_results": [
      {}
    ]
  },
  "wrap_info": null,
  "warnings": null,
  "auth": null
}

The batch_results field contains an empty object - I would expect this object to look like {"plaintext": ""} instead.
To Reproduce
To reproduce, run this script I put together (it assumes you have docker and jq installed). The script will set up Vault in Docker, batch encrypt an empty string, batch decrypt the result, and output the result of the call to /decrypt/key.

Expected behavior
I would expect the data field in the response from /decrypt/key to look like:

"batch_results": [{"plaintext": ""}]

However, it actually looks like:

"batch_results": [{}]

Environment:

  • Vault Server Version (retrieve with vault status): 0.11.4
  • Vault CLI Version (retrieve with vault version): 0.11.4
  • Server Operating System/Architecture: Docker container running in Mac OSX High Sierra

Vault server configuration file(s): -

Additional context
None

@ncabatoff
Copy link
Contributor

You're neglecting to base64-encode the plaintext before passing it in. Replace the response= line in your script with the following to make it work:

pt=""
payload=$(jq -Mnc --arg t $(base64 <<< $pt) '{batch_input: [{plaintext: $t}]}' )
response=$(curl --header "X-Vault-Token: vault-token" -X POST --data "$payload" http://127.0.0.1:8200/v1/transit/encrypt/key)

Leaving the bug open because I feel that should've produced an error.

@ncabatoff ncabatoff changed the title Transit Encryption - batch decrypting an empty string returns a badly formatted response Transit Encryption - encrypting a non-base64 encoded string fails to produce an error Jan 31, 2019
@DaveWM
Copy link
Author

DaveWM commented Jan 31, 2019

I don't think the issue is base64 encoding. I'm using an empty string for the plaintext, and if you base64 encode an empty string you should get an empty string (see here)

@ncabatoff
Copy link
Contributor

My mistake, I didn't realize that <<< would append a newline. I will dig deeper.

@ncabatoff ncabatoff changed the title Transit Encryption - encrypting a non-base64 encoded string fails to produce an error Transit Encryption - batch decrypting an empty string returns a badly formatted response Jan 31, 2019
@ncabatoff
Copy link
Contributor

This appears to be an unintended side-effect of the way JSON serialization is done. The elements in the batch_results array are internally of type BatchResponseItem, which has the following definition:

type BatchResponseItem struct {
	// Ciphertext for the plaintext present in the corresponding batch
	// request item
	Ciphertext string `json:"ciphertext,omitempty" structs:"ciphertext" mapstructure:"ciphertext"`

	// Plaintext for the ciphertext present in the corresponding batch
	// request item
	Plaintext string `json:"plaintext,omitempty" structs:"plaintext" mapstructure:"plaintext"`

	// Error, if set represents a failure encountered while encrypting a
	// corresponding batch request item
	Error string `json:"error,omitempty" structs:"error" mapstructure:"error"`
}

The JSON marshaller sees a struct whose fields are all empty and which all have the omitempty annotation, so it writes an empty map '{}'.

I'm not sure why BatchResponseItem comes in a single flavour, rather than one for encrypting and one for decrypting. That - in conjunction with no omitempty on the Plaintext/Ciphertext fields - is probably the needed fix.

@michelvocks michelvocks added bug Used to indicate a potential bug secret/transit labels Nov 8, 2019
diesalbla added a commit to Banno/vault4s that referenced this issue Jul 13, 2020
Due to hashicorp/vault#6140, sometimes
we may get a response in which the plaintext is missing field,
which happens if it would have been an empty string.
diesalbla added a commit to Banno/vault4s that referenced this issue Jul 13, 2020
Due to hashicorp/vault#6140, sometimes
we may get a response in which the plaintext is missing field,
which happens if it would have been an empty string.
diesalbla added a commit to Banno/vault4s that referenced this issue Jul 13, 2020
Due to hashicorp/vault#6140, sometimes
we may get a response in which the plaintext is missing field,
which happens if it would have been an empty string.
diesalbla added a commit to Banno/vault4s that referenced this issue Jul 13, 2020
Due to hashicorp/vault#6140, sometimes
we may get a response in which the plaintext is missing field,
which happens if it would have been an empty string.
@aphorise
Copy link
Contributor

I suspect that this is already resolved & working. @DaveWM perhaps you can confirm? - Eg (using Vault 1.5.0) for me:

JDATA1='{"batch_input":[{"plaintext": ""}, {"plaintext": ""}]}' ;
JDATA2='{"batch_input":[{"plaintext": "MS5teSBzZWNyZXQgZGF0YQo="}, {"plaintext": "Mi5teSBzZWNyZXQgZGF0YQo="}]}' ;

curl -s -X POST -H "X-Vault-Token: ${VAULT_TOKEN}" -d "${JDATA1}" ${VAULT_ADDR}/v1/transit/encrypt/my-key | jq ;
curl -s -X POST -H "X-Vault-Token: ${VAULT_TOKEN}" -d "${JDATA2}" ${VAULT_ADDR}/v1/transit/encrypt/my-key | jq ;
{
  "request_id": "5f567b5c-a99b-eeca-c07c-1f8425d47445",
  "lease_id": "",
  "renewable": false,
  "lease_duration": 0,
  "data": {
    "batch_results": [
      {
        "ciphertext": "vault:v1:h7+RwSuJ8z/OQI0XLjp+Rqw76YTTivLQIBx86A==",
        "key_version": 1
      },
      {
        "ciphertext": "vault:v1:FRrNeafPiw8bFvx/NBt1Wu+cJ3rUdd5r5P8RwQ==",
        "key_version": 1
      }
    ]
  },
  "wrap_info": null,
  "warnings": null,
  "auth": null
}
{
  "request_id": "1566779b-cfe2-548a-032b-ff7d21221d88",
  "lease_id": "",
  "renewable": false,
  "lease_duration": 0,
  "data": {
    "batch_results": [
      {
        "ciphertext": "vault:v1:C/tIdw2rF0Wfvq9EkdJ4z+kVEItSXmSwZLmvwAaRLFktZT1pDpNatjn+Ye4A",
        "key_version": 1
      },
      {
        "ciphertext": "vault:v1:WsRxuoTv8PvGc0KvkjQNGi2alwuggKYBXHvWNsTWquhWFuMk9UtoprcRTQ4Q",
        "key_version": 1
      }
    ]
  },
  "wrap_info": null,
  "warnings": null,
  "auth": null
}

@LukasFritzeDev
Copy link

@aphorise You did two calls to encrypt. I’ve tried it myself using the docker image vault:1.5.3 and got the following results:

JDATA1='{"batch_input":[{"plaintext": ""}, {"plaintext": ""}]}' ;
curl -s -X POST -H "X-Vault-Token: ${VAULT_TOKEN}" -d "${JDATA1}" http://127.0.0.1:8200/v1/transit/encrypt/my-key

Result:

{
  "request_id": "531a6f79-02e6-4384-345e-87ee1203c62d",
  "lease_id": "",
  "renewable": false,
  "lease_duration": 0,
  "data": {
    "batch_results": [
      {"ciphertext": "vault:v1:QdRc3Vr7ISepl4kD3if1AFASHawBjnf1sbMfAw==", "key_version": 1},
      {"ciphertext": "vault:v1:acFrVjGAHSaSX7TLwh+NyOTddea9lgyDB0hcmw==", "key_version": 1}
    ]
  },
  "wrap_info": null,
  "warnings": null,
  "auth": null
}

Try decrypt with the result from above:

JDATA2='{"batch_input": [{"ciphertext": "vault:v1:QdRc3Vr7ISepl4kD3if1AFASHawBjnf1sbMfAw=="},  {"ciphertext": "vault:v1:acFrVjGAHSaSX7TLwh+NyOTddea9lgyDB0hcmw=="}] }' ;
curl -s -X POST -H "X-Vault-Token: ${VAULT_TOKEN}" -d "${JDATA2}" http://127.0.0.1:8200/v1/transit/decrypt/my-key

Result:

{
  "request_id":"0605c85e-43af-c9ef-3a76-de73770d04bb",
  "lease_id": "",
  "renewable": false,
  "lease_duration": 0,
  "data": {
    "batch_results": [
      {},
      {}
    ]
  },
  "wrap_info": null,
  "warnings": null,
  "auth": null
}

As you can see the batch results of the decrypt are empty objects, instead of the expected:

{
  "plaintext": ""
}

@ins0
Copy link
Contributor

ins0 commented Sep 18, 2020

@michelvocks As this is labeled as a bug, and the single encryption is working as expected - its safe to say that this bug is still present in the batch processing decryption, we would like to get this fixed and patched in time - if we provide a PR that fixes this issue is there anything to discuss beforehand?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug secret/transit
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants