{"action":"created","issue":{"url":"https://api.github.com/repos/openssl/openssl/issues/6698","repository_url":"https://api.github.com/repos/openssl/openssl","labels_url":"https://api.github.com/repos/openssl/openssl/issues/6698/labels{/name}","comments_url":"https://api.github.com/repos/openssl/openssl/issues/6698/comments","events_url":"https://api.github.com/repos/openssl/openssl/issues/6698/events","html_url":"https://github.com/openssl/openssl/issues/6698","id":340392582,"node_id":"MDU6SXNzdWUzNDAzOTI1ODI=","number":6698,"title":"PKCS12_create and PKCS12_parse use inconsistent ordering","user":{"login":"davidben","id":109690,"node_id":"MDQ6VXNlcjEwOTY5MA==","avatar_url":"https://avatars0.githubusercontent.com/u/109690?v=4","gravatar_id":"","url":"https://api.github.com/users/davidben","html_url":"https://github.com/davidben","followers_url":"https://api.github.com/users/davidben/followers","following_url":"https://api.github.com/users/davidben/following{/other_user}","gists_url":"https://api.github.com/users/davidben/gists{/gist_id}","starred_url":"https://api.github.com/users/davidben/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidben/subscriptions","organizations_url":"https://api.github.com/users/davidben/orgs","repos_url":"https://api.github.com/users/davidben/repos","events_url":"https://api.github.com/users/davidben/events{/privacy}","received_events_url":"https://api.github.com/users/davidben/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-07-11T20:25:13Z","updated_at":"2018-07-11T21:10:04Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"See the output of this test file (error-handling and memory leaks ignored out of laziness):\r\n```\r\n#include <stdio.h>\r\n\r\n#include <openssl/ec.h>\r\n#include <openssl/evp.h>\r\n#include <openssl/crypto.h>\r\n#include <openssl/pkcs12.h>\r\n#include <openssl/rand.h>\r\n#include <openssl/x509.h>\r\n\r\n\r\nstatic EVP_PKEY *MakeKey(void) {\r\n  EC_KEY *ec;\r\n  EVP_PKEY *key;\r\n\r\n  ec = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);\r\n  EC_KEY_generate_key(ec);\r\n\r\n  key = EVP_PKEY_new();\r\n  EVP_PKEY_assign_EC_KEY(key, ec);\r\n  return key;\r\n}\r\n\r\nstatic X509_NAME *MakeName(const char *name) {\r\n  X509_NAME *name_obj = X509_NAME_new();\r\n  X509_NAME_add_entry_by_txt(name_obj, \"CN\", MBSTRING_ASC,\r\n                             (const unsigned char *)name, -1, -1, 0);\r\n  return name_obj;\r\n}\r\n\r\nstatic X509 *MakeCert(const char *subject, EVP_PKEY *subject_key,\r\n                      const char *issuer, EVP_PKEY *issuer_key) {\r\n  uint32_t serial;\r\n  X509 *x509;\r\n\r\n  x509 = X509_new();\r\n  RAND_bytes((unsigned char *)&serial, sizeof(serial));\r\n  ASN1_INTEGER_set_uint64(X509_get_serialNumber(x509), serial);\r\n  X509_gmtime_adj(X509_get_notBefore(x509), 0);\r\n  X509_gmtime_adj(X509_get_notAfter(x509), 60 * 60 * 24);\r\n  X509_set_subject_name(x509, MakeName(subject));\r\n  X509_set_issuer_name(x509, MakeName(issuer));\r\n  X509_set_pubkey(x509, subject_key);\r\n  X509_sign(x509, issuer_key, EVP_sha256());\r\n  return x509;\r\n}\r\n\r\nstatic void PrintCert(X509 *x509) {\r\n  if (x509 == NULL) {\r\n    printf(\"  (null)\\n\");\r\n  } else {\r\n    X509_NAME_print_ex_fp(stdout, X509_get_subject_name(x509), 2,\r\n                          XN_FLAG_ONELINE);\r\n    printf(\"\\n\");\r\n  }\r\n}\r\n\r\nint main(int argc, char **argv) {\r\n  EVP_PKEY *leaf_key, *ca1_key, *ca2_key, *root_key, *parsed_key = NULL;\r\n  X509 *leaf, *ca1, *ca2, *root, *parsed_leaf = NULL;\r\n  STACK_OF(X509) *chain, *parsed_chain = NULL;\r\n  int i;\r\n  PKCS12 *p12;\r\n  FILE *f;\r\n\r\n  OPENSSL_init_crypto(0, NULL);\r\n\r\n  leaf_key = MakeKey();\r\n  ca1_key = MakeKey();\r\n  ca2_key = MakeKey();\r\n  root_key = MakeKey();\r\n\r\n  leaf = MakeCert(\"Leaf\", leaf_key, \"CA #1\", ca1_key);\r\n  ca1 = MakeCert(\"CA #1\", ca1_key, \"CA #2\", ca2_key);\r\n  ca2 = MakeCert(\"CA #2\", ca2_key, \"Root\", root_key);\r\n  root = MakeCert(\"Root\", root_key, \"Root\", root_key);\r\n\r\n  chain = sk_X509_new_null();\r\n  sk_X509_push(chain, ca1);\r\n  sk_X509_push(chain, ca2);\r\n  sk_X509_push(chain, root);\r\n\r\n  printf(\"Calling PKCS12_create with leaf:\\n\");\r\n  PrintCert(leaf);\r\n  printf(\"And CAs:\\n\");\r\n  for (i = 0; i < sk_X509_num(chain); i++) {\r\n    PrintCert(sk_X509_value(chain, i));\r\n  }\r\n\r\n  p12 = PKCS12_create(NULL, NULL, leaf_key, leaf, chain, 0, 0, 0, 0, 0);\r\n  if (argc == 2) {\r\n    printf(\"Writing to %s\\n\", argv[1]);\r\n    f = fopen(argv[1], \"w\");\r\n    i2d_PKCS12_fp(f, p12);\r\n    fclose(f);\r\n  }\r\n\r\n  PKCS12_parse(p12, NULL, &parsed_key, &parsed_leaf, &parsed_chain);\r\n  printf(\"PKCS12_parse on the result gives leaf:\\n\");\r\n  PrintCert(parsed_leaf);\r\n  printf(\"And CAs:\\n\");\r\n  for (i = 0; i < sk_X509_num(parsed_chain); i++) {\r\n    PrintCert(sk_X509_value(parsed_chain, i));\r\n  }\r\n\r\n  return 0;\r\n}\r\n```\r\n\r\nIn OpenSSL, this gives:\r\n```\r\nCalling PKCS12_create with leaf:\r\n  CN = Leaf\r\nAnd CAs:\r\n  CN = CA #1\r\n  CN = CA #2\r\n  CN = Root\r\nWriting to foo.pfx\r\nPKCS12_parse on the result gives leaf:\r\n  CN = Leaf\r\nAnd CAs:\r\n  CN = Root\r\n  CN = CA #2\r\n  CN = CA #1\r\n```\r\n\r\nExamining 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:\r\nhttps://github.com/openssl/openssl/blob/1b6a0a261e22eb5a574bdb75da208817ffa2fbba/crypto/pkcs12/p12_crt.c#L68-L80\r\n\r\nRather, the quirk comes from this logic, which ends up reversing the order of all non-leaf certificates:\r\nhttps://github.com/openssl/openssl/blob/1b6a0a261e22eb5a574bdb75da208817ffa2fbba/crypto/pkcs12/p12_kiss.c#L87-L107\r\n\r\nIt'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."},"comment":{"url":"https://api.github.com/repos/openssl/openssl/issues/comments/404310234","html_url":"https://github.com/openssl/openssl/issues/6698#issuecomment-404310234","issue_url":"https://api.github.com/repos/openssl/openssl/issues/6698","id":404310234,"node_id":"MDEyOklzc3VlQ29tbWVudDQwNDMxMDIzNA==","user":{"login":"levitte","id":698918,"node_id":"MDQ6VXNlcjY5ODkxOA==","avatar_url":"https://avatars1.githubusercontent.com/u/698918?v=4","gravatar_id":"","url":"https://api.github.com/users/levitte","html_url":"https://github.com/levitte","followers_url":"https://api.github.com/users/levitte/followers","following_url":"https://api.github.com/users/levitte/following{/other_user}","gists_url":"https://api.github.com/users/levitte/gists{/gist_id}","starred_url":"https://api.github.com/users/levitte/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/levitte/subscriptions","organizations_url":"https://api.github.com/users/levitte/orgs","repos_url":"https://api.github.com/users/levitte/repos","events_url":"https://api.github.com/users/levitte/events{/privacy}","received_events_url":"https://api.github.com/users/levitte/received_events","type":"User","site_admin":false},"created_at":"2018-07-11T21:10:04Z","updated_at":"2018-07-11T21:10:04Z","author_association":"MEMBER","body":"Looks like `sk_X509_shift` should be used on `ocerts`, rather than `sk_X509_pop`...\r\nPersonally, I could see that as a mere bug fix... "}}
{"actor":{"display_login":"levitte"}}