You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/01/26 05:41:14 UTC
[GitHub] [trafficserver] maskit opened a new pull request #7447: Use OpeSSL EVP API instead of SHA1_Init/Update/Final (cache_promote)
maskit opened a new pull request #7447:
URL: https://github.com/apache/trafficserver/pull/7447
https://www.openssl.org/docs/manmaster/man3/SHA1_Init.html
> All of the functions described on this page are deprecated. Applications should instead use EVP_DigestInit_ex(3), EVP_DigestUpdate(3) and EVP_DigestFinal_ex(3).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on a change in pull request #7447: Use OpenSSL EVP API instead of SHA1_Init/Update/Final (cache_promote)
Posted by GitBox <gi...@apache.org>.
bryancall commented on a change in pull request #7447:
URL: https://github.com/apache/trafficserver/pull/7447#discussion_r572294843
##########
File path: plugins/cache_promote/lru_policy.h
##########
@@ -53,11 +55,12 @@ class LRUHash
void
init(char *data, int len)
{
- SHA_CTX sha;
+ EVP_MD_CTX *ctx = EVP_MD_CTX_new();
Review comment:
Can't you just call EVP_Digest()? There is talk about the function here (https://github.com/openssl/openssl/issues/7219) and the issues around heap vs stack allocation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] randall commented on pull request #7447: Use OpenSSL EVP API if SHA1 API is not available (cache_promote)
Posted by GitBox <gi...@apache.org>.
randall commented on pull request #7447:
URL: https://github.com/apache/trafficserver/pull/7447#issuecomment-779923691
[approve ci autest]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit merged pull request #7447: Use OpenSSL EVP API if SHA1 API is not available (cache_promote)
Posted by GitBox <gi...@apache.org>.
maskit merged pull request #7447:
URL: https://github.com/apache/trafficserver/pull/7447
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #7447: Use OpenSSL EVP API instead of SHA1_Init/Update/Final (cache_promote)
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7447:
URL: https://github.com/apache/trafficserver/pull/7447#issuecomment-778990665
@bryancall Updated the code. New code uses `SHA1()` as long as it's available even if it's deprecated. To suppress warnings on compile with OpenSSL 3, I added a couple of `#pragma`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] github-actions[bot] commented on pull request #7447: Use OpenSSL EVP API if SHA1 API is not available (cache_promote)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7447:
URL: https://github.com/apache/trafficserver/pull/7447#issuecomment-864211322
This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7447: Use OpenSSL EVP API instead of SHA1_Init/Update/Final (cache_promote)
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7447:
URL: https://github.com/apache/trafficserver/pull/7447#discussion_r572540396
##########
File path: plugins/cache_promote/lru_policy.h
##########
@@ -53,11 +55,12 @@ class LRUHash
void
init(char *data, int len)
{
- SHA_CTX sha;
+ EVP_MD_CTX *ctx = EVP_MD_CTX_new();
Review comment:
EVP_Digest is basically just a wrapper and it uses heap too. If you worry about the heap allocation, we should keep the context object somewhere to reuse and that would require us to use these separated functions, I'm not sure if we can have a reusable context for this plugin though.
For now I just added a line to set EVP_MD_CTX_FLAG_ONESHOT to minimize the difference from EVP_Digest, but I'm also ok with replacing the lines with EVP_Digest if you strongly think that is better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #7447: Use OpeSSL EVP API instead of SHA1_Init/Update/Final (cache_promote)
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7447:
URL: https://github.com/apache/trafficserver/pull/7447#issuecomment-767351154
[approve ci Clang-Analyzer]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #7447: Use OpenSSL EVP API if SHA1 API is not available (cache_promote)
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #7447:
URL: https://github.com/apache/trafficserver/pull/7447#issuecomment-868680340
I will review this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #7447: Use OpenSSL EVP API instead of SHA1_Init/Update/Final (cache_promote)
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7447:
URL: https://github.com/apache/trafficserver/pull/7447#issuecomment-778035256
Seems like we should stick with the old API as long as we can for the best performance. I'm going to make a change to use the slow new API only if the fast old API is unavailable.
```
$ ./open
SHA1 comparison (1024 bytes * 100000)
OpenSSL SHA: 111.997 ms 60cacbf3d72e1e7834203da608037b1bf83b40e8
OpenSSL EVP: 190.121 ms 60cacbf3d72e1e7834203da608037b1bf83b40e8
Boost: 2106.630 ms 60cacbf3d72e1e7834203da608037b1bf83b40e8
APR: 1059.868 ms 60cacbf3d72e1e7834203da608037b1bf83b40e8
$ ./boring
SHA1 comparison (1024 bytes * 100000)
BoringSSL SHA: 111.878 ms 60cacbf3d72e1e7834203da608037b1bf83b40e8
BoringSSL EVP: 125.462 ms 60cacbf3d72e1e7834203da608037b1bf83b40e8
Boost: 2110.361 ms 60cacbf3d72e1e7834203da608037b1bf83b40e8
APR: 1075.716 ms 60cacbf3d72e1e7834203da608037b1bf83b40e8
```
```cpp
#include <iostream>
#include <iomanip>
#include <chrono>
#include <openssl/sha.h>
#include <openssl/evp.h>
#include <boost/uuid/detail/sha1.hpp>
#include "apr_sha1.h"
using namespace std;
void sha1_openssl_sha(const unsigned char *in, int in_len, unsigned char *out, unsigned int *out_len) {
SHA1(in, in_len, out);
*out_len = SHA_DIGEST_LENGTH;
}
void sha1_openssl_evp(const unsigned char *in, int in_len, unsigned char *out, unsigned int *out_len) {
EVP_Digest(in, in_len, out, out_len, EVP_sha1(), nullptr);
}
void sha1_boost(const unsigned char *in, int in_len, unsigned char *out, unsigned int *out_len) {
boost::uuids::detail::sha1 sha1;
unsigned int inthash[5];
sha1.process_bytes(in, in_len);
sha1.get_digest(inthash);
for(int i=0;i<5;i++){
out[i*4] = (inthash[i] & 0xFF000000) >> 24;
out[i*4+1] = (inthash[i] & 0x00FF0000) >> 16;
out[i*4+2] = (inthash[i] & 0x0000FF00) >> 8;
out[i*4+3] = (inthash[i] & 0x000000FF);
}
*out_len = 20;
}
void sha1_apr(const unsigned char *in, int in_len, unsigned char *out, unsigned int *out_len) {
apr_sha1_ctx_t ctx;
apr_sha1_init(&ctx);
apr_sha1_update_binary(&ctx, in, in_len);
apr_sha1_final(out, &ctx);
*out_len = APR_SHA1_DIGESTSIZE;
}
void measure(const char *name, void (*sha1func)(const unsigned char *, int, unsigned char *, unsigned int *))
{
unsigned char in[1024] = {0};
unsigned char out[20];
unsigned int out_len;
auto start = chrono::high_resolution_clock::now();
for (int i = 0; i < 100000; ++i) {
sha1func(in, sizeof(in), out, &out_len);
}
auto end = chrono::high_resolution_clock::now();
chrono::duration<double, milli> elapsed = end - start;
cout << setw(15) << name << ": " << setw(8) << fixed << setprecision(3) << elapsed.count() << " ms ";
auto prev = cout.fill('0');
for (int i = 0; i < out_len; ++i) {
cout << setw(2) << hex << static_cast<unsigned int>(out[i]);
}
cout.fill(prev);
cout << endl;
}
#ifdef OPENSSL_IS_BORINGSSL
#define LIB_NAME "BoringSSL"
#else
#define LIB_NAME "OpenSSL"
#endif
int main(int argc, char **argv) {
cout << "SHA1 comparison (1024 bytes * 100000)" << endl;
measure(LIB_NAME " SHA", sha1_openssl_sha);
measure(LIB_NAME " EVP", sha1_openssl_evp);
measure("Boost", sha1_boost);
measure("APR", sha1_apr);
return 0;
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org