KNOX-3333 - Update Letsencrypt staging certs

[moresandeep]

(It is very important that you created an Apache Knox JIRA for this change and that the PR title/commit message includes the Apache Knox JIRA ID!)

KNOX-3333 - Update Letsencrypt staging certs

What changes were proposed in this pull request?

Letsencrypt staging certs were updated with few more, adding all.

How was this patch tested?

This patch was tested locally.

[github-actions]

Test Results

22 tests   22 ✅  1s ⏱️
 1 suites   0 💤
 1 files     0 ❌

Results for commit cb6c291.

♻️ This comment has been updated with latest results.

[smolnar82]

I don't think this is a serious security vulnerability as the added certificates are public CA roots from Let's Encrypt's official staging hierarchy, not arbitrary certificates.

However, I've a few questions:

• What use case requires trusting staging roots?
  • Is there a real customer scenario?
  • Is this only for automated testing?
• Can this be made optional?
  • For example via build argument or environment variable.
  • Then test users can enable it while production users keep a smaller trust set.
• Are all these roots necessary?
  • The patch adds multiple generations (X1, X2, YE, YR, cross-signed variants).
  • It may be worth confirming that all are actually needed.

Adding staging roots increases the set of trusted certificate authorities and allows Knox to trust certificates issued by Let's Encrypt's testing infrastructure. Can we clarify the use case and whether this trust should be enabled only for testing environments rather than all Docker deployments (see my question above about making them optional)?

[moresandeep]

I don't think this is a serious security vulnerability as the added certificates are public CA roots from Let's Encrypt's official staging hierarchy, not arbitrary certificates.

However, I've a few questions:

• What use case requires trusting staging roots?

  • Is there a real customer scenario?
  • Is this only for automated testing?

• Can this be made optional?

  • For example via build argument or environment variable.
  • Then test users can enable it while production users keep a smaller trust set.

• Are all these roots necessary?

  • The patch adds multiple generations (X1, X2, YE, YR, cross-signed variants).
  • It may be worth confirming that all are actually needed.

Adding staging roots increases the set of trusted certificate authorities and allows Knox to trust certificates issued by Let's Encrypt's testing infrastructure. Can we clarify the use case and whether this trust should be enabled only for testing environments rather than all Docker deployments (see my question above about making them optional)?

Thanks @smolnar82

1. Is there a real customer scenario? -> Yes, in enterprises there are staging environments which do not have prod certs. This change will help users test Knox in staging without a need to add staging certs into keystore (which they do not have to do in prod) mimicking prod env.
2. Can this be made optional? -> This is interesting, the idea is to make sure the prod and staging are identical in all respects so making it optional means they will have to have a flag that they use to turn this ON in stage and OFF in prod so there will be a change in config although not a big one.
3. They are not test related changes these are runtime/config related changes. These changes do not affect tests.
4. Are all these roots necessary? -> Unfortunately yes. we do not know and cannot dictate what root CA users are going to use for staging.
5. It may be worth confirming that all are actually needed. -> Yes, this came up in an internal investigation, we were using a different gen and got switched to other.

[moresandeep]

@smolnar82 I put the staging certs behind flag (by default false) I also added some truststore hardening.

[moresandeep]

@sneethiraj when you have time can you do a quick review, thanks!

[smolnar82]

I'd call this ENV variable IMPORT_LETS_ENCRYPT_STAGING_CERTS (to be honest the DEFAULT word inside made me think what this is about).
Additionally: you indicate the default is true, but this isn't the case (see below, line 47 sets it to false if not configured).

[smolnar82]

downloadLetEncryptStagingCerts maybe?

[smolnar82]

This is a nice addition, I like that a lot :)