Comments (10)
Thanks. I have now checked this, and indeed ciphertexts generated in Tink with empty AD will not be readable by OpenSSL.
But note that in the https://www.rfc-editor.org/rfc/rfc5297#section-6, it is written what needs to be done if AES-SIV is used in the normal AEAD interface (defined in RFC5116) that only has a single AD:
"Therefore, when it is required to access SIV through the interface defined in [RFC5116], it is necessary to marshal multiple AD inputs into a single string (see Section 1.1) prior to invoking SIV."
So Tink really does this as expected by the RFC. An empty string needs to be treated as a single empty AD.
I'm going to close this now. Thanks for bringing this up, this was very helpful!
I have also updated our documentation here:
https://developers.google.com/tink/wire-format#deterministic_aead
from tink.
Oh, I don't know if this should be another bug too, but technically, multiple associated data updates should be treated differently than one single byte array. In other words, [][]byte{[]byte("ad1"), []byte("ad2")}
is very different from []byte("ad1ad2")
. However, that may not be a bug, since that's the Tink API design for EncryptDeterministically, and keeping that design would just remain as a limitation with interoperability with OpenSSL. I just figured I would drop this here, In case it is relevant for future people.
from tink.
Thanks for pointing this out. I will have a look at this.
from tink.
First of all, Tink uses an interface that only allows to pass in one AD. So, it does not support the full standard defined in the RFC 5297. Which means that if something is encrypted with two ADs, then there is no way in Tink to decrypt that. I agree that it would be nice for AES-SIV to support multiple ADs, but we can't easily change the API of the deterministic AEAD primitive.
Now, in Tink's interface with exactly one AD, there are two ways to map an empty AD byte string to the AES-SIV standard: it is either an empty list of ADs, or it is a list with one empty byte array. Tink consistently uses the 2nd version, so it is a list with one empty byte array. (Also in C++ and Python it is done that way.) And I think this is a valid way to do this. (Also: it would be impossible to change this, because uses that have previously encrypted data with this would not be able to decrypt with a newer version, which would be very bad.) But we should probably make this clear in the documentation.
from tink.
Thanks @juergw, I understand the API limitation, I appreciate the response. Only supporting 1 AD does limit some interoperability with OpenSSL. Yet as you said, it isn't easy to change the API and probably deserves its own issue if it were pursued.
As far as the matter at hand, nil
, make([]byte, 0)
and []byte{}
all produce the same value. Obviously, []byte{0}
is a zero byte, so it would be considered an AD byte string. So, I do agree that Tink is consistently choosing to interpret nil
as a list with one empty byte array. However, in OpenSSL, there is no way to update the Cipher with a 0 length AD. So, the only way to have interoperability between the two libraries is to use exactly 1 non-zero length AD. For reference, here is the line that demonstrates that OpenSSL limitation:
I was able to match Python and Tink via PyCryptodome by passing in an update for []
which is considered the same as a zero byte array by that library. Note, that library also doesn't match Tink when no AD is used, it matches OpenSSL as well. Yet, I'm going to go forward with the recommendation that if Tink is used, that it must be with an AD, so that everyone can decrypt Tink's messages.
from tink.
Thanks. I agree that it is unfortunate that OpenSSL and Tink treat empty AD differently. But there is no easy fix for this.
Using a non-empty AD is anyways a good idea.
from tink.
Could you share some OpenSSL code that I can run that show that you can't add a zero length AD in OpenSSL?
from tink.
Sure! Uh.. Is this okay?
#include <openssl/conf.h>
#include <openssl/evp.h>
#include <openssl/err.h>
#include <string.h>
int encryptDeterministically(unsigned char *key,
unsigned char *ciphertext,
unsigned char *msg, int msg_len,
unsigned char *aad, int aad_len)
{
EVP_CIPHER_CTX *ctx;
EVP_CIPHER *aes_siv;
int ciphertext_len;
int len;
if(!(ctx = EVP_CIPHER_CTX_new())) {
ERR_print_errors_fp(stderr);
return -1;
}
if(!(aes_siv = EVP_CIPHER_fetch(NULL, "AES-256-SIV", NULL))) {
ERR_print_errors_fp(stderr);
return -1;
}
if(EVP_EncryptInit_ex(ctx, aes_siv, NULL, key, NULL) != 1) {
ERR_print_errors_fp(stderr);
return -1;
}
if (aad != NULL) {
if(EVP_EncryptUpdate(ctx, NULL, &len, aad, aad_len) != 1) {
ERR_print_errors_fp(stderr);
return -1;
}
} else if (aad_len == 0) {
/* NEED a valid pointer, 16 zero bytes for illustrative purposes only */
unsigned char zero[] = {0, 0, 0, 0,
0, 0, 0, 0,
0, 0, 0, 0,
0, 0, 0, 0};
/* Length is zero though, so this isn't 16 zero bytes, rather a zero byte array */
if(EVP_EncryptUpdate(ctx, NULL, &len, zero, 0) != 1) {
ERR_print_errors_fp(stderr);
return -1;
}
} else {
/* DO NOT update cipher */
}
if(EVP_EncryptUpdate(ctx, ciphertext+16, &len, msg, msg_len) != 1) {
ERR_print_errors_fp(stderr);
return -1;
}
ciphertext_len = len;
/* Final bytes could be written at this point, we sized the
* ciphertext output to have additional room for extra bytes */
if(EVP_EncryptFinal_ex(ctx, ciphertext + 16 + len, &len) != 1) {
ERR_print_errors_fp(stderr);
return -1;
}
ciphertext_len += len;
EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_GET_TAG, 16, ciphertext);
ciphertext_len += 16;
EVP_CIPHER_CTX_free(ctx);
return ciphertext_len;
}
int main(void) {
unsigned char key[] = { 0, 1, 2, 3, 4, 5, 6, 7,
8, 9, 10, 11, 12, 13, 14, 15,
16, 17, 18, 19, 20, 21, 22, 23,
24, 25, 26, 27, 28, 29, 30, 31,
0, 17, 34, 51, 68, 85, 102, 119,
136, 153, 170, 187, 204, 221, 238, 255,
240, 241, 242, 243, 244, 245, 246, 247,
248, 249, 250, 251, 252, 253, 254, 255 };
unsigned char *msg = (unsigned char *)"Some data to encrypt.";
unsigned char *aad = (unsigned char *)"Additional data";
/* 16-bytes for SIV, additional 16-bytes for finalized data,
* which shouldn't exist in this test case */
unsigned char ciphertextWithAAD[strlen((char *)msg)+16+16];
unsigned char ciphertextWithZeroBytes[strlen((char *)msg)+16+16];
unsigned char ciphertextNoUpdate[strlen((char *)msg)+16+16];
int ciphertextWithAAD_len;
int ciphertextWithZeroBytes_len;
int ciphertextNoUpdate_len;
ciphertextWithAAD_len = encryptDeterministically(key, ciphertextWithAAD,
msg, strlen((char *)msg),
aad, strlen((char *)aad));
ciphertextWithZeroBytes_len = encryptDeterministically(key, ciphertextWithZeroBytes,
msg, strlen((char *)msg),
NULL, 0);
ciphertextNoUpdate_len = encryptDeterministically(key, ciphertextNoUpdate,
msg, strlen((char *)msg),
NULL, -1 /* Flag for no update */);
if (ciphertextWithAAD_len > 0) {
printf("Ciphertext with AAD is:\n");
BIO_dump_fp(stdout,
(const char *)ciphertextWithAAD, ciphertextWithAAD_len);
}
if (ciphertextWithZeroBytes_len > 0) {
printf("Ciphertext without any AAD is:\n");
BIO_dump_fp(stdout,
(const char *)ciphertextWithZeroBytes, ciphertextWithZeroBytes_len);
}
if (ciphertextNoUpdate_len > 0) {
printf("Ciphertext without Cipher Update is:\n");
BIO_dump_fp(stdout,
(const char *)ciphertextNoUpdate, ciphertextNoUpdate_len);
}
return 0;
}
from tink.
Update: this turned out to be a vulnerability in OpenSSL. I reported that to the maintainers of OpenSSL, and they have now fixed it. See:
https://www.openssl.org/news/secadv/20230714.txt
https://www.cve.org/CVERecord?id=CVE-2023-2975
from tink.
Thank you for the update, so glad you followed through on that.
from tink.
Related Issues (20)
- Support addIntegerClaim in RawJwt HOT 2
- tink cannot be installed on arm64 linux HOT 9
- Cannot install it with googleapis-common-protos and other related libs in python HOT 7
- Envelope AEAD Performance with GCP KMS HOT 1
- java tink library is not scaling with java threads HOT 1
- Tink installation on bazel and pip3 fail HOT 4
- Missing classes dectected while running R8 HOT 11
- Is Tink for Objective-C buildable via Bazel? HOT 9
- Recommendation on the best approach for rotation HOT 1
- tink 1.7.0 depends on protobuf 3.20.1 which has a security vulnerability patched in a newer version HOT 4
- Windows OS Support HOT 4
- Cannot download tinkey - The project to be billed is associated with a closed billing account. HOT 2
- cannot verify signature HOT 3
- java.security.GeneralSecurityException: cannot verify signature HOT 1
- No manager for type 'type.googleapis.com/google.crypto.tink.HpkePrivateKey' has been registered HOT 2
- H HOT 1
- compile tink cpp library for ios HOT 4
- OSS-Fuzz issue 66058 HOT 1
- Intent to archive https://github.com/google/tink
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from tink.