srvrco / getssl

obtain free SSL certificates from letsencrypt ACME server Suitable for automating the process on remote servers.
GNU General Public License v3.0
2.11k stars 378 forks source link

FULL_CHAIN_INCLUDE_ROOT innefective #768

Open szolnokit opened 2 years ago

szolnokit commented 2 years ago

If FULL_CHAIN_INCLUDE_ROOT not defined (default: false) or FULL_CHAIN_INCLUDE_ROOT="false" "fullchain.pem" always contains root CA.

No difference when FULL_CHAIN_INCLUDE_ROOT defined or value "true" or "false". "fullchain.pem" always contains 3 cert:

  1. domain cert
  2. R3 (intermediate)
  3. ISRG Root X1 (root CA)
szolnokit commented 2 years ago

Patched: https://github.com/srvrco/getssl/pull/770/commits/0925dcd21fd32d8cab4587d561f8c3a72ab95837

szolnokit commented 2 years ago

This is critical bugfix!

Without this bugfix, the bahvior of fullchain was like FULL_CHAIN_INCLUDE_ROOT="true" always.

After apply this bugfix, the fullchain.pem handling will be different, because the FULL_CHAIN_INCLUDE_ROOT default value is "false". In this case, after the next getssl update, the root-CA will be removed "silently" from fullchain because the nature of this bug. This would be a "silent" problem.

Dear author, please consider: Change dfault value of FULL_CHAIN_INCLUDE_ROOT to "true" in paralell with this bugfix. In this case, applying this bugfix (mostly) will not affect the fullchain.

timkimber commented 2 years ago

Hi @szolnokit

Thanks for finding this, but according the BashFAQ (http://mywiki.wooledge.org/BashFAQ/031) and my testing, it makes no difference whether the test uses = or ==

With what OS and version of bash did you see this problem?

(I'll merge the PR as for consistency it's better if all the tests use the same style)

szolnokit commented 2 years ago

Hmmm....

I made a small test with "==" and "=" regarding Wikipedia. Same result.

I'm tested again the getssl with "==" and "=". Same result now. Currently working good with only one "=". But I updated many things after the patch. I can't tell you what bash version I used before.

Because I definetely remember before open this ticket, root-CA always included in fullchain.pem I tested many times. And I was happy when found the "bogus" if-then...

Sorry...

githubRover commented 2 years ago

@szolnokit The default chain from Let's Encrypt is the "long chain" and looks like this:

Certificate chain
 0 s:/CN=lencr.org
   i:/C=US/O=Let's Encrypt/CN=R3
 1 s:/C=US/O=Let's Encrypt/CN=R3
   i:/C=US/O=Internet Security Research Group/CN=ISRG Root X1
 2 s:/C=US/O=Internet Security Research Group/CN=ISRG Root X1
   i:/O=Digital Signature Trust Co./CN=DST Root CA X3

The first is the leaf but the other two are intermediates. Their names are misleading.

As I understand the full_chain_include_root option that would attach the CA Root cert to the chain. This CA Root is normally in the client's CA cert store. But, for some (lesser used) servers they want it included with the chain you supply.

Let's Encrypt also have a "short chain" which is an alternate option. It omits the last one from above. Which chain you use depends on what clients you want to support. There is much discussion about these two chains in the Let's Encrypt community forum.

Hope this helps