Direct username + password does not work after change in php-amqplib about laravel-amqp HOT 13 CLOSED

Hi,

Adding check for !empty($ssl_options) into AMQPSSLConnection fixes the problem.

        if (!empty($ssl_options) && !isset($ssl_options['verify_peer'])) {
            $ssl_options['verify_peer'] = true;
        }

I'll send this to php-amqplib also a bit later.

Thanks to you!
-Olli

from laravel-amqp.

Hi,

Without further investigation my first try would be to set 'ssl' to null in options which should trigger the empty condition. (as long as $ssl_protocol = null matches it should work without ssl)

But maybe we also have a look into our connect() method which directly use AMQPSSLConnection.

-skdo

from laravel-amqp.

Hi,

It seems that setting 'ssl_options' => ['ssl' => null] is not enough. I'll try to find the place where ssl is forced and i'll inform you if i find something.

-Olli

from laravel-amqp.

Hi,

you might be right. We most likely need to add the new constructor parameter otherwise it's always set to ssl.
Here is the problem: https://github.com/bschmitt/laravel-amqp/blob/master/src/Request.php#L43

As a test, can you change that directly on your side and add an additional null? So it must look like this afterwards:

        $this->connection = new AMQPSSLConnection(
            $this->getProperty('host'),
            $this->getProperty('port'),
            $this->getProperty('username'),
            $this->getProperty('password'),
            $this->getProperty('vhost'),
            $this->getProperty('ssl_options'),
            $this->getProperty('connect_options'),
            null
        );

If that works we might need to add some more ssl configuration there.

Do you have any idea why they introduced redundant configuration there? Seems like they want ssl as a sane default which would make sense but breaks stuff.

-skdo

from laravel-amqp.

Hi,

I just tried to add that extra null as well but it wouldn't help. I think the problem might be somewhere even deeper.

I think they had a problem when someone wanted to use tls and at the same time they introduced this $ssl_protocol parameter defaulting to ssl.

-Olli

from laravel-amqp.

Hi,

This piece of code in constructor of AMQPSSLConnection causes that $options['ssl'] is always set and not empty.

       if (!isset($ssl_options['verify_peer'])) {
           $ssl_options['verify_peer'] = true;
       }

This enforces SSL to be used always. I have no quick fix for this :) Maybe it is a problem in their side.

-Olli

from laravel-amqp.

@olanko Thank you for the clarification.

I would like them to fix this problem prior to us. As we just consume them.
What do you think? Is this closed on our side?

This might also be a feature request to use tls instead of ssl which should not be possible without further enhancement.

from laravel-amqp.

Yes, I would consider this closed in here. I'm just writing an issue on their side.

-Olli

from laravel-amqp.

Thanks. Please don't hesitate to link the issue you create here so we can track this further.

from laravel-amqp.

Hi,

PR is here: php-amqplib/php-amqplib#674 and issue report here php-amqplib/php-amqplib#672

-Olli

from laravel-amqp.

Their comment was simple that why to use AMQPSSLConnection if ssl is not defined. I understand their point and I think I know the reason why it is done this way after all.

I'll fix our problem locally for now and hope you'll find a solution some day :)

All the best,
-Olli

from laravel-amqp.

Thanks for your efforts, I'm gonna prepare a PR later today. The handy thing with the old implementation was that you could define by configuration if you want to use SSL or not.

from laravel-amqp.

New version 2.0.4 has been published.

from laravel-amqp.