python-cryptography/0001-fix-pkcs12-parse-order...

131 lines
4.3 KiB
Diff

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/4] 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.31.1