Fix P12 parsing for certificates with "Basic constraints" #122056

jportner · 2021-12-27T22:50:50Z

Resolves #122054.

See that issue for a detailed description.

To test, use the P12 file provided in that issue's "Steps to reproduce" section, and verify that Kibana starts without any errors.

This fixes the root cause of the bug. I had to use a yarn dependency resolution to force-upgrade this, as it's a transitive dependency of the @elastic/request-crypto package. I'll open a separate issue in that repo to get it fixed there, too; when that's fixed and upgraded in the Kibana repo we can remove this resolution.

jportner

I'd prefer to change the P12 parser to log a warning when a cert bag is empty. However, we don't have access to a logger when this is being used (while we are reading/validating the config).

Another idea I had is to debug log the fingerprints of each certificate that is loaded when they are consumed (for example, in the Elasticsearch client). That would at least make it easier to diagnose that a certificate is missing when a user is having TLS issues. However, that doesn't cover all of our bases (certificates are also used for the HTTP server).

So, I think this fix is sufficient for now.

jportner · 2021-12-28T01:53:41Z

package.json

@@ -86,6 +86,7 @@
    "**/istanbul-lib-coverage": "^3.2.0",
    "**/json-schema": "^0.4.0",
    "**/minimist": "^1.2.5",
+    "**/node-jose": "^1.1.4",


As mentioned in ad696ff, I had to use a yarn dependency resolution to upgrade this transitive dependency.

We depend on node-jose via @elastic/request-crypto, so I submitted another PR there to upgrade that dependency: elastic/request-crypto#11

Once that is merged and a new version is deployed to the npm registry, I can remove this dependency resolution. I wasn't sure how long that would take but it looks like that might be done tomorrow, so I might be able to upgrade that instead and remove this dependency resolution before merging this PR.

Both options sounds good to me 👍

Updated to @elastic/request-crypto 2.0.0 and node-jose 2.0.0, and removed the dependency resolutions, in fa4eecc.

azasypkin · 2021-12-28T10:50:42Z

ACK: reviewing...

azasypkin

LGTM, tested locally and everything worked as expected. Thanks a lot for the thorough investigation and the fix(es)!

azasypkin · 2021-12-28T11:05:14Z

package.json

@@ -86,6 +86,7 @@
    "**/istanbul-lib-coverage": "^3.2.0",
    "**/json-schema": "^0.4.0",
    "**/minimist": "^1.2.5",
+    "**/node-jose": "^1.1.4",


Both options sounds good to me 👍

Also updates node-jose to 2.0.0. The reason for the major version change is that it drops support for Node 8. By doing this we were able to remove two dependency resolutions in the package.json file.

kibana-ci · 2022-01-04T14:46:07Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💚 Build #15183 succeeded ad696ff

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2022-01-04T14:49:33Z

💔 Backport failed

Status	Branch	Result
❌	8.0	Commit could not be cherrypicked due to conflicts
❌	7.17	Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 122056

…2056) # Conflicts: # yarn.lock

…122262) # Conflicts: # yarn.lock

…122264) # Conflicts: # yarn.lock

kibanamachine · 2022-01-05T05:32:59Z

💔 Backport failed

Status	Branch	Result
❌	7.16	Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 122056

…2056) # Conflicts: # yarn.lock

…122310) # Conflicts: # yarn.lock

elasticmachine · 2022-01-05T22:09:08Z

Pinging @elastic/kibana-security (Team:Security)

jportner added v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 v7.17.0 labels Dec 27, 2021

jportner force-pushed the issue-122054-fix-p12-parsing branch from b177a81 to ad696ff Compare December 27, 2021 22:56

jportner marked this pull request as ready for review December 28, 2021 00:28

jportner commented Dec 28, 2021

View reviewed changes

jportner requested a review from azasypkin December 28, 2021 01:50

jportner commented Dec 28, 2021

View reviewed changes

azasypkin added the release_note:fix label Dec 28, 2021

azasypkin approved these changes Dec 28, 2021

View reviewed changes

Joe Portner added 2 commits January 4, 2022 08:17

