security/openssl: Enable ASM by default - Enable ASM option - By extension this enables AES-NI [1] - Order OPTIONS_DEFAULT alphabetically - Switch to using @sample [2] PR: 216559 [2] Reported by: dtestke [1] Submitted by: Franco Fichtner <franco@opnsense.org> [2]
Details
Details
- Reviewers
dteske brnrd allanjude - Commits
- rP433671: security/openssl: Enable ASM by default
Diff Detail
Diff Detail
- Repository
- rP FreeBSD ports repository
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 7261 Build 7429: arc lint + arc unit
Event Timeline
Comment Actions
Are the new *.so files 100% compatible with the old ones if one enables ASM ? Or does this need a shlib version bump ?
| security/openssl/pkg-plist | ||
|---|---|---|
| 1707 | This feels like a bad idea. | |
Comment Actions
Apart from the PORTREVISION issue, it LGTM. Please bump PORTREVISION.
| security/openssl/pkg-plist | ||
|---|---|---|
| 1707 | I tested it, and it's fine. If openssl.cnf is already there, then updating the port does _not_ replace it. | |
Comment Actions
The ASM defines mask the C-routines and enable the optimized assembly replacements.
There's a bit more to it I've learned. Various commits referring to ASM:
- rP422700 add option ASM for OPNsense
- rP421025 remove options ASM and GMP (PR210859)
- (Upgrade to 1.1.0 and then downgrade to 1.0.2)
- rP408778 mark options ASM broken on sparc64 (PR204527)
- rP383877 disable option ASM by default (PR198788??)
Unfortunately the commit messages aren't very informative... It seems to me that people hitting bugs had not rebuilt their ports correctly or were mixing base and ports SSL libs (via sasl, krb, curl, ...)
Given that:
- Base OpenSSL has ASM enabled by default
- There are no reports on broken ports with base OpenSSL pertaining to ASM
- There are reports that ASM does work (e.g. PR19788 #49)
- ASM is enabled by default upstream
- Insecure options are enabled by default in port but not upstream
I propose the following:
- Enable ASM by default as soon as possible
And as a separate action/commit
- Announce upcoming change on ML
- Bump .so version
- Disable MD2 and SSLv2
- Add entry to UPDATING
| Option | Default | Port | Comment |
| asm | on | forced off | Performance impacted |
| ec_nistp | off | forced on | configure can't detect |
| gmp | off | forced off | |
| jpake | off | default off | |
| krb5 | off | default off | |
| libunbound | off | default off | |
| md2 | off | forced on | Insecure |
| rc5 | off | forced off | |
| rfc3779 | off | forced off | |
| sctp | off | forced on | |
| shared | off | forced on | |
| ssl-trace | off | default off | |
| ssl2 | off | forced on | Insecure |
| ssl3 | forced on | presumed on by default | |
| static-engine | on | off | automatic by "shared" option |
| store | off | forced off | |
| threads | forced on | ||
| unit-test | off | default off | |
| weak-ciphers | off | forced off | |
| zlib | off | forced off | |
| zlib-dynamic | off | forced off |
| security/openssl/pkg-plist | ||
|---|---|---|
| 1707 | Well, of course if there is an existing one it will not be replaced, it is what @sample does. My question was "does openssl behaves the same way if the provided openssl.cnf is here or not ?" | |
Comment Actions
Works as expected for me. We have been using the ASM option in production since the start of 2016.
| security/openssl/pkg-plist | ||
|---|---|---|
| 1707 | Not in all circumstances, it sets defaults for files, directories, and DN. The openssl command works without an openssl.cnf but will emit a warning it is missing. I believe the default @sample behaviour is what's required here. | |
Comment Actions
Bumping the .so revision puts a big onus on some vendors to recompile their software for almost zero reason.
The ASM enabled .so is absolutely compatible with the ASM disabled .so -- there should be no need to bump the .so revision.
Most notably, people like nginx whom provide a paid-for product called "nginx-plus" available as binary-package only from their own public pkg server will have to recompile their binaries and issue a new package to the world if you bump the .so revision.
Comment Actions
Tried closing this revision but because asomers blocked it, phabricator tells me it can't be closed.
Can someone else try to close this review?
Comment Actions
As long as features are DISABLED and not removed (e.g. at $JOB we need MD2) then ok.
I think we should make two versions of this port: openssl and openssl-insecure, which has features like sslv2 and MD2 enabled for people who need that functionality. (e.g. for talking to old machines or handling old certificates etc.)
maybe with a different .so number. (just an idea).
Comment Actions
For reference, here's where this all stemmed from https://fraubsd.org/doc/nginx_plus_with_ports_openssl_and_aesni.png