Schannel read from file simplify

[Vasiliy-Kkk]

In schannel_verify.c, curl now uses curlx_CreateFile to read ca_file. This code builds only on Windows, which is okay. However, I suggest using a more common and understandable API, such as curlx_fopen.

[bagder]

If we do this, we can convert curlx_CreateFile into a tool-specific function since this is the only user in libcurl.

[Copilot]

Pull request overview

This PR refactors Schannel CA bundle loading in lib/vtls/schannel_verify.c to use curl’s cross-platform file APIs (curlx_fopen/curlx_fseek/curlx_fclose) and stdio reads instead of Windows CreateFile/ReadFile logic, simplifying the implementation while remaining Windows-only code.

Changes:

• Switch CA file opening/closing from WinAPI handles to curlx_fopen/curlx_fclose.
• Replace WinAPI file sizing/reading with curlx_fseek + ftell + fread.
• Preserve the existing “read whole file into memory (up to 1 MiB)” behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[vszakats]

If we do this, we can convert curlx_CreateFile into a tool-specific function since this is the only user in libcurl.

I wonder if its two uses in tool_filetime.c may also be eliminated in favor of CRTL calls? Supported Windows platforms have HAVE_UTIME.

[jay]

I wonder if its two uses in tool_filetime.c may also be eliminated in favor of CRTL calls? Supported Windows platforms have HAVE_UTIME.

there is a dst issue, see comments

curl/src/tool_filetime.c

Lines 35 to 41 in a05f349

	int getfiletime(const char *filename, curl_off_t *stamp)
	{
	int rc = 1;
	/* Windows stat() may attempt to adjust the Unix GMT file time by a daylight
	saving time offset and since it is GMT that is bad behavior. When we have
	access to a 64-bit type we can bypass stat and get the times directly. */

curl/src/tool_filetime.c

Lines 86 to 90 in a05f349

	void setfiletime(curl_off_t filetime, const char *filename)
	{
	/* Windows utime() may attempt to adjust the Unix GMT file time by a daylight
	saving time offset and since it is GMT that is bad behavior. When we have
	access to a 64-bit type we can bypass utime and set the times directly. */

[vszakats]

I wonder if its two uses in tool_filetime.c may also be eliminated in favor of CRTL calls? Supported Windows platforms have HAVE_UTIME.

there is a dst issue, see comments

curl/src/tool_filetime.c

Lines 35 to 41 in a05f349

	int getfiletime(const char *filename, curl_off_t *stamp)
	{
	int rc = 1;
	/* Windows stat() may attempt to adjust the Unix GMT file time by a daylight
	saving time offset and since it is GMT that is bad behavior. When we have
	access to a 64-bit type we can bypass stat and get the times directly. */

curl/src/tool_filetime.c

Lines 86 to 90 in a05f349

	void setfiletime(curl_off_t filetime, const char *filename)
	{
	/* Windows utime() may attempt to adjust the Unix GMT file time by a daylight
	saving time offset and since it is GMT that is bad behavior. When we have
	access to a 64-bit type we can bypass utime and set the times directly. */

Indeed!

Would it be possible to still switch to curlx_fopen(), obtain the Win32 file handle
via _get_osfhandle() and call SetFileTime() with it? (_get_osfhandle() is
already used and CRT _futime*() uses it internally the same way.)

[Vasiliy-Kkk]

I have another suggestion, but I’m not sure where it would be best to discuss it, and it may not be worth opening a separate issue, so I’ll mention it here, as in the nearby PR discussion.

In schannel, the Curl_verify_certificate function in hExclusiveRoot uses only the certificates provided by the user. It seems to me that it could be useful to allow the Windows Root Store to be used when verifying the certificate chain. What do you think about that?

This should be fairly easy to implement in the code, and it seems to me that using the system root store could be a good option in some cases. It would be useful for me.

[vszakats]

I have another suggestion, but I’m not sure where it would be best to discuss it, and it may not be worth opening a separate issue, so I’ll mention it here, as in the nearby PR discussion.

In schannel, the Curl_verify_certificate function in hExclusiveRoot uses only the certificates provided by the user. It seems to me that it could be useful to allow the Windows Root Store to be used when verifying the certificate chain. What do you think about that?

This should be fairly easy to implement in the code, and it seems to me that using the system root store could be a good option in some cases. It would be useful for me.

I may be misunderstanding your question, but with Schannel, the Windows Root Store is
used by default, if the user doesn't provide a CA bundle themself. Do you mean to use
the Windows Root Store as fallback when the user explicitly provided a .crt file/directory?

Though I suggest https://github.com/curl/curl/discussions for discussing topics not strictly
related to this PR.

[Vasiliy-Kkk]

I answered in discussion.

[jay]

Would it be possible to still switch to curlx_fopen(), obtain the Win32 file handle
via _get_osfhandle() and call SetFileTime() with it? (_get_osfhandle() is
already used and CRT _futime*() uses it internally the same way.)

It would work but not as well, I would leave it alone even if it leaves us with the CreateFile wrapper.

The current code opens the file with only attribute access which is supposed to be faster and unaffected by if the file is already open, regardless of sharing mode. As I understand it Windows allows just attribute access even if the file open shared.

If the code is changed to fopen then the file would have to be opened for write to write the attributes. Actually opening the file (not just opening access to presumably NTFS file system for attributes) is likely to be slower.

(edit: I may not be right about having to open the file for write because if you do that then maybe it would set the file's last modified time to the time it was open for write? Not sure about this. In any case the file would have to be actually opened and closed with generic access because I assume that's what fopen does and is likely to be slower).

[vszakats]

Would it be possible to still switch to curlx_fopen(), obtain the Win32 file handle
via _get_osfhandle() and call SetFileTime() with it? (_get_osfhandle() is
already used and CRT _futime*() uses it internally the same way.)

It would work but not as well, I would leave it alone even if it leaves us with the CreateFile wrapper.

The current code opens the file with only attribute access which is supposed to be faster and unaffected by if the file is already open, regardless of sharing mode. As I understand it Windows allows just attribute access even if the file open shared.

If the code is changed to fopen then the file would have to be opened for write to write the attributes. Actually opening the file (not just opening access to presumably NTFS file system for attributes) is likely to be slower.

(edit: I may not be right about having to open the file for write because if you do that then maybe it would set the file's last modified time to the time it was open for write? Not sure about this. In any case the file would have to be actually opened and closed with generic access because I assume that's what fopen does and is likely to be slower).

Fair points.

I guess another way to approach this may be to set the timestamp on the
already opened file, before closing it. This would have other advantages
and the best performance. This'd still not help with the reading part, which'd
then need tests for regressions in shared situations (and performance).

[jay]

Thanks