search

zenhack / simp_le

Simple Let's Encrypt client
GNU General Public License v3.0
224 stars 38 forks source link

Retain order of domains #30

Closed zenhack closed 7 years ago

zenhack commented 7 years ago

Issue by zx2c4 Sunday Feb 28, 2016 at 02:19 GMT _Originally opened as https://github.com/kuba/simp_le/pull/80_

This ensures that the first domain specified on the command line will be the domain that winds up in the CN field. Otherwise, users have no way of controlling this.

This also supports Python 2.6, which is an improvement over the prior pull request. It also correctly tames pylint for an extremely widespread conditional import pattern.

It should pass all tests on Travis.

Fixes: kuba/simp_le#72 Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com

_zx2c4 included the following code: https://github.com/kuba/simp_le/pull/80/commits_

zenhack commented 7 years ago

Comment by kuba Sunday Feb 28, 2016 at 15:59 GMT

This is missing any kind of tests.

zenhack commented 7 years ago

Comment by zx2c4 Sunday Feb 28, 2016 at 16:57 GMT

Okay, now it locally disables for the linter. Good thinking.

I just took a look at how you're implementing the test suite. I can struggle and grunt to add the tests to the test suite here, and in the end I'm sure it won't fit your criteria for inclusion. Would you rather we do some iterations of me struggling, or could you just add this in five minutes yourself?

zenhack commented 7 years ago

Comment by kuba Sunday Feb 28, 2016 at 17:21 GMT

I could try to write tests, but not in foreseeable future - unfortunately I'm extremely busy atm. If you're fine with waiting a couple of weeks before this get merged, then OK...

buchdag commented 7 years ago

I was unable to reproduce what this and #19 aimed to fix : I did a few tests with multiple (up to four) domains on the CLI in different orders and I couldn't get the CN field to be anything else than the first domain specified.

The SAN DNS names in the certificate don't retain the order from the CLI but I don't think that was the issue here (or an issue at all).

buchdag commented 7 years ago

@cpu > If I understand correctly, boulder currently derive the Subject CN from the SAN if no CN was provided in the Certificate Signing Request. How does that work exactly ? Does boulder just pick the first domain in the SAN and populate the CN with it ?

cpu commented 7 years ago

If I understand correctly, boulder currently derive the Subject CN from the SAN if no CN was provided in the Certificate Signing Request.

@buchdag That's correct. We currently promote a SAN to the CN if there isn't a CN specified in the CSR.

How does that work exactly ? Does boulder just pick the first domain in the SAN and populate the CN with it ?

That's correct. Boulder picks the first SAN entry and makes it the CN as well. Presently that's controlled by a flag that is always set to true in staging/production. In the future after a baseline requirements update we'll likely support issuing certificates without a CN and only SANs and this flag will change.

Hope that helps!

buchdag commented 7 years ago

That helps a ton. The ongoing work at boulder / certbot / LE is really invaluable. As is your info on boulder/le inner working.

If I'm still correct, as per RCF5280, having a (the) FQDN as Subject CN has been deprecated for ... well a while, and Boulder is going to deprecate it too in the near future, right ?

cpu commented 7 years ago

If I'm still correct, as per RCF5280, having a (the) FQDN as Subject CN has been deprecated for ... well a while, and Boulder is going to deprecate it too in the near future, right ?

@buchdag Yup, exactly! That's why there's the flag to control the behaviour. I believe allowing issuance of certs with only SANs and no subject CN is blocked on a baseline requirements ballot. I can't recall offhand what the problem was but remember that we're waiting on a CA/Browser forum update to go ahead.

buchdag commented 7 years ago

Okay, so in my opinion there isn't anything to fix here.

zenhack commented 7 years ago

Works for me, closing.