Skip to content

sectransp: support CURLINFO_CERTINFO#7372

Closed
sergio-nsk wants to merge 2 commits into
curl:masterfrom
sergio-nsk:sectransp
Closed

sectransp: support CURLINFO_CERTINFO#7372
sergio-nsk wants to merge 2 commits into
curl:masterfrom
sergio-nsk:sectransp

Conversation

@sergio-nsk
Copy link
Copy Markdown
Contributor

I need it for verifying certificate chains with root certificates missing in system storages.

Fixes #4130

Also fixed compiler warning: unused buffer, when the option CURL_DISABLE_VERBOSE_STRINGS is ON.

Comment thread docs/libcurl/opts/CURLINFO_CERTINFO.3 Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't make it into 7.78.0, so it should sat 7.79.0. That being said, I think we need a more structured way to document the version availability than free text like this - that's not the responsibility of this patch but it's worth to start thinking about.

Comment thread lib/connect.c Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unrelated?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is unrelated. It fixes the warning unused local variable buffer when I compile with cmake option -DCURL_DISABLE_VERBOSE_STRINGS=ON. I can remove this change it from this PR.

@danielgustafsson danielgustafsson added feature-window A merge of this requires an open feature window TLS labels Jul 10, 2021
@danielgustafsson
Copy link
Copy Markdown
Member

danielgustafsson commented Jul 10, 2021 via email

@sergio-nsk
Copy link
Copy Markdown
Contributor Author

Please do, and file it as its own PR. That smells like a bug that we should fix in 7.78 already.

See #7377.

Updated the supported version.

@bagder bagder removed the feature-window A merge of this requires an open feature window label Aug 8, 2021
@bagder
Copy link
Copy Markdown
Member

bagder commented Aug 8, 2021

Can you please rebase this (and fix the merge conflicts) and force-push?

@bagder
Copy link
Copy Markdown
Member

bagder commented Aug 16, 2021

The sectransp CI build failed:

vtls/sectransp.c:2975:66: error: expected ')'
        result = collect_server_cert_single(data, server_cert, i);
                                                                 ^
vtls/sectransp.c:2974:64: note: to match this '('
        server_cert = (SecCertificateRef)CFArrayGetValueAtIndex(server_certs,
                                                               ^
vtls/sectransp.c:2957:47: error: implicit conversion loses integer precision: 'CFIndex' (aka 'long') to 'int' [-Werror,-Wshorten-64-to-32]
        result = Curl_ssl_init_certinfo(data, count);
                 ~~~~~~~~~~~~~~~~~~~~~~       ^~~~~
vtls/sectransp.c:2972:47: error: implicit conversion loses integer precision: 'CFIndex' (aka 'long') to 'int' [-Werror,-Wshorten-64-to-32]
        result = Curl_ssl_init_certinfo(data, count);
                 ~~~~~~~~~~~~~~~~~~~~~~       ^~~~~

@sergio-nsk
Copy link
Copy Markdown
Contributor Author

It seems I've broken the code while fixing the merge conflicts.

@bagder
Copy link
Copy Markdown
Member

bagder commented Aug 17, 2021

Thanks!

@bagder bagder closed this in 1828f6a Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

curl built with DarwinSSL doesn't collect server cert

3 participants