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

[1.x] Fix the issue for connecting if the SSL is not verified peer #85

Merged
merged 5 commits into from
Mar 27, 2024

Conversation

michaelnabil230
Copy link
Contributor

See: #78

@vatsake
Copy link

vatsake commented Mar 14, 2024

If the certificate does not need local_pk, then it won't load local_cert at the moment.

I think part of the tls content should be commented out.

@joedixon
Copy link
Collaborator

I want to sit on this one a little while as I'm not convinced it's needed.

@taylorotwell taylorotwell marked this pull request as draft March 14, 2024 15:55
@mateusztumatek
Copy link

This fixed my problem during local development with Laravel Valet. Thanks :)

@benbjurstrom
Copy link

I want to sit on this one a little while as I'm not convinced it's needed.

I wouldn't sit on this. I've had nothing but trouble trying to get reverb working locally on MacOS with Herd secured sites. The most frustrating part is it will work one day and be broken the next.

The changes to the Factory.php with this config entry resolves my problems:

            'options' => [
                'tls' => [
                    'verify_peer' => false,
                ],
            ],

I would suggest adding verify_peer as it's own env variable.

@lsdevelop
Copy link

Really, the problem has solved, please @joedixon merge this, If you want, I can make another more detailed PR.

@joedixon
Copy link
Collaborator

Pushed some updates here which will resolve #93, #105 and possibly #106.

I opted not to include config by default for all of the SSL context options, but instead allow users to configure it manually.

Now, the check for Valet and Herd certificates will only be carried out if both local_cert and local_pk are not set. This means the other SSL options such as verify_peer, allow_self_signed, etc may still be used with Valet and Herd certificates.

Additionally, null values are removed from the TLS options array to allow environment variables to be used to control whether or not TLS is enabled.

@joedixon joedixon marked this pull request as ready for review March 27, 2024 12:16
@taylorotwell taylorotwell merged commit d1b72f4 into laravel:main Mar 27, 2024
9 checks passed
@lsdevelop
Copy link

Empurrei algumas atualizações aqui que resolverão #93 , #105 e possivelmente #106 .

Optei por não incluir a configuração por padrão para todas as opções de contexto SSL, mas em vez disso permitir que os usuários a configurem manualmente.

Agora, a verificação dos certificados de Valet e Rebanho só será realizada se ambos local_certe local_pknão estiverem configurados. Isso significa que as outras opções de SSL, como verify_peer, allow_self_signed, etc, ainda podem ser usadas com certificados Valet e Herd.

Além disso, nullos valores são removidos da matriz de opções TLS para permitir que variáveis ​​de ambiente sejam usadas para controlar se o TLS está habilitado ou não.

This update broken forge applications running with ssl

@joedixon
Copy link
Collaborator

Hi @lsdevelop, can you elaborate on what was broken? Are you running your Reverb server directly from the main branch?

@lsdevelop
Copy link

lsdevelop commented Mar 28, 2024

Hi @lsdevelop, can you elaborate on what was broken? Are you running your Reverb server directly from the main branch?

Hi, yeah,

I have a chat app with deprecated websockets, after launching reverb, I up stating environment in forge for testing using @beta of reverb,

After merge this commit in next deployment the connection has return error for tls.

Solved after downgrade for beta4, I'm trying again now.

@joedixon
Copy link
Collaborator

@lsdevelop you would have only seen this update if you explicitly used dev-main or @develop. This PR has not been released yet and beta4 is still currently the latest release.

Can you confirm the steps you took and verify the error you saw?

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

Successfully merging this pull request may close these issues.

7 participants