[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [VOTE} Merge svn-auth-x509 branch to trunk?

From: Ben Reser <ben_at_reser.org>
Date: Wed, 28 Jan 2015 21:48:22 -0800

On 1/28/15 6:48 PM, Evgeny Kotkov wrote:
> 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.

I've attached a patch that adds an x509-parser command line tool that lets you
use our X.509 parser easily against arbitrary files (or stdin if you don't pass
a file on the command line). Which makes it quick and easy to test stuff like

> 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.

Yup I'd come to the same conclusion. I was writing up an email but you beat me
to it. Being liberal in what we accept here seems to be fine.

> I came up with the attached patch for /branches/svn-auth-x509.

Patch is fine with me. Although I'd prefer that the subject on the test
includes something that describes what it's for. In this case I'd put
www.example.com in the SAN and put v1withv3ext.example.com in the CN.

> 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. [3].

We don't parse it because the parser is just wrong in this case. Something we
inherited from TropicSSL and I didn't realize was wrong. That said the
specific certificate we're talking about is of a very questionable formatting.
 However, I agree with Google's view [1] that it's probably not something worth
worrying about because the subject/issuer is really only used for display or
equality testing in our context that this shouldn't be treated as a serious error.

I've attached a patch that fixes this. Which would have this commit message:

Fix the X.509 parser to support multiple Relative Distinguished Names (RDN).

Certificates of this nature can be somewhat questionable as to their validity,
but for our purposes it's irrelevant and sometimes people generate certificates
this way. So simply accept them and ignore the minor semantic difference.

* subversion/libsvn_subr/x509parse.c
  (x509_get_attribute): New function.
  (x509_get_name): Remove the code that went into x509_get_attribute() and
    iterate over the members of the RDN set. Adjust some variables and
    comments variables to be clearer in the process.

* subversion/tests/libsvn_subr/x509-test.c
  (cert_tests): Add the cert from Chromium's test suite for this.

I'd like to generate our own certificate for this test but I haven't tried to
figure out how to generate such a certificate.

[1] https://code.google.com/p/chromium/issues/detail?id=101009

Received on 2015-01-29 06:49:28 CET

This is an archived mail posted to the Subversion Dev mailing list.