Wt

Hi,

The provided method for setting the SSL password callback has 2 issues:

- when called, the application crashes (I think the ssl_context_ is created only after calling start(), therefore it is invalid when setting the callback before start, as recommended. Calling it after calling start will probably have no effect with current implementation). It should still be set before start(), but is shouldn't crash so maybe it should be stored somewhere and set inside start()).

- it has only 1 parameter. The SSL callback expected by boost::asio has 2 (same as the underlying OpenSSL), see boost::asio::ssl::basic_context.hpp:

\" * `param callback The function object to be used for obtaining the password.

• The function signature of the handler must be:

• `code std::string password_callback(

  • std::size_t max_length, // The maximum size for a password.
    
  • password_purpose purpose // Whether password is for reading or writing.
    
  • ); @endcode
    
  • The return value of the callback is a string containing the password. \"

Can you please correct this ? In which release should the fix be expected ? It would be useful to have a patch with the fix before the release.

Thank you!

Is it really necessary to call boost::bind(cb, _1) inside Server::setSslPasswordCallback ? I guess the caller of WServer::setSslPasswordCallback should provide a binded callback, something like this:

WebServer.setSslPasswordCallback(boost::bind(&Class::callback, this, _1))... well there should be 2 parameters...

The idea is to throw away the second parameter here. It will be 'for_reading' anyway, since we never write certificates in the httpd. Am I mistaken? What would you do with the second parameter?

I'm not sure why this call crashes. ssl_context_ is created int he constructor, so your claim in your first post doesn't seem correct to me. Do you have a stack trace?

BR,

Wim.

Regarding the number of parameters for the callback, basically using only the first should be OK for WT server as usually the callback is used for reading.

But what I can say is that in boost\asio\ssl\detail\openssl_context_service.hpp it is called with 2 parameters (I'm not sure whether the second parameter is ignored in this case):

static int password_callback(char* buf, int size, int purpose, void* data)
{
    if (data)
    {
      password_callback_type* callback =
        static_cast<password_callback_type*>(data);
      std::string passwd = (*callback)(static_cast<std::size_t>(size),
          purpose ? context_base::for_writing : context_base::for_reading);
    ...
}

and it is typedef-ed as:

  // The type for the password callback function object.
  typedef boost::function<std::string(std::size_t,
      context_base::password_purpose)> password_callback_type;

Regarding the crash, the way I see it working, based on the call stack, is that the Impl::Server is created during the WServer::start(). Impl::Server constructor initializes the ssl_context_ also. So the issue might be that the ssl_context_ is not initialized before calling WServer::start().

    .dll!http::server::Server::Server(const http::server::Configuration & config={...}, Wt::WServer & wtServer={...})  Line 77  C++
    .dll!Wt::WServer::start()  Line 150 + 0x47 bytes    C++

which is called after calling WServer::setSslPasswordCallback. The stack for the crash is:

    ntdll.dll!NtRaiseException()  + 0x12 bytes  
    [Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll] 
>   .dll!http::server::Server::setSslPasswordCallback(boost::function<std::basic_string<char,std::char_traits<char>,std::allocator<char> > __cdecl(unsigned int)> cb={...})  Line 241   C++
    .dll!Wt::WServer::setSslPasswordCallback(boost::function<std::basic_string<char,std::char_traits<char>,std::allocator<char> > __cdecl(unsigned int)> cb={...})  Line 235    C++

I'm calling WServer::setSslPasswordCallback with this callback:

std::string ssl_callback(std::size_t);

WServerObj.setSslPasswordCallback(boost::bind(&Object::ssl_callback, this, _1));

Thanks for the quick response. I will try it, but I would have a few questions regarding the implementation:

- is it really necessary to use WT_API for the callback declaration in WServer (in my opinion it should not be dllexport-ed). As a matter of fact it shouldn't be accessible to the outside world in any way (since this callback exposes sensible data).

So, this would be my other questions:

- does the provided implementation makes sure that the callback will not be accessible, once set, to anyone else than the Server implementation ?

• is there any approach to store it in one place only (in WServer or the server configuration). Or at least clear it from the WServer after setting it into the server configuration.

I think that using the callback should be as limited as possible to the area where is needed.

To aoid storing the callback in WServer.C::Impl and in http::Configuration I would do this:

void WServer::setSslPasswordCallback(ssl_password_cb_t cb)
{
  BOOST_ASSERT(impl_->serverConfiguration_);
  impl_->serverConfiguration_->setSslPasswordCallback(cb_);
}

and have the documentation in WServer changed:

  /*! \brief Change input method for server certificate passwords (http backend)
   *
   * ...
   * This function must be called after *setServerConfiguration* and before calling start().
   *
   * ...
   */
  WT_API void setSslPasswordCallback(ssl_password_cb_t cb);

That would be OK too, but would be a bit uglier if the user calls this method before setServerConfiguration due to the assert.