Ben Reser <b...@reser.org> writes:
> Not sure how you're processing those cert files (and you have to be doing
> something since our parser only accepts DER certs and most of those are PEM)
> but that's where the problem could be. I'm writing a quick little command
> line tool that can read PEM, raw base64 encode certs (i.e. PEM without the
> header/footer) and DERs and feed them through our parser. Which should make
> it trivial to test arbitrary certificates.
I grepped everything between BEGIN/END certificate markers and (with a bit
of post-processing) plugged the results into the 'x509_test cert_tests[]'
data set. As a consequence, the PEM → DER conversion happened within the
test_x509_parse_cert() test.
> This may be a case of our parser being overly paranoid about the input data
> being consistent. The buffer you pass to svn_x509_parse_cert() must end at
> exactly the place where the initial SEQUENCE says it should. Put an extra
> byte
> on the end and our parser will error out.
As it turns out, almost every failing certificate has X.509 Version: 1 (0x0),
but also happens to have V3 extensions. These are ill-formed per RFC 5280
s. 4.1 [1], but we could encounter them in the real world. For instance,
OpenSSL 1.0.1f does produce such certificates when signing end-entities
with the ee.cnf config from [2] (might be a bug). Googling shows at least
one thread with a person having an X.509 V1 certificate with V3 extensions.
I tend to think that being too strict in this aspect might be harmful. We
only want to display data in a certificate, and I think we should be able
to parse these certificates. OpenSSL and Microsoft CryptoAPI do not error
out on them and show what's there in V3 extensions even if the certificate
itself is a V1. For example, with 'openssl x509 -text -noout' I get:
Data:
Version: 1 (0x0)
Serial Number: 15 (0xf)
...
X509v3 extensions:
X509v3 Subject Alternative Name:
DNS:b.example.com
I came up with the attached patch for /branches/svn-auth-x509. Log message:
[[[
On svn-auth-x509 branch: Try to parse issuerUniqueID, subjectUniqueID and
extensions for every X.509 certificate version (v1, v2 and v3).
If they aren't present, we are fine, but we don't want to throw an error if
they are. v1 and v2 certificates with the corresponding extra fields are
ill-formed per RFC 5280 s. 4.1, but we suspect they could exist in the real
world. Other X.509 parsers (e.g., within OpenSSL or Microsoft CryptoAPI)
aren't picky about these certificates. As long as we are only willing to
display the certificate data in the 'svn auth' command, we can also be less
strict about them.
* subversion/libsvn_subr/x509parse.c
(svn_x509_parse_cert): Try to parse issuerUniqueID, subjectUniqueID and
extensions for all known X.509 versions (v1, v2, v3).
(x509parse_get_hostnames): Do not check CRT->DNSNAMES for null, because
it is no longer necessary.
subversion/tests/libsvn_subr/x509-test.c
(cert_tests): Add a new test case.
]]]
Now, the only remaining fail is the 'fail-2.cer.txt' from my previous e-mail,
which has multiple AttributeTypeAndValue entries in RelativeDistinguishedName
(RDN). I am not yet sure why do we fail to parse the certificate, but the
certificate itself is structurally valid per RFC 5280 s. 4.1.2.4 [3]. Here
is what 'openssl asn1parse -dump' says about it (output trimmed):
188:d=3 hl=2 l= 111 cons: SET
190:d=4 hl=2 l= 9 cons: SEQUENCE
192:d=5 hl=2 l= 3 prim: OBJECT :countryName
197:d=5 hl=2 l= 2 prim: PRINTABLESTRING : ...
201:d=4 hl=2 l= 15 cons: SEQUENCE
203:d=5 hl=2 l= 3 prim: OBJECT :organizationName
208:d=5 hl=2 l= 8 prim: UTF8STRING : ...
218:d=4 hl=2 l= 22 cons: SEQUENCE
220:d=5 hl=2 l= 10 prim: OBJECT :domainComponent
232:d=5 hl=2 l= 8 prim: IA5STRING : ...
242:d=4 hl=2 l= 26 cons: SEQUENCE
244:d=5 hl=2 l= 3 prim: OBJECT :commonName
249:d=5 hl=2 l= 19 prim: UTF8STRING : ...
270:d=4 hl=2 l= 29 cons: SEQUENCE
272:d=5 hl=2 l= 3 prim: OBJECT :organizationalUnitName
277:d=5 hl=2 l= 22 prim: UTF8STRING : ...
[1] https://tools.ietf.org/html/rfc5280#section-4.1
[2] https://src.chromium.org/svn/trunk/src/net/data/ssl/scripts/ee.cnf
[3] https://tools.ietf.org/html/rfc5280#section-4.1.2.4
Regards,
Evgeny Kotkov
Index: subversion/libsvn_subr/x509parse.c
===================================================================
--- subversion/libsvn_subr/x509parse.c (revision 1655502)
+++ subversion/libsvn_subr/x509parse.c (working copy)
@@ -1010,7 +1010,7 @@ x509parse_get_hostnames(svn_x509_certinfo_t *ci, x
{
ci->hostnames = NULL;
- if (crt->dnsnames && crt->dnsnames->nelts > 0)
+ if (crt->dnsnames->nelts > 0)
{
int i;
@@ -1155,18 +1155,19 @@ svn_x509_parse_cert(svn_x509_certinfo_t **certinfo
* extensions [3] EXPLICIT Extensions OPTIONAL
* -- If present, version
shall be v3
*/
- if (crt->version == 2 || crt->version == 3)
- SVN_ERR(x509_get_uid(&p, end, &crt->issuer_id, 1));
+ crt->dnsnames = apr_array_make(scratch_pool, 3, sizeof(x509_buf *));
- if (crt->version == 2 || crt->version == 3)
- SVN_ERR(x509_get_uid(&p, end, &crt->subject_id, 2));
+ /* Try to parse issuerUniqueID, subjectUniqueID and extensions for *every*
+ * version (X.509 v1, v2 and v3), not just v2 or v3. If they aren't present,
+ * we are fine, but we don't want to throw an error if they are. v1 and v2
+ * certificates with the corresponding extra fields are ill-formed per RFC
+ * 5280 s. 4.1, but we suspect they could exist in the real world. Other
+ * X.509 parsers (e.g., within OpenSSL or Microsoft CryptoAPI) aren't picky
+ * about these certificates, and we also allow them. */
+ SVN_ERR(x509_get_uid(&p, end, &crt->issuer_id, 1));
+ SVN_ERR(x509_get_uid(&p, end, &crt->subject_id, 2));
+ SVN_ERR(x509_get_ext(crt->dnsnames, &p, end));
- if (crt->version == 3)
- {
- crt->dnsnames = apr_array_make(scratch_pool, 3, sizeof(x509_buf *));
- SVN_ERR(x509_get_ext(crt->dnsnames, &p, end));
- }
-
if (p != end)
{
err = svn_error_create(SVN_ERR_ASN1_LENGTH_MISMATCH, NULL, NULL);
Index: subversion/tests/libsvn_subr/x509-test.c
===================================================================
--- subversion/tests/libsvn_subr/x509-test.c (revision 1655502)
+++ subversion/tests/libsvn_subr/x509-test.c (working copy)
@@ -437,6 +437,39 @@ static struct x509_test cert_tests[] = {
"x509v1.example.com",
"5730dd65a7f77fdf0dfd90e5a53119f38854af29"
},
+ /* X.509 v1 certificate with an X.509 v3 Subject Alternative Name
+ * extension. Although these are ill-formed per RFC 5280 s. 4.1, we
+ * suspect that they could exist in the real world. Make sure we do
+ * not error out, and that we pick up SAN (b.example.com) from the
+ * extension. */
+ { "MIIDLzCCAhcCAQ8wDQYJKoZIhvcNAQEFBQAwKzEpMCcGA1UEAwwgSW50ZXJuZXQg"
+ "V2lkZ2l0cyBJbnRlcm1lZGlhdGUgQ0EwHhcNMTUwMTI5MDAzMzU1WhcNMTYwMTI5"
+ "MDAzMzU1WjByMQswCQYDVQQGEwJVUzETMBEGA1UECAwKV2FzaGluZ3RvbjETMBEG"
+ "A1UEBwwKTm9ydGggQmVuZDEhMB8GA1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkg"
+ "THRkMRYwFAYDVQQDDA1hLmV4YW1wbGUuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOC"
+ "AQ8AMIIBCgKCAQEAs0hj2xPRQZpecqk0Ih1l4juAuQZeSgv3yD/VtSq/9sTBH6iA"
+ "4XjJQcHROYxYaK0QS/qlCjpl+Q3mOaVIu+59TLy3T2YVgqMYmgB453ntuJPkdF1C"
+ "fJ2j19YAQZHHdOFaP1G+auBwjmHns3+MkG4s7EPuJP7TBCcSFlOmz5D4GUui3NVG"
+ "LBYUog1ZhF4oe/7d4jc2Cn8uypNT/Hc1ViIlCT4rFoAirv9Uob+4zjQ3Z18I1Ql1"
+ "t8oszVCj3kKDboEty2RduwPLx/2ztWYBCvFhd49JGdi/nzMi+j2d5HCI3V8W06pN"
+ "mvrVU4G0ImVRa8wpmQCSm2Tp0s42FAVHWw8yMwIDAQABoxwwGjAYBgNVHREEETAP"
+ "gg1iLmV4YW1wbGUuY29tMA0GCSqGSIb3DQEBBQUAA4IBAQDI/n0NYakuRP/485/A"
+ "dan71qBy3sljjOreq71IfBdtq+GEjCL1B0TD0V338LXki9NicCLeD/MWfceDjV0u"
+ "AjPTxaZEn/NWqXo0mpNC535Y6G46mIHYDGC8JyvCJjaXF+GVstNt6lXzZp2Yn3Si"
+ "K57uVb+zz5zAGSO982I2HACZPnF/oAtp7bwxzwvBsLqSLw3hh0ATVPp6ktE+WMoI"
+ "X75CVcDmU0zjXqzKiFPKeTVjQG6YxgvplMaag/iNngkgEhX4PIrxdIEsHf8l9ogC"
+ "dz51MFxetsC4D2KRq8IblF9i+9r3hlv+Dbf9ovYe9Hu0usloSinImoWOw42iWWmP"
+ "vT4l",
+ "C=US, ST=Washington, L=North Bend, O=Internet Widgits Pty Ltd, "
+ "CN=a.example.com",
+ "2.5.4.6 2.5.4.8 2.5.4.7 2.5.4.10 2.5.4.3",
+ "CN=Internet Widgits Intermediate CA",
+ "2.5.4.3",
+ "2015-01-29T00:33:55.000000Z",
+ "2016-01-29T00:33:55.000000Z",
+ "b.example.com",
+ "47fa5c76fee6e21e37def6da3746bba84a5a09bf"
+ },
{ NULL }
};
