Strick PKCS7 PEM Block Check Issue after version 0.45.1 + BER Warning also triggered.

[giantg]

Hi,

I have a certificate issuance and renewal script that leverages both this cryptography library (cryptography.hazmat.primitives.serialization.pkcs7) as well as pkcs7csr to both request and renew certificates from a Microsoft ActiveDirectory Certificate Services server.

There appears to be two issues:

1. Line 754 of pkcs7.rs (version 0.45.1) has a strict check of the PEM block tag for -----BEGIN PKCS7-----.
   

   cryptography/src/rust/src/pkcs7.rs

   Lines 754 to 760 in 8ea28e0

   	if pem_block.tag() != "PKCS7" {
   	return Err(CryptographyError::from(
   	pyo3::exceptions::PyValueError::new_err(
   	"The provided PEM data does not have the PKCS7 tag.",
   	),
   	));
   	}

   
   Unfortunately, it appears the HTTPS call made to the Microsoft Certificate Services P7B endpoint: https://hostname.domain.tld/certsrv/certnew.p7b?ReqID=CACert&Renewal=1&Enc=b64 to retreive the CA's chain of authority PKCS7 payload is erroneously returned with a -----BEGIN CERTIFICATE----- PEM block tag.
   While OpenSSL pkcs7 -in [payload] handles parsing said PKCS7 payload returning two certificates. This new strict check throws an unhandled exception breaking my script. I tried commenting out lines 754-760 (above) and when I did things were able to complete (without an exception), but with the BER warning taking me to my second issue.

2. UserWarning: PKCS#7 certificates could not be parsed as DER, falling back to parsing as BER. Please file an issue at https://github.com/pyca/cryptography/issues explaining how your PKCS#7 certificates were created. In the future, this may become an exception. as others have mentioned in ticket Remove BER in PKCS#7 and PKCS#12 #12936.

