SIPp / sipp

The SIPp testing tool
https://sipp.readthedocs.io
Other
891 stars 376 forks source link

Support SHA-256 algorithm (RFC 8760) #676

Closed Maratk1n closed 5 months ago

orgads commented 7 months ago

Thank you!

  1. Please add -DUSE_SHA256=1 also to build.sh in --full.
  2. There are many deprecation warnings with openssl 3. Example:
src/auth.cpp:348:18: warning: ‘int SHA256_Update(SHA256_CTX*, const void*, size_t)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  348 |     SHA256_Update( &SHA256Ctx, (unsigned char *) user, strlen(user));
      |     ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Maratk1n commented 7 months ago

Hi, @orgads

  1. Done
  2. I replaced openssl/sha with EVP module

Thanks for review!

orgads commented 7 months ago

Thank you!

I didn't test functionality yet, but compilation and tests pass.

orgads commented 6 months ago

Verified it works, thank you!

Please remove the flag.

orgads commented 6 months ago

LGTM

lemenkov commented 6 months ago

I've found only a small mostly cosmetic issue. Please address it and rebase on top of current master branch.

Maratk1n commented 6 months ago

Hi @lemenkov . I've rebased branch and added unit test.

orgads commented 6 months ago

Thank you! Maybe squash all the commits at this point? I would leave only the first one as a separate commit.

Maratk1n commented 6 months ago

No problem. I've squashed commits and rebased branch.

orgads commented 6 months ago

Thanks! Please resolve the open discussion.

Maratk1n commented 6 months ago

Did you mean comment from github-advanced-security bot? I fixed failure but don't have permission to resolve the thread.

orgads commented 6 months ago

I meant our discussion about CMakeLists. You already resolved it :)

lemenkov commented 5 months ago

Did you mean comment from github-advanced-security bot? I fixed failure but don't have permission to resolve the thread.

@Maratk1n your PR raises a minimum OpenSSL version up to 1.1.0 but still there is a mention of 0.9.8 version in the doc-files. See my notes above.

Maratk1n commented 5 months ago

Thank you @lemenkov . Fixed.

lemenkov commented 5 months ago

@orgads looks good to me. What's your opinion?

orgads commented 5 months ago

lgtm