WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
124569
[curl] Improve ssl certificate storage and check
https://bugs.webkit.org/show_bug.cgi?id=124569
Summary
[curl] Improve ssl certificate storage and check
sipka
Reported
2013-11-19 01:08:18 PST
Created
attachment 217277
[details]
proposed patch Storage and check the whole certificate chain, not just the root certificate.
Attachments
proposed patch
(3.79 KB, patch)
2013-11-19 01:08 PST
,
sipka
no flags
Details
Formatted Diff
Diff
proposed patch
(3.84 KB, patch)
2013-11-21 01:48 PST
,
sipka
no flags
Details
Formatted Diff
Diff
proposed patch
(4.65 KB, patch)
2013-11-21 02:04 PST
,
sipka
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Brent Fulgham
Comment 1
2013-11-19 16:53:02 PST
Comment on
attachment 217277
[details]
proposed patch View in context:
https://bugs.webkit.org/attachment.cgi?id=217277&action=review
Looks good overall. I have some concern about the BIO_get_mem_data call (though I realize this wasn't code you added). Can you review and let me know what you think?
> Source/WebCore/platform/network/curl/SSLHandle.cpp:146 > + unsigned char *certificateData;
unsigned char* certificateData;
> Source/WebCore/platform/network/curl/SSLHandle.cpp:147 > + long len = BIO_get_mem_data(bio, &certificateData);
The BIO_get_mem_data documentation is pretty weak. Does it ever return a negative value? If not, why is the return value signed? If it is negative, then the following code will do bad things.
sipka
Comment 2
2013-11-21 01:47:39 PST
(In reply to
comment #1
)
> (From update of
attachment 217277
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=217277&action=review
>
Thanks for the detailed review.
> Looks good overall. I have some concern about the BIO_get_mem_data call (though I realize this wasn't code you added). Can you review and let me know what you think? >
BIO_get_mem_data() sets pp to a pointer to the start of the memory BIOs data and returns the total amount of data available. length = BIO_get_mem_data(bio, &certificateData); // here - certificateData is a pointer to encoded data, length - length of data.
> > Source/WebCore/platform/network/curl/SSLHandle.cpp:146 > > + unsigned char *certificateData; > > unsigned char* certificateData;
> I changed this.
> > Source/WebCore/platform/network/curl/SSLHandle.cpp:147 > > + long len = BIO_get_mem_data(bio, &certificateData); > > The BIO_get_mem_data documentation is pretty weak. Does it ever return a negative value? If not, why is the return value signed? If it is negative, then the following code will do bad things.
Yes, I made the change what is necessary to avoid unexpected behaviors in
Bug119436
which this bug depends on.
sipka
Comment 3
2013-11-21 01:48:36 PST
Created
attachment 217534
[details]
proposed patch
sipka
Comment 4
2013-11-21 02:04:28 PST
Created
attachment 217537
[details]
proposed patch Add missing ChangeLog
Brent Fulgham
Comment 5
2013-11-21 09:14:07 PST
Comment on
attachment 217537
[details]
proposed patch r=me.
WebKit Commit Bot
Comment 6
2013-11-21 09:40:42 PST
Comment on
attachment 217537
[details]
proposed patch Clearing flags on attachment: 217537 Committed
r159632
: <
http://trac.webkit.org/changeset/159632
>
WebKit Commit Bot
Comment 7
2013-11-21 09:40:45 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug