131 lines
4.3 KiB
Diff
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/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
|
||
|
|