Remove incorrect upper size limits for SSL certificates in NetSSL_Win#2603
Merged
obiltschnig merged 1 commit intopocoproject:developfrom Aug 16, 2019
Merged
Conversation
…rtificates are larger than the limit; let the underlying API decide
Author
|
Hello guys, any new info on this one? It seems to be a rather simple fix, we tested it in our production for about half a year now. Is there any reason you don't even want to consider this one? Maybe I can help you somehow or change the description? |
Author
|
Wow, that was fast) Thank you! We have several more corrections to this Win SSL module, we'll submit them in the nearest future. |
obiltschnig
added a commit
that referenced
this pull request
Aug 19, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In our experience, many valid certificates which we used were larger than the limits set in code (4096 bytes) and thus couldn't be used with our app at all. I believe this restriction (4096) comes from confusing max key size in bits with the file size. In any case I think that calls to the underlying WinAPI should return an error in case the certificate file is defective in some way.
We've been using this "fix" for several months in production, no problems whatsoever. Don't know about the lower size in X509Certificate.cpp (size < 32) though - following the same logic maybe it should be removed too. Anyways we haven't seen a certificate as small as that.
The linux NetSSL seems to work regardless of certificate sizes, so I suppose there is no such confusion there. We haven't experienced any issues with our linux builds and larger certs.