Skip to content
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.

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

Invalid conversion from ‘const unsigned char*’ to ‘const char*’ credentials.c #10938

Closed
InputOutputZ opened this issue Nov 2, 2021 · 7 comments

Comments

@InputOutputZ
Copy link

InputOutputZ commented Nov 2, 2021

  • Program: Recursor, dnsdist
  • Issue type: Compiling dnsdist-1.7.0-alpha2 and rec-4.6.0-alpha1 in centOS 7 and (Red Hat 8.3.1-3) (GCC)

Short description

When compiling against openssl 1.1.1l and openssl 3.1.0-dev/master branch it throws errors and fails make process with following result:-

Environment

  • Operating system: centOS 7
  • Software version: gcc version 8.3.1 20190311
  • Software source: PDNS git source

Steps to reproduce

1- autoreconf -vi
2- ./configure --with-lua --enable-dnscrypt --enable-systemd --enable-dns-over-tls --with-systemd=/etc/systemd/system/ --with-nghttp2=auto --enable-dns-over-https
3- Update Makefile

CFLAGS = -gdwarf-4 -fPIC ' and remove -fPIE '
CXXFLAGS = -gdwarf-4 -fPIC ' and remove  -fPIE'

4- make

Expected behaviour

Successful make

Actual behaviour

credentials.cc: In function ‘std::string hashPasswordInternal(const string&, const string&, uint64_t, uint64_t, uint64_t)’:
credentials.cc:108:46: error: invalid conversion from ‘const unsigned char*’ to ‘const char*’ [-fpermissive]
   if (EVP_PKEY_CTX_set1_pbe_pass(pctx.get(), reinterpret_cast<const unsigned char*>(password.data()), password.size()) <= 0) {
                                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from credentials.cc:33:
/usr/local/openssl/include/openssl/kdf.h:119:63: note:   initializing argument 2 of ‘int EVP_PKEY_CTX_set1_pbe_pass(EVP_PKEY_CTX*, const char*, int)’
 int EVP_PKEY_CTX_set1_pbe_pass(EVP_PKEY_CTX *ctx, const char *pass,
                                                   ~~~~~~~~~~~~^~~~
credentials.cc:112:58: error: invalid conversion from ‘const char*’ to ‘const unsigned char*’ [-fpermissive]
   if (EVP_PKEY_CTX_set1_scrypt_salt(pctx.get(), salt.data(), salt.size()) <= 0) {
                                                 ~~~~~~~~~^~
In file included from credentials.cc:33:
/usr/local/openssl/include/openssl/kdf.h:123:56: note:   initializing argument 2 of ‘int EVP_PKEY_CTX_set1_scrypt_salt(EVP_PKEY_CTX*, const unsigned char*, int)’
                                   const unsigned char *salt, int saltlen);
                                   ~~~~~~~~~~~~~~~~~~~~~^~~~
credentials.cc:132:35: error: invalid conversion from ‘const unsigned char*’ to ‘unsigned char*’ [-fpermissive]
   if (EVP_PKEY_derive(pctx.get(), reinterpret_cast<const unsigned char*>(out.data()), &outlen) <= 0 || outlen != pwhash_output_size) {
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from credentials.cc:32:
/usr/local/openssl/include/openssl/evp.h:1922:55: note:   initializing argument 2 of ‘int EVP_PKEY_derive(EVP_PKEY_CTX*, unsigned char*, size_t*)’
 int EVP_PKEY_derive(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keylen);

Other information

Tested solution: Since gcc disallows conversion unsigned char* to const char* and vice versa, I therefore replaced methods EVP_PKEY_CTX_set1_pbe_pass and EVP_PKEY_derive params out.data() and password.data() cast to const char* and unsigned char*. Also in EVP_PKEY_CTX_set1_scrypt_salt call encapsulated salt.data() with cast like following reinterpret_cast<const unsigned char*>(salt.data()). Not sure if this is how it meant to be fixed. Any comments will be appreciated

@rgacogne
Copy link
Member

rgacogne commented Nov 3, 2021

Thanks for reporting this. I wonder why I don't see these warnings with a more recent compiler, and I'm bit surprised because EVP_PKEY_CTX_set1_pbe_pass, for example, is documented to take a const unsigned char* in both 1.1.1 and 3.0.0, but I'll investigate.

@rgacogne
Copy link
Member

rgacogne commented Nov 3, 2021

So:

  • EVP_PKEY_CTX_set1_pbe_pass used to take an unsigned char* but now takes a char* in 3.0.0, which shipped with an outdated man page.
  • EVP_PKEY_CTX_set1_scrypt_salt takes an unsigned char* in all versions and we pass a char*, we should fix this.
  • invalid conversion from ‘const unsigned char*’ to ‘unsigned char*’ being raised for reinterpret_cast<const unsigned char*>(out.data()) looks like a compiler that does not properly support C++17, where std::string::data() has a non-const version.

@rgacogne
Copy link
Member

rgacogne commented Nov 3, 2021

I'm not sure what to do about the first point, I wonder if it's a mistake in 3.0.0? #10943 will fix the second point, but the third one looks like an issue with the compiler you are using (we use the g++ version provided by devtoolset-8-gcc-c++ to build our packages on CentOS 7, if that helps).

@InputOutputZ
Copy link
Author

@rgacogne thanks for the clarification. I'm using just the same compiler, but I have version 12.0.0 (experimental) (GCC) installed beside it and used scl enable devtoolset-8 -- bash and exit to switch between both and tested with the latter, its same results yet further outputs more other errors when I use 12 and managed only to compile successfully with gcc 8. Once more, thanks for your clarification.

@rgacogne
Copy link
Member

rgacogne commented Nov 4, 2021

The type of the pass parameter to EVP_PKEY_CTX_set1_pbe_pass was changed in openssl/openssl@194de84 . Funnily enough that function still casts it to unsigned char* right away ¯_(ツ)_/¯

@omoerbeek
Copy link
Member

One way to work around the issue:

index 460df05ed..bd63f53e8 100644
--- a/pdns/credentials.cc
+++ b/pdns/credentials.cc
@@ -105,7 +105,8 @@ static std::string hashPasswordInternal(const std::string& password, const std::
     throw std::runtime_error("Error intializing the scrypt context to hash the supplied password");
   }
 
-  if (EVP_PKEY_CTX_set1_pbe_pass(pctx.get(), reinterpret_cast<const unsigned char*>(password.data()), password.size()) <= 0) {
+  // OpenSSL 3.0 changed the string arg to const unsigned char*, other versions use const char *, so cast to const void * to satisfy both
+  if (EVP_PKEY_CTX_set1_pbe_pass(pctx.get(), reinterpret_cast<const void*>(password.data()), password.size()) <= 0) {
     throw std::runtime_error("Error adding the password to the scrypt context to hash the supplied password");
   }

If this is deemed OK, I'll add it to #10943

@rgacogne
Copy link
Member

rgacogne commented Nov 5, 2021

That works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants