treeform / puppy

Puppy fetches via HTTP and HTTPS
MIT License
184 stars 27 forks source link

Draft for adding insecure certificates option #68

Closed Techno-Fox closed 1 year ago

Techno-Fox commented 2 years ago

The linux and windows are working, however I'm having issues with the macos side of things. I don't have much experience with macos, so please forgive my ignorance. I look forward to discussing this with you. Also. Sorry if my replies seem a little late. Work can be long.

treeform commented 2 years ago

This is missing tests. Is it possible to create a debug test server with a self signed cert?

Techno-Fox commented 2 years ago

I've made a test server yes. I didn't add the tests, because I didn't know if you would want that or not. But yes. I can provide the tests. It might take a while though. Getting ready for work.

Techno-Fox commented 2 years ago

I've added a test block "insecure" to tests.nim

I've also added test server in python

and I've also added self signed certificates utilizing openSSL.

Techno-Fox commented 2 years ago

So I've tried both of your recommendations. with setAllowsAnyHTTPSCertificate, however I still get the error attached as a screenshot.

image

treeform commented 2 years ago

Yeah setAllowsAnyHTTPSCertificate is a private API so apple can and probably just removed it or it might have different arguments now. We need to find an official way to do this that is documented by Apple.

Techno-Fox commented 2 years ago

So, if my research is correct. Apple API doesn't allow self signed certificates (Not without the private api).

guzba commented 1 year ago

@Techno-Fox If you're still interested in this PR and would like your version merged, consider https://github.com/treeform/puppy/pull/79 as some feedback (no loop on Windows, smaller Python server, more clear name since insecure would need docs saying allow any https certificate so mind as well just call the flag that.

I'm sure @treeform will have opinions too and you're welcome to share your thoughts. Just trying to save a bunch of back and forth. Not trying to steal your PR commit if you would like that accomplishment.

Techno-Fox commented 1 year ago

Yes actually. I would still be interested in merging. Thank you for just not stealing. Accomplishments are always welcome.

Techno-Fox commented 1 year ago

It's been a while. Mind catching me up on what's happening with the PR?

Techno-Fox commented 1 year ago

If I remember correctly. During testing it didn't work without the loop on Windows. Unless someone has found a way to do so.

Techno-Fox commented 1 year ago

I thought I did make the variable name to allowAnyHttpsCertificate?

Techno-Fox commented 1 year ago

And how small do you want the python server?

guzba commented 1 year ago

I thought I did make the variable name to allowAnyHttpsCertificate?

It is still insecure for this PR.

https://github.com/treeform/puppy/blob/1a009e36826357f11718fd7f47c1f2e845f45328/src/puppy/common.nim#L18

guzba commented 1 year ago

If I remember correctly. During testing it didn't work without the loop on Windows. Unless someone has found a way to do so.

A loop is not needed, just a second call to WinHttpSendRequest, see https://github.com/treeform/puppy/pull/79/files#diff-1ebf7a29255776895f8c7e956c2c1a4ffa73ceec647a3723a5c75e0712a253ff

Note I want to ignore the retry error, unless we can reproduce and test it. It is not needed in my testing for self-signed certs.

guzba commented 1 year ago

And how small do you want the python server?

The server in this PR includes some big ###### comments that don't add anything to such a small test file. I'd like to just have the simple Python. That's probably the only thing I'd prefer changed.

guzba commented 1 year ago

Yes actually. I would still be interested in merging. Thank you for just not stealing. Accomplishments are always welcome.

No problem, we'll get this merged soon here with just a few tweaks.

Techno-Fox commented 1 year ago

K. Thanks for letting me know what's needed. I'm just getting off work, so I'll see about fixing these possibly tomorrow (my time at least)

guzba commented 1 year ago

K. Thanks for letting me know what's needed. I'm just getting off work, so I'll see about fixing these possibly tomorrow (my time at least)

Sounds good!

Techno-Fox commented 1 year ago

sorry that took so many tries, made a python script that could look at the diffs of each commit, and well it was an old script, so I had to fix some bugs.

Techno-Fox commented 1 year ago

Let me know if I've forgot anything

Techno-Fox commented 1 year ago

alright, I'll get on it

Techno-Fox commented 1 year ago

I've added the function parameters

Techno-Fox commented 1 year ago

sorry for the wait. Had work.

treeform commented 1 year ago

Thank you for this PR. Its a big one. We appreciate you working with us to get it in.

Congrats!

Techno-Fox commented 1 year ago

Welcome.