Here is the raw response from the Microsoft ActiveDirectory Certificate Services Server (https://hostname.domain.tld/certsrv/certnew.p7b?ReqID=CACert&Renewal=1&Enc=b64):

-----BEGIN CERTIFICATE-----
MIIOEAYJKoZIhvcNAQcCoIIOATCCDf0CAQExADALBgkqhkiG9w0BBwGggg3lMIIH
CzCCBPOgAwIBAgITOgAAAANqxkGcqRq+ewAAAAAAAzANBgkqhkiG9w0BAQsFADCB
rjELMAkGA1UEBhMCVVMxFTATBgNVBAgTDFBlbm5zeWx2YW5pYTEXMBUGA1UEBxMO
S2VubmV0dCBTcXVhcmUxKjAoBgNVBAsTIUluZm9ybWF0aW9uIFRlY2hub2xvZ3kg
RGVwYXJ0bWVudDEZMBcGA1UEChMQTG9uZ3dvb2QgR2FyZGVuczEoMCYGA1UEAxMf
TG9uZ3dvb2QgR2FyZGVucyBSb290IEF1dGhvcml0eTAeFw0yNTEwMDYyMjU5NTJa
Fw0zNTEwMDYyMzA5NTJaMIG2MQswCQYDVQQGEwJVUzEVMBMGA1UECBMMUGVubnN5
bHZhbmlhMRcwFQYDVQQHEw5LZW5uZXR0IFNxdWFyZTEZMBcGA1UEChMQTG9uZ3dv
b2QgR2FyZGVuczEqMCgGA1UECxMhSW5mb3JtYXRpb24gVGVjaG5vbG9neSBEZXBh
cnRtZW50MTAwLgYDVQQDEydMb25nd29vZCBHYXJkZW5zIEludGVybWVkaWF0ZSBB
dXRob3JpdHkwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDNOZas31QW
si7h4/cLiOW4rhL8SsSYBcCgx6o48TQ5aNyxxGXrIAB55Ru2/ZYHS4BNMOnlDNQP
jtI7mUNdde2nKGcEO21yaTI8uXuUcCw5czQJ4M3p+0BvbOF7/Eq+7yRTvHIQ1CeF
NAGxFtRSjTkqTuxq/IWwuVPtcwcVuaL78DVWPd5IoCHfYBfHJ4goYQT74ouz/65l
whyf6W++KulCtnM25Up16rR9ldWbaA1F/zpOIsQxCoDjhyapLEKH3cMzg+po9MhH
QWpgQ9uWrr6hAQwsEOl7AmvTO3VygKN+IND7nJJ9Anp/rGjI6+Rii7XDED5RvWJ6
sTkxYg1kAsIMGoTIEuIZz5VwK1WyANQc1mtUnd76S6CMyZpyf3ciSM2S0ZLb1dbV
d/RJV1aogCkSLeycEHYnvirU6ECaBHQedKOM6sLTjoGW2xoGnwH/ag+KrqiblC6R
2YgGvNAOewCbxpJunJfi3UTNJbEoLGOPHjqRa0IxDuGZxIQRq3XHz8xyaL1k3r6u
s+MsV/kczBgfubX//k0/4focihQBQdzaHUW4LO9k8eMNbPy1tq7fXlLDxz/hrM7n
5OYlrsqkjiSfCaxFtmEXbCBP5rMn/aIx4AFJ9nUlFKFaerr13f8dBHhXyN7dMzF7
yYuwNeX9+u4YNhdxjf2nN1I6Gz9AB+mMKQIDAQABo4IBFjCCARIwEAYJKwYBBAGC
NxUBBAMCAQEwIwYJKwYBBAGCNxUCBBYEFIr5Mw+NL44WVO9ftZtotWQNIzgpMB0G
A1UdDgQWBBTzfK0TVUMwRII9UT/aE3VqaNUF7jAZBgkrBgEEAYI3FAIEDB4KAFMA
dQBiAEMAQTALBgNVHQ8EBAMCAYYwDwYDVR0TAQH/BAUwAwEB/zAfBgNVHSMEGDAW
gBRbAGTa/Cvzl0tb+xeOENdyDr7F1zBgBgNVHR8EWTBXMFWgU6BRhk9odHRwOi8v
cGtpLmhxLmxvbmd3b29kZ2FyZGVucy5vcmcvcGtpL0xvbmd3b29kJTIwR2FyZGVu
cyUyMFJvb3QlMjBBdXRob3JpdHkuY3JsMA0GCSqGSIb3DQEBCwUAA4ICAQBV/O0B
0hEEZJTKoN3PAH0/ENQlTl2t7VvGnFl7nIFO/ZmDKB6BP6DoAjlFUwHQ/c+tU+0V
p5Kt8YIxeLn9UmoWkLHs+zcuhPMLYgMPo1alP4jxuZX4UNhplg1D3bt1Qn4ySpcn
+A3CpBSTyoJ4DucYaogXd+im6fH6e2k/dnUKSlbekfa6EbVQ5YoVCER/OsnX3WIX
YHUvkcJ2H0BQj1/obngjrdKVq3LRAmEcER7/sqOGP1pDPcrD92/pWhqDXQ18k5a4
zo9gvMwXhAAjZsKMd0o2EvzfAmoQWnoPLnYrC4WxpLiyuD332hL8lo9+l+qCxcLY
ajHKb0i9CJSEQRcAAbw2XBwZCUrAOM2M7IkasrtMAYYuYSgEYnHNrJnEd8ECzXT5
74f7wAzXNkn/jP0jjSkrM0NIIB0qaI7geNg5RHjXAWyvfWtz9b8LGSJZe8QVso1K
hTS9Dy6K99dbErqm1yu+U2/hPevQNoKxF4AqOcnPdTMZrRGURx2TEETUV7nT7xbt
9DPLG7D/LTeOHVQllU+2rksKjzVqPVmZbN5i9tvf4DbUh5Zhu5NrZlQbwMLrWu4h
23rojDsrl4A/uX5RIZyVzmfIC+RaHQNFvU8T1YCpAK3iZzvDQOkXP0apvg2rlqtV
9WW2uPIvDNgSS6PDfZ1kYz3V+ZPrwL8TetsvYDCCBtIwggS6oAMCAQICEDk+A+zg
AJqBS8cjDsPNGJIwDQYJKoZIhvcNAQELBQAwga4xCzAJBgNVBAYTAlVTMRUwEwYD
VQQIEwxQZW5uc3lsdmFuaWExFzAVBgNVBAcTDktlbm5ldHQgU3F1YXJlMSowKAYD
VQQLEyFJbmZvcm1hdGlvbiBUZWNobm9sb2d5IERlcGFydG1lbnQxGTAXBgNVBAoT
EExvbmd3b29kIEdhcmRlbnMxKDAmBgNVBAMTH0xvbmd3b29kIEdhcmRlbnMgUm9v
dCBBdXRob3JpdHkwIBcNMTYwNTEyMDA1MDE1WhgPMjA2NjA1MTIwMTAwMTBaMIGu
MQswCQYDVQQGEwJVUzEVMBMGA1UECBMMUGVubnN5bHZhbmlhMRcwFQYDVQQHEw5L
ZW5uZXR0IFNxdWFyZTEqMCgGA1UECxMhSW5mb3JtYXRpb24gVGVjaG5vbG9neSBE
ZXBhcnRtZW50MRkwFwYDVQQKExBMb25nd29vZCBHYXJkZW5zMSgwJgYDVQQDEx9M
b25nd29vZCBHYXJkZW5zIFJvb3QgQXV0aG9yaXR5MIICIjANBgkqhkiG9w0BAQEF
AAOCAg8AMIICCgKCAgEAvVqnut2jQYm3kXb8DcGEqJmePlYHgE9DBxba81ba34w2
ouLS+SyZhr1g189fKlA1yOf1sKG9mZsShLkwGiitnzuzD+xBuoXd/KOT9HAws9rY
PnBopi0tur8gmP9BClNswhhmZUS2ZnOQ6G8JxlfqcE4hrGN3HAKb94Nqn7IAYkuF
xJ6hvd65ubewHL4ek+wkFzNxQIznpofLvb371FhlcZmuEgCk0B0C/55qQrlp30Yl
sl2qYjkb+1D93it5w10OZUCOc81x2jI8R3YRJGfVcUfaHzFgSZQ6GuCM6Bqq/sK3
o3iUXAolPTYAfP/T67j63KQNVZryAk7x4bHTstLd918donKzkGfvdhQEVdCnG73F
0FGeaPa6mPrqa0YffyZcs+g6n99HXj4dnKnv/VsWp1sMoc0MX1ZmVrtdPITX9cNZ
/txOj3wq47kfcl0H752Vz8UPgB4SQZi3kfFiUSwm4LnrQ4n1X+fV0sGTnZDoZ/k6
DNLMwqK3IPhDbZnNgwFT2CJ6oBqnX2sHMvjrJpnGPJWsWzo0ooUvS3PeEGxCIwkk
IztTGNBUAdHlQFAZ/uj+Dsi1soGZx4h5Q/GGwPOFw31Mz6uqMGR2OA4rDsUgOrYE
WB453+IMMPLok10HUSQhoq4YxSzUWDu3SZfdmjh6nP8aoTDwVdS5DcTmKDiJnEcC
AwEAAaOB5zCB5DALBgNVHQ8EBAMCAYYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4E
FgQUWwBk2vwr85dLW/sXjhDXcg6+xdcwEAYJKwYBBAGCNxUBBAMCAQAwgZIGA1Ud
IASBijCBhzCBhAYIKgMEiy9DWQUweDA6BggrBgEFBQcCAjAuHiwATABlAGcAYQBs
ACAAUABvAGwAaQBjAHkAIABTAHQAYQB0AGUAbQBlAG4AdDA6BggrBgEFBQcCARYu
aHR0cDovL3BraS5ocS5sb25nd29vZGdhcmRlbnMub3JnL3BraS9jcHMudHh0ADAN
BgkqhkiG9w0BAQsFAAOCAgEAlcf+zC85AD57vXAF/b85KcKJ5UgftBew7NszElex
E0RMNA2b9qwjIz/aHQANNfadLdZYMHkeUzsdj9QCW72jBOnWrJps/aeUiyzOwGVF
0GFlHssHv40YY8wC1JpA/iFqMSes98LT3+hG/2updo/iVF4X7rgRmHePxLi67NMa
ecS37wnsbb86E/gIR029xhKSDnewjtp0ODZ16zG9rHVJlxG2hIN/GcsvXDqP3nf2
Vd4SjUM0TGLwmUM1v0X9f2H3lQCGIrOjvBvSFgEEiXJGaWmL5iaJSliJRniRwBFq
LU5oDPng18uoVoepIvQS4wQpzcGUuMTKCuGfFTwpY8mez+9BgiBb+xC2hC02n6lt
lmXMx572KTZM8i39N0EZTCXwgPC3svTxXuFRDGgOwlfXh1StsU7wsL3mAKdbiBDg
NukKgvW/AsTKTZVTL6pqSBaDj4gSokunWya56eUVVOwGbA4UJvqsmW8E3D8Ftys8
qZnQc0Nc2qkTI4jxTqlLpvMisy7CvTcxm/9B4LoZyqu2rUJjWTTRK62QKFLo30xA
wBCnxsX7de96uDyB5HsdNovs5V89B5haeMcG99y+tFp5DLKUvqoNZNpYsluMceqN
a1rHluEOyPBiV4v70vWNWV6yfRAJ4fLW+gCoVh/IOrbNXSHNCfkBDQoyuWNtz1C0
dZ4xAA==
-----END CERTIFICATE-----

Writing that to a file named samplepkcs7.pem and running openssl pkcs7 -print -in samplepkcs7.pem -noout does appear to show a valid PKCS7 structure.

I realize that rather than comment out the strict check I could probably insert a search/replace or regex replace of BEGIN/END CERTIFICATE for BEGIN/END PKCS7 PEM Block tag, but that feels like a bit of a hack (which I'm not opposed to as it does appear that Microsoft Certificate Services server does not appear to be doing the correct behavior). However, I'm wondering if this stricter check could be relaxed a bit and just fail if the actual PEM data does not represent a valid PKCS7 payload.

[alex]

Huh, so they are genuinely serving a PKCS#7 bundle with a non-PKCS7 PEM delimeter. Fascinating!

In the extremely short term, doing a find/replace is probably the easiest workaround for you. @reaperhulk what are your thoughts on if we should allow the (nonsense) PEM delimieter with a warning?

[giantg]

Thank you @alex for such a fast response!

I took your suggestion for the time being and applied the following change to my internal script:

 def complete_request(pemstore,new_cert,pkcs7_chain_pem_bytes):
    # Hack to fix bug in ActiveDirectory Certificate Services PKCS7 handler...
    pkcs7_chain_pem_bytes = re.sub(b"(BEGIN|END) CERTIFICATE", b"\\1 PKCS7", pkcs7_chain_pem_bytes)

That has at least brought things back to working with warning!

Thanks for even reading / considering my request!

[reaperhulk]

I’m not thrilled about allowing this with warning MS committing to a fix. Even after that we’d have a very long wait before we could reject them.

On the other hand, we have an explicit loader so we can contain this hack quite well, so from that perspective it’s not an undue burden.

[giantg]

I hear you on the potential to get MS to commit to a fix. To be honest, most of the ActiveDirectory Certificate Services stuff is still ASP Classic (Probably VB + COM/C++ vs VB.NET) and I imagine has not been touched much in a long time.

I was able to address my use in the meantime using the RE above (and I imagine sometimes it's better to have the hack in the application programmer's code vs. polluting library code with it).

I appreciate you both even considering my questions. Thank you!

As far as the PKCS7 data generating the PKCS#7 certificates could not be parsed... UserWarning should I just share the PKCS7 from my case on issue #12936 instead of asking that part here?

[alex]

Yes, let's handle those separately.