From cb1908043d5daa7c5c38945c048c4a2477a46221 Mon Sep 17 00:00:00 2001
From: Paul Kehrer <paul.l.kehrer@gmail.com>
Date: Sun, 28 Feb 2021 16:06:11 -0600
Subject: [PATCH 1/3] fix pkcs12 parse ordering. fixes #5872 (#5879)

* fix pkcs12 parse ordering. fixes #5872

* remove an unneeded print

* simplify the test a bit more

* index

* black

* Update tests/hazmat/primitives/test_pkcs12.py

Co-authored-by: Alex Gaynor <alex.gaynor@gmail.com>

Co-authored-by: Alex Gaynor <alex.gaynor@gmail.com>
---
 .../hazmat/backends/openssl/backend.py        |  5 +-
 tests/hazmat/primitives/test_pkcs12.py        | 58 ++++++++++++++++++-
 2 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py
index 271873d9..a96d08d8 100644
--- a/src/cryptography/hazmat/backends/openssl/backend.py
+++ b/src/cryptography/hazmat/backends/openssl/backend.py
@@ -6,6 +6,7 @@
 import collections
 import contextlib
 import itertools
+import typing
 import warnings
 from contextlib import contextmanager
 
@@ -2562,9 +2563,7 @@ class Backend(object):
             sk_x509 = self._lib.sk_X509_new_null()
             sk_x509 = self._ffi.gc(sk_x509, self._lib.sk_X509_free)
 
-            # reverse the list when building the stack so that they're encoded
-            # in the order they were originally provided. it is a mystery
-            for ca in reversed(cas):
+            for ca in cas:
                 res = self._lib.sk_X509_push(sk_x509, ca._x509)
                 backend.openssl_assert(res >= 1)
 
diff --git a/tests/hazmat/primitives/test_pkcs12.py b/tests/hazmat/primitives/test_pkcs12.py
index b5de09f9..b1759a1b 100644
--- a/tests/hazmat/primitives/test_pkcs12.py
+++ b/tests/hazmat/primitives/test_pkcs12.py
@@ -4,13 +4,15 @@
 
 
 import os
+from datetime import datetime
 
 import pytest
 
 from cryptography import x509
 from cryptography.hazmat.backends.interfaces import DERSerializationBackend
 from cryptography.hazmat.backends.openssl.backend import _RC2
-from cryptography.hazmat.primitives import serialization
+from cryptography.hazmat.primitives import hashes, serialization
+from cryptography.hazmat.primitives.asymmetric import ec
 from cryptography.hazmat.primitives.serialization import load_pem_private_key
 from cryptography.hazmat.primitives.serialization.pkcs12 import (
     load_key_and_certificates,
@@ -273,3 +275,57 @@ class TestPKCS12Creation(object):
                 DummyKeySerializationEncryption(),
             )
         assert str(exc.value) == "Unsupported key encryption type"
+
+
+def test_pkcs12_ordering():
+    """
+    In OpenSSL < 3.0.0 PKCS12 parsing reverses the order. However, we
+    accidentally thought it was **encoding** that did it, leading to bug
+    https://github.com/pyca/cryptography/issues/5872
+    This test ensures our ordering is correct going forward.
+    """
+
+    def make_cert(name):
+        key = ec.generate_private_key(ec.SECP256R1())
+        subject = x509.Name(
+            [
+                x509.NameAttribute(x509.NameOID.COMMON_NAME, name),
+            ]
+        )
+        now = datetime.utcnow()
+        cert = (
+            x509.CertificateBuilder()
+            .subject_name(subject)
+            .issuer_name(subject)
+            .public_key(key.public_key())
+            .serial_number(x509.random_serial_number())
+            .not_valid_before(now)
+            .not_valid_after(now)
+            .sign(key, hashes.SHA256())
+        )
+        return (key, cert)
+
+    # Make some certificates with distinct names.
+    a_name = "A" * 20
+    b_name = "B" * 20
+    c_name = "C" * 20
+    a_key, a_cert = make_cert(a_name)
+    _, b_cert = make_cert(b_name)
+    _, c_cert = make_cert(c_name)
+
+    # Bundle them in a PKCS#12 file in order A, B, C.
+    p12 = serialize_key_and_certificates(
+        b"p12", a_key, a_cert, [b_cert, c_cert], serialization.NoEncryption()
+    )
+
+    # Parse them out. The API should report them in the same order.
+    (key, cert, certs) = load_key_and_certificates(p12, None)
+    assert cert == a_cert
+    assert certs == [b_cert, c_cert]
+
+    # The ordering in the PKCS#12 file itself should also match.
+    a_idx = p12.index(a_name.encode("utf-8"))
+    b_idx = p12.index(b_name.encode("utf-8"))
+    c_idx = p12.index(c_name.encode("utf-8"))
+
+    assert a_idx < b_idx < c_idx
-- 
2.30.2