Skip to content

x509storeissuer update #50

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

Closed
esyr wants to merge 25 commits into from
Closed

x509storeissuer update #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6398216
x509storeissuer: factor out usage printing
esyr Oct 20, 2025
15c799b
s/err/error where apropriate easiest way to fix liner issues on windows
Sashan Sep 29, 2025
7448789
Use perflib/err.h unconditionally
esyr Sep 25, 2025
ee5fd86
perflib: add vwarn/err/warn
esyr Sep 25, 2025
6b108da
perflib: add ossl_noreturn qualifier and use it for err/errx
esyr Nov 10, 2025
2b7fcf4
x509storeissuer: use errx/warnx for error message output
esyr Oct 20, 2025
68a7b91
x509storeissuer: factor out integer parameter parsing into a separate…
esyr Oct 20, 2025
9c03311
x509storeissuer: add ability to configure verbosity level
esyr Oct 20, 2025
be5de07
x509storeissuer: provide the initial infrastructure for nonce configu…
esyr Oct 20, 2025
2cd00eb
x509storeissuer: unify funtion definition formatting
esyr Oct 20, 2025
18fab94
x509storeissuer: add -T option to specify the run time
esyr Oct 20, 2025
b9e8a78
x509storeissuer: support providing multiple certificate directories
esyr Oct 20, 2025
09f24e3
x509storeissuer: count the iterations in a local variable to avoid ha…
esyr Oct 21, 2025
92ca087
x509storeissuer: tolerate X509_STORE_CTX_get1_issuer() successes, cou…
esyr Oct 21, 2025
8a68e53
x509storeissuer: add certificates from the provided directories to th…
esyr Oct 21, 2025
319e6ec
x509storeissuer: add and option to configure X509_STORE_CTX sharing
esyr Oct 21, 2025
aec1748
x509storeissuer: report the store size before the test run if the ver…
esyr Oct 22, 2025
ca94c96
x509storeissuer: make the verbose reporting more elaborate
esyr Oct 22, 2025
a600b7e
x509storeissuer: make thread_data cache-line-aligned
esyr Oct 23, 2025
5acec49
x509storeissuer: add limits for certificate file/directory load counts
esyr Nov 4, 2025
753613f
x509storeissuer: add an ability to skip loading default paths
esyr Nov 4, 2025
5444f48
x509storeissuer: store excess certificates in a pool for later use
esyr Nov 5, 2025
8ba8e6b
x509storeissuer: add ability to add certificates to the store during …
esyr Oct 22, 2025
176619a
CMakeLists.txt: add recently added x509storeissuer options to the tes…
esyr Nov 6, 2025
14fa76e
CMakeLists.txt, test.yml: make "run" target optional, continute to us…
esyr Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ jobs:
- name: "Config perftools build"
working-directory: "./perftools/source"
run: |
cmake -S . -B build -DOPENSSL_ROOT_DIR="$GITHUB_WORKSPACE/openssl" -Drun_add_version_dep:BOOL=OFF ${{ matrix.release.cmakeopts }}
cmake -S . -B build -DOPENSSL_ROOT_DIR="$GITHUB_WORKSPACE/openssl" -Denable_run_target:BOOL=ON -Drun_add_version_dep:BOOL=OFF ${{ matrix.release.cmakeopts }}
- name: "Build perftools"
working-directory: "./perftools/source"
run: |
Expand Down Expand Up @@ -157,7 +157,7 @@ jobs:
- name: "Config perftools build"
working-directory: "./perftools/source"
run: |
cmake -S . -B build -DOPENSSL_ROOT_DIR="$GITHUB_WORKSPACE/openssl" -Drun_add_version_dep:BOOL=OFF ${{ matrix.release.cmakeopts }}
cmake -S . -B build -DOPENSSL_ROOT_DIR="$GITHUB_WORKSPACE/openssl" -Denable_run_target:BOOL=ON -Drun_add_version_dep:BOOL=OFF ${{ matrix.release.cmakeopts }}
- name: "Build perftools"
working-directory: "./perftools/source"
run: |
Expand Down Expand Up @@ -262,7 +262,7 @@ jobs:
shutdown_vm: false
run: |
cd perftools/source
cmake -S . -B build -DOPENSSL_ROOT_DIR="$GITHUB_WORKSPACE/openssl" -Drun_add_version_dep:BOOL=OFF ${{ matrix.release.cmakeopts }}
cmake -S . -B build -DOPENSSL_ROOT_DIR="$GITHUB_WORKSPACE/openssl" -Denable_run_target:BOOL=ON -Drun_add_version_dep:BOOL=OFF ${{ matrix.release.cmakeopts }}
- name: "Build perftools"
uses: "cross-platform-actions/action@fe0167d8082ac584754ef3ffb567fded22642c7d" #v0.27.0
with:
Expand Down Expand Up @@ -358,7 +358,7 @@ jobs:
- name: "Config perftools build"
working-directory: ".\\perftools\\source"
run: |
cmake -S . -B .\build -DOPENSSL_ROOT_DIR="$env:GITHUB_WORKSPACE\openssl" -Drun_add_version_dep:BOOL=OFF ${{ matrix.release.cmakeopts }}
cmake -S . -B .\build -DOPENSSL_ROOT_DIR="$env:GITHUB_WORKSPACE\openssl" -Denable_run_target:BOOL=ON -Drun_add_version_dep:BOOL=OFF ${{ matrix.release.cmakeopts }}
- name: "Build perftools"
working-directory: ".\\perftools\\source"
run: |
Expand Down Expand Up @@ -435,7 +435,7 @@ jobs:
- name: "Config perftools build"
working-directory: "./perftools/source"
run: |
cmake -S . -B build -DOPENSSL_ROOT_DIR="$GITHUB_WORKSPACE/openssl" -Drun_add_version_dep:BOOL=OFF ${{ matrix.release.cmakeopts }}
cmake -S . -B build -DOPENSSL_ROOT_DIR="$GITHUB_WORKSPACE/openssl" -Denable_run_target:BOOL=ON -Drun_add_version_dep:BOOL=OFF ${{ matrix.release.cmakeopts }}
- name: "Build perftools"
working-directory: "./perftools/source"
run: |
Expand Down
143 changes: 94 additions & 49 deletions source/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,12 @@ if(WIN32)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE "${OUTPUT_DIRECTORY}")
endif()

