-
-
Notifications
You must be signed in to change notification settings - Fork 11.2k
Closed
Milestone
Description
See the output of this test file (error-handling and memory leaks ignored out of laziness):
#include <stdio.h>
#include <openssl/ec.h>
#include <openssl/evp.h>
#include <openssl/crypto.h>
#include <openssl/pkcs12.h>
#include <openssl/rand.h>
#include <openssl/x509.h>
static EVP_PKEY *MakeKey(void) {
EC_KEY *ec;
EVP_PKEY *key;
ec = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
EC_KEY_generate_key(ec);
key = EVP_PKEY_new();
EVP_PKEY_assign_EC_KEY(key, ec);
return key;
}
static X509_NAME *MakeName(const char *name) {
X509_NAME *name_obj = X509_NAME_new();
X509_NAME_add_entry_by_txt(name_obj, "CN", MBSTRING_ASC,
(const unsigned char *)name, -1, -1, 0);
return name_obj;
}
static X509 *MakeCert(const char *subject, EVP_PKEY *subject_key,
const char *issuer, EVP_PKEY *issuer_key) {
uint32_t serial;
X509 *x509;
x509 = X509_new();
RAND_bytes((unsigned char *)&serial, sizeof(serial));
ASN1_INTEGER_set_uint64(X509_get_serialNumber(x509), serial);
X509_gmtime_adj(X509_get_notBefore(x509), 0);
X509_gmtime_adj(X509_get_notAfter(x509), 60 * 60 * 24);
X509_set_subject_name(x509, MakeName(subject));
X509_set_issuer_name(x509, MakeName(issuer));
X509_set_pubkey(x509, subject_key);
X509_sign(x509, issuer_key, EVP_sha256());
return x509;
}
static void PrintCert(X509 *x509) {
if (x509 == NULL) {
printf(" (null)\n");
} else {
X509_NAME_print_ex_fp(stdout, X509_get_subject_name(x509), 2,
XN_FLAG_ONELINE);
printf("\n");
}
}
int main(int argc, char **argv) {
EVP_PKEY *leaf_key, *ca1_key, *ca2_key, *root_key, *parsed_key = NULL;
X509 *leaf, *ca1, *ca2, *root, *parsed_leaf = NULL;
STACK_OF(X509) *chain, *parsed_chain = NULL;
int i;
PKCS12 *p12;
FILE *f;
OPENSSL_init_crypto(0, NULL);
leaf_key = MakeKey();
ca1_key = MakeKey();
ca2_key = MakeKey();
root_key = MakeKey();
leaf = MakeCert("Leaf", leaf_key, "CA #1", ca1_key);
ca1 = MakeCert("CA #1", ca1_key, "CA #2", ca2_key);
ca2 = MakeCert("CA #2", ca2_key, "Root", root_key);
root = MakeCert("Root", root_key, "Root", root_key);
chain = sk_X509_new_null();
sk_X509_push(chain, ca1);
sk_X509_push(chain, ca2);
sk_X509_push(chain, root);
printf("Calling PKCS12_create with leaf:\n");
PrintCert(leaf);
printf("And CAs:\n");
for (i = 0; i < sk_X509_num(chain); i++) {
PrintCert(sk_X509_value(chain, i));
}
p12 = PKCS12_create(NULL, NULL, leaf_key, leaf, chain, 0, 0, 0, 0, 0);
if (argc == 2) {
printf("Writing to %s\n", argv[1]);
f = fopen(argv[1], "w");
i2d_PKCS12_fp(f, p12);
fclose(f);
}
PKCS12_parse(p12, NULL, &parsed_key, &parsed_leaf, &parsed_chain);
printf("PKCS12_parse on the result gives leaf:\n");
PrintCert(parsed_leaf);
printf("And CAs:\n");
for (i = 0; i < sk_X509_num(parsed_chain); i++) {
PrintCert(sk_X509_value(parsed_chain, i));
}
return 0;
}
In OpenSSL, this gives:
Calling PKCS12_create with leaf:
CN = Leaf
And CAs:
CN = CA #1
CN = CA #2
CN = Root
Writing to foo.pfx
PKCS12_parse on the result gives leaf:
CN = Leaf
And CAs:
CN = Root
CN = CA #2
CN = CA #1
Examining foo.pfx (which does not use PKCS12_parse) shows that the ordering in the file is the PKCS12_create one. PKCS12_create writes out the leaf, followed by the CAs in the supplied order:
openssl/crypto/pkcs12/p12_crt.c
Lines 68 to 80 in 1b6a0a2
| if (cert) { | |
| bag = PKCS12_add_cert(&bags, cert); | |
| if (name && !PKCS12_add_friendlyname(bag, name, -1)) | |
| goto err; | |
| if (keyidlen && !PKCS12_add_localkeyid(bag, keyid, keyidlen)) | |
| goto err; | |
| } | |
| /* Add all other certificates */ | |
| for (i = 0; i < sk_X509_num(ca); i++) { | |
| if (!PKCS12_add_cert(&bags, sk_X509_value(ca, i))) | |
| goto err; | |
| } |
Rather, the quirk comes from this logic, which ends up reversing the order of all non-leaf certificates:
openssl/crypto/pkcs12/p12_kiss.c
Lines 87 to 107 in 1b6a0a2
| while ((x = sk_X509_pop(ocerts))) { | |
| if (pkey && *pkey && cert && !*cert) { | |
| ERR_set_mark(); | |
| if (X509_check_private_key(x, *pkey)) { | |
| *cert = x; | |
| x = NULL; | |
| } | |
| ERR_pop_to_mark(); | |
| } | |
| if (ca && x) { | |
| if (!*ca) | |
| *ca = sk_X509_new_null(); | |
| if (!*ca) | |
| goto err; | |
| if (!sk_X509_push(*ca, x)) | |
| goto err; | |
| x = NULL; | |
| } | |
| X509_free(x); | |
| } |
It's no doubt to late to change this for 1.1.x (I learned about this from code which was sensitive to this), but it should at least be documented.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels