Skip to content

Full pkcs11 integration #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
matyasselmeci merged 6 commits into from
Jan 29, 2026
Merged

Full pkcs11 integration #42

matyasselmeci merged 6 commits into from
Jan 29, 2026

Conversation

@h2zh
Copy link

@h2zh h2zh commented Dec 10, 2025 •
edited
Loading

This is the XRootD counterpart for PelicanPlatform/pelican#2706

See commit message for explanation.

bbockelm and others added 2 commits December 10, 2025 15:02
@bbockelm @h2zh
When using a pkcs11:-style URL, load via the engine API
549d763 
With this, the following works:

```
xrd.tls /home/foo/.config/certificates/tls.crt \
       pkcs11:token=test-token;object=priv_key;type=public?pin-value=1234
```

(cherry picked from commit 682f449)
@h2zh
Full pkcs11 integration
30900fb 
- EnsureOpenSSLConfigLoaded() is a helper function ensuring the OpenSSL global configuration file (OPENSSL_CONF) is loaded exactly once per process

- Export PKCS11_MODULE_PATH (e.g. /usr/lib64/pkcs11/p11-kit-client.so) from the Pelican launcher and feed that directly to pkcs11 engine, guaranteeing it loads the RPC client module

- Abort early if we can’t ingest the config, because without it the pkcs11 engine will never know which module to use

- Use `ENGINE_free(e)` to clean up ENGINE objects
This was referenced Dec 12, 2025
Merged
Closed
@h2zh
Copy link
Author

h2zh commented Dec 19, 2025

Could @bbockelm or @matyasselmeci review this PR?

@patrickbrophy briefly reviewed this PR and successfully completed the end-to-end testing along with the counterpart PR in Pelican repo.

I think these failed CI workflows here are irrelevant to this PR. I speculate it is related to the config of this repo. Maybe @matyasselmeci could have some insights?

  113: Keytab file is not secure!
  113: Secsss (getKeyTab): Unable to process sss.keytab; file is not secure!
  113: 
  113: Keytab file is not secure!
  113: [FATAL] Auth failed: No protocols left to try

Btw, this PR's predecessor is in the upstream XRootD repo. And the failed CIs here drawed their attention.

@h2zh h2zh requested a review from matyasselmeci December 22, 2025 16:15
h2zh added 2 commits December 31, 2025 00:01
@h2zh
Fix compilation errors when building against OpenSSL configured with
c87c13b 
no-engine or no-deprecated options.

Changes:
- Include opensslconf.h first to get feature detection macros before
  attempting to include engine.h (which may not exist)
- Check OPENSSL_NO_ENGINE, OPENSSL_NO_DEPRECATED_3_0, and
  OPENSSL_NO_DEPRECATED to determine ENGINE API availability
- Define XRDTLS_HAVE_ENGINE macro to guard all ENGINE-related code
- Provide clear runtime error when PKCS11 is requested but ENGINE
  API is unavailable

This fixes CI build failures where OpenSSL 3.0 was compiled with
deprecated APIs removed, causing:
- "openssl/engine.h: No such file or directory" when header missing
- "ENGINE_by_id was not declared in this scope" when APIs unavailable

PKCS11 support now requires OpenSSL built with ENGINE support enabled.
@h2zh
Use Provider API instead of ENGINE API for OpenSSL 3.0+
a377eb7
@h2zh h2zh mentioned this pull request Jan 6, 2026
Merged
@h2zh
Copy link
Author

h2zh commented Jan 6, 2026

Commit a377eb7 aims to fix the deprecated & disabled ENGINE issue on Alma 10. Here's its general idea:

Provider API for OpenSSL 3+ (Alma 10); ENGINE API for OpenSSL 1.x (EL8). User installs different packages (pkcs11-provider, openssl-pkcs11) based on their OpenSSL version, to trigger the corresponding API.

