eclipse-score / eclipse-score/inc_security_crypto

openssl-sys patch declares a full-featured OpenSSL: empty cargo:conf= mismatches the feature-reduced build

Open
#200 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
3
Forks
15
Avg merge
1d 22h
Merged PRs (30d)
23

Description

## Summary

The vendored `openssl-sys` build-script patch (`third_party/patches/openssl_sys_build_rs.patch`) declares a full-featured OpenSSL to the Rust crate graph: it hardcodes the version cfgs up to `ossl350` but emits an **empty** feature-exclusion list:

```rust
println!("cargo:conf=");
```

The S-CORE OpenSSL build (`//third_party/openssl`) is heavily feature-reduced (`no-ocsp`, `no-sm4`, `no-idea`, `no-cms`, `no-srtp`, ~25 `no-*` Configure flags). `cargo:conf=` is how `openssl-sys` tells the high-level `openssl` crate which `OPENSSL_NO_*` features to gate out (`#[cfg(osslconf = "OPENSSL_NO_…")]`). With the empty list, the `openssl` crate compiles bindings for OCSP, SM4, IDEA, CMS, … that reference symbols **the linked library does not export**.

## Why it doesn't blow up today

Dead-stripping: the unused binding objects are discarded at link time, so the undefined references vanish. The mismatch is latent, not harmless:

- Calling any of these APIs from Rust compiles fine and fails only at **link time** with `undefined symbol: OCSP_basic_verify`-style errors, far from the offending code. With a correct conf, the same mistake fails at **compile time** ("no method named …") because the API genuinely doesn't exist — the right failure mode for a security library.
- Anything that defeats dead-stripping re-detonates it across every binary that links openssl. Discovered exactly that way in #198: the coverage configuration's `-Clink-dead-code` (standard in the S-CORE LLVM coverage pipeline) broke all 23 `//score/cryptoki/tests` links with undefined OCSP/SM4/IDEA symbols. That PR works around it by omitting the flag; this issue is the root cause.

## Proposed fix

Change the patch to emit the real exclusion set:

```rust
println!("cargo:conf=OPENSSL_NO_ACVP_TESTS,OPENSSL_NO_AFALGENG,…,OPENSSL_NO_OCSP,…,OPENSSL_NO_SM4,…");
```

The authoritative source is the **generated** `include/openssl/configuration.h` in the `openssl_make` output (85 `OPENSSL_NO_*` defines for the current flags):

```
grep -oE "define OPENSSL_NO_[A-Z0-9_]+" bazel-bin/third_party/openssl/openssl_make/include/openssl/configuration.h \
| sed 's/define //' | sort | paste -sd,
```

The list must be regenerated whenever the `Configure` flags in `//third_party/openssl/BUILD` change (worth a comment next to them, or a small check).

This was validated in the course of #198: with the conf emitted, the OCSP/SM4/IDEA bindings are correctly gated out and those links succeed even with `-Clink-dead-code`.

## Known limitation

`openssl` 0.10.68's four SRTP bindings (`SSL_CTX_set_tlsext_use_srtp` etc.) carry **no** `osslconf` gate upstream, so the conf list cannot remove them — under `-Clink-dead-code` they still fail. Fixing that needs either an upstream gate or a second small patch on the `openssl` crate; until then, coverage keeps dead-stripping on (documented in `tools/coverage/coverage.bazelrc`).

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with third_party/patches/openssl_sys_build_rs.patch and compare its cargo:conf output with the generated third_party/openssl/openssl_make/include/openssl/configuration.h. Derive the exclusion list from the OPENSSL_NO_* definitions, then verify the openssl bindings are gated and the links in score/cryptoki/tests succeed with coverage's dead-code setting. Keep the SRTP limitation documented as described in the issue.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
build-system, cryptography, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.