-
Notifications
You must be signed in to change notification settings - Fork 2
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
CI #8
Comments
Maybe define them in Travis or encrypt them. |
What I mean is that if I disable the pinning (or whatever I'm currently doing 😇) I have to request a https domain and the credentials will get send to that. That's ok when working with fake credentials as I currently do but when testing locally with real credentials this is not what someone wants to do. However your proposal might be a solution for issue #2 . |
Ah, all right… maybe we need another issue for that as you said in the comments already. |
Yes! I'll have to read me into the topic first. My "blueprint" was the python client which has the same feature. Idk if you have influence on this but it uses a deprecated -> via SHA1. I had no time yet to create an issue. |
Yes, you obviously also had no time to look for an existing issue… 😉 So yes, and I explained there in detail why pinning the certificate is bad. You should really have a look into what HPKP is, which is a good technique for pinning (except that in your case you'd pin the hash statically instead of using the cert hash. |
For testing failing requests in case of unsuccessful HTTP Public Key Pinning there is currently a private request-bin in use to prevent leaking credentials:
https://github.com/thorsteneckel/threema/blob/55c406869528c8f31e17ed44c692745daa49062f/spec/threema/client_spec.rb#L56
https://github.com/thorsteneckel/threema/blob/55c406869528c8f31e17ed44c692745daa49062f/spec/threema/client_spec.rb#L61
This is necessary since (currently) the API credentials get added to the requests. However this is not a proper way. There should be a dedicated method for testing this.
The text was updated successfully, but these errors were encountered: