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

Do not disable certificate verification connecting to Redis #115

Merged
merged 1 commit into from
Sep 28, 2022

Conversation

enerdgumen
Copy link
Contributor

@enerdgumen enerdgumen commented Sep 28, 2022

Related to:

  • DO-1331 Redis - Certificato SSL non correttamente verificato
  • MOTOR-1735 Update Redis 6 domain configuration and SSL settings

@enerdgumen enerdgumen requested a review from a team as a code owner September 28, 2022 11:02
@cpiemontese cpiemontese requested a review from Fs00 September 28, 2022 12:09
Copy link
Contributor

@cpiemontese cpiemontese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need a new release ASAP or do you want to test by pointing to master?

@enerdgumen
Copy link
Contributor Author

We are not sure whether to apply these changes before or after fixing the Redis settings (see comments in card).

@primait/shared-services do you have an opinion about this?

@cpiemontese
Copy link
Contributor

We could wait a bit and merge/release after you fix the Redis settings, but even if we did everything before I don't think you'd be losing out on patches or something like that...

@enerdgumen
Copy link
Contributor Author

enerdgumen commented Sep 28, 2022

do you want to test by pointing to master?

I tested this settings on staging and everything is ok:

iex(atreyu@atreyu-web-deployment-f678b8bf9-86vqm)13> {:ok, r} = Redix.start_link("redis://atreyu:***redacted***@master.redis-staging-cluster.oznmfz.euw1.cache.amazonaws.com/46", ssl: true, socket_opts: [ customize_hostname_check: [ match_fun: :public_key.pkix_verify_hostname_match_fun(:https) ] ] )
{:ok, #PID<0.8840.0>}

iex(atreyu@atreyu-web-deployment-f678b8bf9-86vqm)14> Redix.command(r, ["get", "prima_auth0_ex_tokens:atreyu:peano"]) 
{:ok, "06aXrrIwQVf0Wp2BRuefa3hmfuBqG78mcmnQkyv69lt5T..."}

I don't think you'd be losing out on patches or something like that

Ok, since there isn't any shortcoming I would say to release it before fixing the settings, so that we will fix the security issue in one shot.
It's not urgent for us. Just keep in mind that this is now a requirement for our task, planned for the next sprint. 🙂

@cpiemontese cpiemontese merged commit b3b7aed into master Sep 28, 2022
@cpiemontese cpiemontese deleted the fix-ssl-verification branch September 28, 2022 12:59
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.

2 participants