if(WIN32)
set(libm_dep)
else()
set(libm_dep PUBLIC m)
endif()

target_include_directories(perf PUBLIC "${PROJECT_SOURCE_DIR}")
target_link_libraries(perf PUBLIC OpenSSL::SSL OpenSSL::Crypto)

Expand Down Expand Up @@ -188,17 +194,13 @@ add_executable(rsasign rsasign.c)
target_link_libraries(rsasign PRIVATE perf)

add_executable(x509storeissuer x509storeissuer.c)
target_link_libraries(x509storeissuer PRIVATE perf)
target_link_libraries(x509storeissuer ${libm_dep} PRIVATE perf)

add_executable(rwlocks rwlocks.c)
target_link_libraries(rwlocks PRIVATE perf)

add_executable(pkeyread pkeyread.c)
if(WIN32)
target_link_libraries(pkeyread PRIVATE perf)
else()
target_link_libraries(pkeyread PUBLIC m PRIVATE perf)
endif()
target_link_libraries(pkeyread ${libm_dep} PRIVATE perf)

add_executable(evp_setpeer evp_setpeer.c)
target_link_libraries(evp_setpeer PRIVATE perf)
Expand All @@ -211,6 +213,8 @@ target_link_libraries(evp_hash PRIVATE perf)

## Running tests
# Options
option(enable_run_target "Enable generation of run* targets")

