Page MenuHome GnuPG
Create Task
Maniphest T6551

translate_sys2libc_fd_int on Windows 64-bit
Open, WishlistPublic
Actions

Description

(Although it's quite a corner case, for the case of integrity of the code,) translate_sys2libc_fd_int should be fixed.

  • Its API has the argument as type int, which is not right to represent the HANDLE of Windows
    • the semantics here is that the value can be: 0, 1, 2 to represent standard input, output, and error, or else HANDLE
  • It works now (most of the case)
    • it works when the value of HANDLE can be represented in 31-bit positive integer
  • It may fail (in a corner case)
    • Even if Windows will keep the value of HANDLE within 32-bit, it has a possibility that the value may be negative in 32-bit integer
      • in this case, it may fail in the conversion to 64-bit representation
        • when the conversion is by sign extension to 64-bit, it fails
        • when the conversion is by zero extension to 64-bit, it is OK

Revisions and Commits

rG GnuPG
rG6049d61991a1 common: Fix the cast for 64-bit Windows.
rG3672c29156e3 common: Raise an error correctly in check_special_filename.
rGb9b0c183204f common,gpg,sm,tools: Don't remove translate_sys2libc_fd_int.
rG87a73e8eb0c9 common: Remove translate_sys2libc_fd_int.
rG2756147e392c gpg,sm,tools: Use string for option --*-fd.
rG04d0851ccaae common: Add gnupg_sys2libc_fdstr function.
rGe9e7b5425fdd common: Add translate_sys2libc_fdstr.

Related Objects
Search...

Event Timeline

• gniibe triaged this task as Normal priority.Jun 22 2023, 3:12 AM
• gniibe created this task.
• gniibe created this object with edit policy "Contributor (Project)".
• gniibe updated the task description. (Show Details)Jun 22 2023, 3:16 AM
• gniibe added a comment.EditedJun 22 2023, 3:46 AM

The use cases are:

  • oPassphraseFD for gpgsm, gpg
  • oStatusFD for gpg-auth, gpg-wks-client, gpg-card, gpg-pair-tool, gpgtar, gpgconf, gpgsm, gpg, gpgv
  • oLoggerFD for gpgsm, gpg, gpgv
  • oAttributeFD for gpg
  • oCommandFD for gpg
  • oOverrideSessionKeyFD for gpg

The use cases for such invocations are only in gpgtar:

  • gpgtar-extract.c, gpgtar-list.c, gpgtar-create.c for --status-fd with %d
  • the cases in tests/ by gpgscm are all with 0 or 1
• gniibe added a commit: rGe9e7b5425fdd: common: Add translate_sys2libc_fdstr..Jun 22 2023, 4:40 AM
• gniibe changed the task status from Open to Testing.Jun 23 2023, 6:38 AM

Fixed in master.

• gniibe added a commit: rG04d0851ccaae: common: Add gnupg_sys2libc_fdstr function..Jun 23 2023, 6:40 AM
• gniibe added a commit: rG2756147e392c: gpg,sm,tools: Use string for option --*-fd..
• gniibe added a commit: rG87a73e8eb0c9: common: Remove translate_sys2libc_fd_int..
• werner added a comment.Jun 23 2023, 9:03 AM

Just to clarify this change for readers not accustomed to Windows internals: This function was used to translate the file descriptor as passed to gpg (which is a HANDLE) to the libc file descriptor as used by stdio. Obviously we won't anymore work with stdio file descriptors in the future but use the Windows32 API (ReadFile et al). libc fds 0,1,2 are handled in a special way on Windows.

Note further that Windows will never assign negative values (except for INVALID_HANDLE_VALUE) or values larger that 2^31 to a a HANDLE. The background is that this would break the majority of existing Windows code as weel as the GUI API which uses casting all over the place and assumes that a (long) is a signed 32 bit value.

• werner mentioned this in rG2756147e392c: gpg,sm,tools: Use string for option --*-fd..Jun 23 2023, 9:06 AM
• gniibe added a comment.EditedJun 26 2023, 3:29 AM

I don't argue about the technical necessity for the change. I agree the fact it works (without such changes).

I'm open to revert the changes.

My point is that it identifies the points where we have possible glitch by different abstractions between a file descriptor and HANDLE.

• gniibe added a comment.Jun 27 2023, 7:41 AM

The changes are intrusive to other implementations (POSIX and Windows 32-bit).
So, I revert the changes of replacing translate_sys2libc_fd_int.

I am going to create a task for the problem where we really need gnupg_sys2libc_fdstr for gpgtar (on Windows).

• gniibe added a commit: rGb9b0c183204f: common,gpg,sm,tools: Don't remove translate_sys2libc_fd_int..Jun 27 2023, 8:01 AM
• gniibe changed the status of subtask T6562: gpgtar: --status-fd requires HANDLE (not POSIX fd) when spawning a process from Open to Testing.Jun 28 2023, 7:12 AM
• gniibe added a comment.Jun 29 2023, 7:00 AM

Except a case, all use cases of translate_sys2libc_fd_int is with a result of integer from command line argument.

Since libgpg-error's argparse doesn't have capability to have 64-bit integer on Windows (for now, as it's LLP64 architecture), when the string represents a value which cannot in int, it occurs an error at parsing the argument.

A case where we need to fix is the use case in check_special_filename. Currently, it uses atoi for Windows to call translate_sys2libc_fd_int. We have no check here.

• gniibe added a commit: rG3672c29156e3: common: Raise an error correctly in check_special_filename..Jun 29 2023, 7:09 AM
• gniibe added a commit: rG6049d61991a1: common: Fix the cast for 64-bit Windows..
• gniibe changed the task status from Testing to Open.Jul 3 2023, 4:16 AM
• gniibe lowered the priority of this task from Normal to Wishlist.

The case in check_special_filename is fixed. So, there is no cases in GnuPG where the value of out of range is silently converted to wrong value.

Now, we keep translate_sys2libc_fd_int. If anything problematic, it will be the program which should detect the error when it tries to invoke a process with --status-fd; When it will see the value of the HANDLE is out of the range (within 32-bit/31-bit), then, it is the responsibility of the program to handle the case. Or, if the program correctly formats the larger value, an invoked process raises an error (of parsing the value, as it's out of the range).

When we will encounter such an issue, in future, let us consider again about translate_sys2libc_fd_int.