matyasselmeci
matyasselmeci reviewed Jan 7, 2026
View reviewed changes
h2zh added 2 commits January 8, 2026 03:10
@h2zh
Amend the comment for private key file path check
286c1e5 
- Note that if it's a PKCS#11 URI, it will be validated later.
@h2zh
XrdTls: Isolate PKCS#11 operations in dedicated OSSL_LIB_CTX
c88349d 
Load PKCS#11 provider in an isolated OpenSSL context to prevent
interference with other TLS operations (e.g., SciTokens library).

- Add InitIsolatedPKCS11Context() to create isolated OSSL_LIB_CTX
- Load OPENSSL_CONF into isolated context for proper configuration
- Use OSSL_STORE_open_ex() with isolated context for PKCS#11 URIs
- Add cleanup for isolated context in destructor

This fixes "bad record MAC" errors that occurred when the global
PKCS#11 provider affected unrelated HTTPS operations (when SciTokens tried to fetch JWKS over HTTPS).
@h2zh
Copy link
Author

h2zh commented Jan 13, 2026 •
edited
Loading

The problem commit c88349d fixes:

When Pelican enabled PKCS#11 support, it set the OPENSSL_CONF environment variable globally, which loaded the PKCS#11 provider for the entire process. However, PKCS#11 is designed for key storage and signing, not for general-purpose cryptography:

  • Can: Load private key from token; Sign with private key
  • Can't: Verify signatures (TLS client); TLS client operations; Certificate chain verification

This caused the SciTokens C++ library (used within XRootD) to inherit the PKCS#11 provider configuration. When the SciTokens library attempted to fetch JWKS over HTTPS from self-signed certificates, certificate verification failed, resulting in "bad record MAC" TLS handshake errors.

How does this commit fix the problem:

OpenSSL 3's provider architecture allows multiple providers to coexist. Different components can use different providers.

  1. Create an isolated OSSL_LIB_CTX (official docs) specifically for PKCS#11 operations
  2. Load the OPENSSL_CONF configuration into this isolated context
  3. Use the isolated context exclusively for loading private keys from PKCS#11
  4. Leave the default context clean for all other TLS operations (including SciTokens)
┌─────────────────────────────────────────────────────┐
│ XRootD Process (spawned by Pelican)                 │
│                                                     │
│ ┌─────────────────────┐  ┌─────────────────────┐  │
│ │ XRootD TLS Server   │  │ SciTokens Library    │  │
│ │                     │  │ (loaded as plugin)   │  │
│ │ Uses isolated ctx   │  │ Uses default ctx     │  │
│ │ with PKCS11         │  │ (clean, no PKCS11)   │  │
│ └─────────────────────┘  └─────────────────────┘   │
│    Isolated OSSL_LIB_CTX    Default OSSL_LIB_CTX    │
└─────────────────────────────────────────────────────┘

patrickbrophy
patrickbrophy reviewed Jan 15, 2026
View reviewed changes
Copy link

@patrickbrophy patrickbrophy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me and I have tested this e2e with the Pelican integration and building XrootD with this patch.

@h2zh
Copy link
Author

h2zh commented Jan 15, 2026

Thanks @patrickbrophy.

Could Mat @matyasselmeci review the rest of this PR? Then merge it to a new XRootD version (5.9.2)? After the merge and new RPM build, Pelican needs to update the XRootD version number in Dockerfile and other installation script to actually able to use the PKCS#11 feature.

@matyasselmeci matyasselmeci merged commit 8f0075b into PelicanPlatform:v5.9.1-pelican Jan 29, 2026
1 of 12 checks passed
@matyasselmeci matyasselmeci mentioned this pull request Jan 29, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@patrickbrophy patrickbrophy patrickbrophy left review comments

@matyasselmeci matyasselmeci matyasselmeci approved these changes

Assignees

@matyasselmeci matyasselmeci

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@h2zh @matyasselmeci @patrickbrophy @bbockelm