set(run_tests evp_fetch
evp_hash
evp_setpeer
Expand Down Expand Up @@ -277,6 +281,36 @@ set(run_writeread_dtls
set(run_writeread_buffers
writeread "" "" "-b 256" "-b 4096"
CACHE STRING "Buffer size for writeread")
# Note that this essentially forces all x509storeissuer runs to take
# 0.12 seconds by default, as otherwise it takes too long due to the size
# of the test matrix.

@nhorman nhorman Nov 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm a bit concerned about this reduction in test run time:

nhorman@hmsbeagle:~/git/perftools/source/build$ ./x509storeissuer -T 0.12 -w 3 -W 0.1 -t /home/nhorman/git/worktrees/work1/test/certs 4
1.227123
nhorman@hmsbeagle:~/git/perftools/source/build$ ./x509storeissuer -T 5 -w 3 -W 0.1 -t /home/nhorman/git/worktrees/work1/test/certs 4
1.095945

thats a 12% increase in average call time just by reducing the test run period from 5 seconds to 0.12 seconds. As to weather thats a more accurate representation of our actual run time, I honestly can't say, but if I had to guess I'd hypothesize that running for such a short period of time makes the library initialization time more prominent in our average figures.

If we have to limit the run time like this, I would suggest running a "warm up round first" to ensure that libcrypto has all its data strctures setup prior to us recording any run times.

Though, if I'm reading the below properly this fanout is indeed huge. I think its the number of combinations for each set element below yes? Meaning we're running 2x3x4x3x3x3x3x4x3=23328 combinations of this test! Theres got to be a way to reduce that. For instance if we unilaterally added the -q 20 and -v option to every test, we could reduce this matrix to 1944 combinations, which should run in about 2.7 hours, which still isn't great, but its on par with how some of our other tests run.

Theres also the question as to which of these test runs do we keep graphing data for on grafana? It might be worth restricting what we run here to just those, as I'm hard pressed to think of who is going to be reviewing all 23,000 of these current outputs.

@npajkovsky npajkovsky Nov 7, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We should test only options that are used for the Grafana dashboard.

@esyr esyr Nov 7, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'd rather have all the code we are not going to test be removed, then.

@npajkovsky npajkovsky Nov 10, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We don't really have to be that strict in this case.

set(run_x509storeissuer_timeout
x509storeissuer "" "-T 0.12"
CACHE STRING "Test timeout for x509storeissuer")
set(run_x509storeissuer_verbosity
x509storeissuer "" "" "-v"
CACHE STRING "Output verbosity level for x509storeissuer")
set(run_x509storeissuer_quantiles
x509storeissuer "" "" "-q 1" "-q 20"
CACHE STRING "Number of quantiles for x509storeissuer")
set(run_x509storeissuer_nonces
x509storeissuer "" "" "-n file:server-pss-cert.pem"
CACHE STRING "Nonce configurations for x509storeissuer")
set(run_x509storeissuer_modes
x509storeissuer "" "" "-m rw"
CACHE STRING "Run modes for x509storeissuer")
set(run_x509storeissuer_max_writers
x509storeissuer "" "" "-w 3"
CACHE STRING "Maximum writers count for x509storeissuer")
set(run_x509storeissuer_write_prob
x509storeissuer "" "" "-W 0.1"
CACHE STRING "Write probability for x509storeissuer")
set(run_x509storeissuer_cert_load_opts
x509storeissuer "" "" "-L 3" "-L 1 -l 23 -E"
CACHE STRING "Certificate count limit for x509storeissuer")
set(run_x509storeissuer_ctx_share
x509storeissuer "" "" "-C 2"
CACHE STRING "X509_STORE_CTX share options for x509storeissuer")

# The list of per-tet options
set(run_opts run_evp_fetch_pqs
Expand All @@ -294,7 +328,16 @@ set(run_opts run_evp_fetch_pqs
run_writeread_ctx_sharing
run_writeread_dtls
run_writeread_buffers
CACHE STRING "List of per-text options")
run_x509storeissuer_timeout
run_x509storeissuer_verbosity
run_x509storeissuer_quantiles
run_x509storeissuer_nonces
run_x509storeissuer_modes
run_x509storeissuer_max_writers
run_x509storeissuer_write_prob
run_x509storeissuer_cert_load_opts
run_x509storeissuer_ctx_share
CACHE STRING "List of per-test options")

# Used across multiple tests
set(run_certdir_tests handshake writeread x509storeissuer
Expand Down Expand Up @@ -405,61 +448,63 @@ function(gen_run_taget _CMD _TEST)
endfunction()

# run targets generation
foreach(test IN LISTS run_tests)
set(cmds "${test}")
if (enable_run_target)
foreach(test IN LISTS run_tests)
set(cmds "${test}")

# test-specific options
foreach(opt_name IN LISTS run_opts)
set(opt "${${opt_name}}")
list(GET opt 0 test_name)
list(GET opt 1 test_opt)
list(REMOVE_AT opt 0 1)

if(test IN_LIST test_name)
set(new_cmds)
foreach(cmd IN LISTS cmds)
foreach(val IN LISTS opt)
list(APPEND new_cmds "${cmd} ${test_opt} ${val}")
endforeach()
endforeach()
set(cmds ${new_cmds})
endif()
endforeach()

# test-specific options
foreach(opt_name IN LISTS run_opts)
set(opt "${${opt_name}}")
list(GET opt 0 test_name)
list(GET opt 1 test_opt)
list(REMOVE_AT opt 0 1)
# terse
set(new_cmds)
foreach(cmd IN LISTS cmds)
foreach(val IN LISTS run_terse)
list(APPEND new_cmds "${cmd} ${val}")
endforeach()
endforeach()
set(cmds ${new_cmds})

if(test IN_LIST test_name)
# certdir
if(test IN_LIST run_certdir_tests)
set(new_cmds)
foreach(cmd IN LISTS cmds)
foreach(val IN LISTS opt)
list(APPEND new_cmds "${cmd} ${test_opt} ${val}")
endforeach()
list(APPEND new_cmds "${cmd} ${run_certdir}")
endforeach()
set(cmds ${new_cmds})
endif()
endforeach()

# terse
set(new_cmds)
foreach(cmd IN LISTS cmds)
foreach(val IN LISTS run_terse)
list(APPEND new_cmds "${cmd} ${val}")
endforeach()
endforeach()
set(cmds ${new_cmds})

# certdir
if(test IN_LIST run_certdir_tests)
# threads
set(new_cmds)
foreach(cmd IN LISTS cmds)
list(APPEND new_cmds "${cmd} ${run_certdir}")
foreach(val IN LISTS run_threads)
list(APPEND new_cmds "${cmd} ${val}")
endforeach()
endforeach()
set(cmds ${new_cmds})
endif()

# threads
set(new_cmds)
foreach(cmd IN LISTS cmds)
foreach(val IN LISTS run_threads)
list(APPEND new_cmds "${cmd} ${val}")
# Run targets
foreach(cmd IN LISTS cmds)
gen_run_taget("${cmd}" "${test}" VAR run_target_name)
add_dependencies(run "${run_target_name}")
endforeach()
endforeach()
set(cmds ${new_cmds})

# Run targets
foreach(cmd IN LISTS cmds)
gen_run_taget("${cmd}" "${test}" VAR run_target_name)
add_dependencies(run "${run_target_name}")
# Per-test version target
gen_run_taget("${test} -V" "${test}" NAME "run-version-${test}")
add_dependencies(run-version "run-version-${test}")
endforeach()

# Per-test version target
gen_run_taget("${test} -V" "${test}" NAME "run-version-${test}")
add_dependencies(run-version "run-version-${test}")
endforeach()
endif()
22 changes: 11 additions & 11 deletions source/evp_fetch.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
size_t *counts;
OSSL_TIME max_time;

int err = 0;
int error = 0;

@nhorman nhorman Nov 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm still at a loss for why this had to be done everywhere. I just reverted the changes to evp_fetch.c regarding this conversion, and with Visual Studio 17 2022, the perftools repo built fine. What compiler version produced errors for you?

@Sashan Sashan Nov 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If I rememer correctly the change was prompted by thread affinity which is gone. so perhaps we can drop this changeset from this PR too.

@esyr esyr Nov 7, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I guess any refactoring that introduces usage of this BSD-specific function would lead to that, so I've decided to leave it as is; note, however, that the symbol name "error" also conflicts with the GNU-specific function, so we might want to rethink that rename in favor of something that doesn't hit either of those.

@bob-beck bob-beck Nov 13, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As mentioned, this is pretty intrusive and should probably be a separate change from this.

like with a real issue and stuff about the name of this global symbol in this file and what it conflicts with. To me it seems like changing err to error is just moving the deck chair over one place and waiting for the next conflict. While making a decision to solve that is fine, we should solve it rather than just hacking this just for the purpose of having err() in this test.

int pq = 0;

static int threadcount;
Expand Down Expand Up @@ -197,7 +197,7 @@ void do_fetch(size_t num)
fetch_alg = exclusive_fetch_alg;
}

if (err == 1)
if (error == 1)
return;

switch (j) {
Expand All @@ -206,7 +206,7 @@ void do_fetch(size_t num)
fetch_entries[j].propq);
if (md == NULL) {
fprintf(stderr, "Failed to fetch %s\n", fetch_alg);
err = 1;
error = 1;
return;
}
EVP_MD_free(md);
Expand All @@ -217,7 +217,7 @@ void do_fetch(size_t num)
fetch_entries[j].propq);
if (cph == NULL) {
fprintf(stderr, "Failed to fetch %s\n", fetch_alg);
err = 1;
error = 1;
return;
}
EVP_CIPHER_free(cph);
Expand All @@ -228,7 +228,7 @@ void do_fetch(size_t num)
fetch_entries[j].propq);
if (kdf == NULL) {
fprintf(stderr, "Failed to fetch %s\n", fetch_alg);
err = 1;
error = 1;
return;
}
EVP_KDF_free(kdf);
Expand All @@ -239,7 +239,7 @@ void do_fetch(size_t num)
fetch_entries[j].propq);
if (mac == NULL) {
fprintf(stderr, "Failed to fetch %s\n", fetch_alg);
err = 1;
error = 1;
return;
}
EVP_MAC_free(mac);
Expand All @@ -250,7 +250,7 @@ void do_fetch(size_t num)
fetch_entries[j].propq);
if (rnd == NULL) {
fprintf(stderr, "Failed to fetch %s\n", fetch_alg);
err = 1;
error = 1;
return;
}
EVP_RAND_free(rnd);
Expand All @@ -261,7 +261,7 @@ void do_fetch(size_t num)
fetch_entries[j].propq);
if (kem == NULL) {
fprintf(stderr, "Failed to fetch %s\n", fetch_alg);
err = 1;
error = 1;
return;
}
EVP_KEM_free(kem);
Expand All @@ -272,14 +272,14 @@ void do_fetch(size_t num)
fetch_entries[j].propq);
if (sig == NULL) {
fprintf(stderr, "Failed to fetch %s\n", fetch_alg);
err = 1;
error = 1;
return;
}
EVP_SIGNATURE_free(sig);
break;
}
default:
err = 1;
error = 1;
return;
}
counts[num]++;
Expand Down Expand Up @@ -392,7 +392,7 @@ int main(int argc, char *argv[])
goto out;
}

