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

WIP: Added skip_ssl_verify setting to OX push driver #191

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TitaniumHocker
Copy link

This is WIP patch, I didn't tested it yet.
Updates about tests will be provided later.
I'll update documentation after this PR will be merged.

@cmouse
Copy link
Contributor

cmouse commented Dec 7, 2022

Hi.

You should probably use lua driver instead of modifying this driver, as it is heavily interdependent on Open-Xchange Appsuite. Also, if you want to do this, I would recommend adding the certificate (or CA cert) to your system store instead.

@TitaniumHocker
Copy link
Author

TitaniumHocker commented Dec 7, 2022

You should probably use lua driver instead of modifying this driver, as it is heavily interdependent on Open-Xchange Appsuite.

In my opinion, using lua scripts is actually adding new point of errors. This is not necessary for just disabling ssl verification.

Also, if you want to do this, I would recommend adding the certificate (or CA cert) to your system store instead.

In my case it's unavailable option.

@TitaniumHocker TitaniumHocker force-pushed the push-skip-ssl-verify branch 2 times, most recently from 1f9dcc3 to 83782ca Compare December 7, 2022 14:03
@sirainen
Copy link
Contributor

sirainen commented Dec 7, 2022

We're not much changing v2.3 anymore, and I'm planning on doing larger changes to v2.4 configuration handling which will make this possible in a generic way.

@TitaniumHocker
Copy link
Author

TitaniumHocker commented Dec 7, 2022

We're not much changing v2.3 anymore, and I'm planning on doing larger changes to v2.4 configuration handling which will make this possible in a generic way.

As I see you are planning to do changes that will solve my problem.
This patch will solve my problem much earlier then it may be solved in one of v2.4.x releases.
It just forwards the setting from config, nothing complicated.

@sirainen
Copy link
Contributor

sirainen commented Dec 7, 2022

I mean, unless something unexpected happens, the first v2.4 release will have this solved in a different way (any ssl setting can be specified in any context). I'm actively working on the config rewrite now.

@TitaniumHocker
Copy link
Author

I mean, unless something unexpected happens, the first v2.4 release will have this solved in a different way (any ssl setting can be specified in any context). I'm actively working on the config rewrite now.

Could you clarify when the release of v2.4 is expected? The last release was almost half a year ago.

@sirainen
Copy link
Contributor

sirainen commented Dec 7, 2022

Could you clarify when the release of v2.4 is expected?

There is no schedule for now. Not anytime soon.

@TitaniumHocker
Copy link
Author

TitaniumHocker commented Dec 7, 2022

There is no schedule for now. Not anytime soon.

Based on this information, I can only insist on applying this patch.

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.

3 participants