Merge branch 'main' into issue-122054-fix-p12-parsing

667a7ca

Update @elastic/request-crypto to 2.0.0

fa4eecc

Also updates node-jose to 2.0.0. The reason for the major version change is that it drops support for Node 8. By doing this we were able to remove two dependency resolutions in the package.json file.

jportner enabled auto-merge (squash) January 4, 2022 13:27

jportner merged commit 75ea37b into elastic:main Jan 4, 2022

jportner deleted the issue-122054-fix-p12-parsing branch January 4, 2022 14:46

jportner added a commit to jportner/kibana that referenced this pull request Jan 4, 2022

Fix P12 parsing for certificates with "Basic constraints" (elastic#12…

02ee74a

…2056) # Conflicts: # yarn.lock

jportner mentioned this pull request Jan 4, 2022

[8.0] Fix P12 parsing for certificates with "Basic constraints" (#122056) #122262

Merged

jportner added a commit to jportner/kibana that referenced this pull request Jan 4, 2022

Fix P12 parsing for certificates with "Basic constraints" (elastic#12…

31a014f

…2056) # Conflicts: # yarn.lock

jportner mentioned this pull request Jan 4, 2022

[7.17] Fix P12 parsing for certificates with "Basic constraints" (#122056) #122264

Merged

jportner added a commit that referenced this pull request Jan 4, 2022

Fix P12 parsing for certificates with "Basic constraints" (#122056) (#…

d6c4e4d

…122262) # Conflicts: # yarn.lock

jportner added a commit that referenced this pull request Jan 4, 2022

Fix P12 parsing for certificates with "Basic constraints" (#122056) (#…

6e3fe5e

…122264) # Conflicts: # yarn.lock

jportner added v7.16.3 auto-backport Deprecated - use backport:version if exact versions are needed and removed auto-backport Deprecated - use backport:version if exact versions are needed labels Jan 5, 2022

jportner added a commit to jportner/kibana that referenced this pull request Jan 5, 2022

Fix P12 parsing for certificates with "Basic constraints" (elastic#12…

bba1853

…2056) # Conflicts: # yarn.lock

jportner mentioned this pull request Jan 5, 2022

[7.16] Fix P12 parsing for certificates with "Basic constraints" (#122056) #122310

Merged

jportner added a commit that referenced this pull request Jan 5, 2022

Fix P12 parsing for certificates with "Basic constraints" (#122056) (#…

17c0c79

…122310) # Conflicts: # yarn.lock

KOTungseth added the Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! label Jan 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix P12 parsing for certificates with "Basic constraints" #122056

Fix P12 parsing for certificates with "Basic constraints" #122056

jportner commented Dec 27, 2021

jportner left a comment

jportner Dec 28, 2021 •

edited

Loading

azasypkin Dec 28, 2021

jportner Jan 4, 2022

azasypkin commented Dec 28, 2021

azasypkin left a comment

azasypkin Dec 28, 2021

kibana-ci commented Jan 4, 2022

kibanamachine commented Jan 4, 2022

kibanamachine commented Jan 5, 2022

elasticmachine commented Jan 5, 2022

Fix P12 parsing for certificates with "Basic constraints" #122056

Fix P12 parsing for certificates with "Basic constraints" #122056

Conversation

jportner commented Dec 27, 2021

jportner left a comment

Choose a reason for hiding this comment

jportner Dec 28, 2021 • edited Loading

Choose a reason for hiding this comment

azasypkin Dec 28, 2021

Choose a reason for hiding this comment

jportner Jan 4, 2022

Choose a reason for hiding this comment

azasypkin commented Dec 28, 2021

azasypkin left a comment

Choose a reason for hiding this comment

azasypkin Dec 28, 2021

Choose a reason for hiding this comment

kibana-ci commented Jan 4, 2022

💚 Build Succeeded

Metrics [docs]

History

kibanamachine commented Jan 4, 2022

💔 Backport failed

kibanamachine commented Jan 5, 2022

💔 Backport failed

elasticmachine commented Jan 5, 2022

jportner Dec 28, 2021 •

edited

Loading