if (err) {
if (error) {
printf("Error during test\n");
goto out;
}
Expand Down
12 changes: 6 additions & 6 deletions source/evp_setpeer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

#define RUN_TIME 5

int err = 0;
int error = 0;

size_t num_calls;
static int threadcount;
Expand All @@ -43,13 +43,13 @@ void do_setpeer(size_t num)

pkey_ctx = EVP_PKEY_CTX_new(pkey, NULL);
if (pkey_ctx == NULL) {
err = 1;
error = 1;
printf("Failed to create pkey_ctx\n");
return;
}

if (EVP_PKEY_derive_init(pkey_ctx) <= 0) {
err = 1;
error = 1;
printf("Failed to init pkey_ctx\n");
EVP_PKEY_CTX_free(pkey_ctx);
return;
Expand All @@ -59,7 +59,7 @@ void do_setpeer(size_t num)

do {
if (EVP_PKEY_derive_set_peer(pkey_ctx, pkey) <= 0) {
err = 1;
error = 1;
break;
}
counts[num]++;
Expand Down Expand Up @@ -98,7 +98,7 @@ static double get_avcalltime(void)

static void report_result(int key_id, int terse)
{
if (err) {
if (error) {
fprintf(stderr, "Error during test of %s\n",
sample_names[key_id]);
exit(EXIT_FAILURE);
Expand Down Expand Up @@ -232,7 +232,7 @@ int main(int argc, char *argv[])
EVP_PKEY_free(pkey);
}

if (err) {
if (error) {
printf("Error during test\n");
goto out;
}
Expand Down
Loading