Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing AF_UNIX support from 0.9.13 #498

Open
Parakleta opened this issue Feb 6, 2022 · 9 comments
Open

Missing AF_UNIX support from 0.9.13 #498

Parakleta opened this issue Feb 6, 2022 · 9 comments
Labels

Comments

@Parakleta
Copy link

This issue presents itself in x11vnc, but is caused by a deviation between this project and 0.9.13 of the original project.

Specifically, running x11vnc -display :0 -rfbport 0 -unixsock /tmp/sock launches without a problem, but every attempt to connect using a client is rejected with the following error:

check_access: denying empty host IP address string.
denying client:  does not match (null)

The relevant code is in rfbNewTCPOrUDPClient. Specifically, after

the following code appears instead:

    if(isUDP) {
      rfbLog(" accepted UDP client\n");
    } else {
      struct sockaddr_in addr;
      socklen_t addrlen = sizeof(addr);
      memset(&addr, 0, sizeof(addr));

      if (getpeername(sock, (struct sockaddr *)&addr, &addrlen) < 0) {
        cl->host = strdup("NON_SOCKET");
      } else if (addr.sin_family == AF_UNIX) {
        struct sockaddr_un uaddr;
        addrlen = sizeof(uaddr);
        memset(&uaddr, 0, sizeof(uaddr));
        if (getsockname(sock, (struct sockaddr *)&uaddr, &addrlen) < 0) {
          cl->host = strdup("NAMEFAIL_AF_UNIX");
        } else if (!strcmp(uaddr.sun_path, "")) {
          cl->host = strdup("UNNAMED_AF_UNIX");
        } else {
          cl->host = strdup(uaddr.sun_path);
        }
      } else if (addr.sin_family == AF_INET) {
        if (!rfbSetTcpNoDelay(sock)) {
          close(sock);
          return NULL;
        }
        cl->host = strdup(inet_ntoa(addr.sin_addr));
      } else {
        cl->host = strdup("UNKNOWN_AF");
      }

The specific feature difference of this code is that it always sets cl->host so some value (except in the case of isUDP, not sure what the path is supposed to be there?). This is important because the callback cl->screen->newClientHook(cl) later in this same method does the access check that fails as shown above.

Curiously there is a bit of code in the x11vnc project under check_unix_sock() which updates the value of cl->host but this occurs only after the check_access method has already failed, and I cannot see any historical version of this project where that would have ever worked.

@Parakleta Parakleta added the bug label Feb 6, 2022
@bk138
Copy link
Member

bk138 commented Feb 6, 2022

Thanks for reporting. Do you think this is a regression? I.e. is there a LibVNCServer version that worked before?

@Parakleta
Copy link
Author

This is definitely a regression. This use case (unix sockets) worked in the 0.9.13 source tarball download from sourceforge, and in the distros that based off that.

I cannot find this code in any repository however, just in the source tarball, so I’m not sure what has happened with it or what exactly it came from.

@bk138
Copy link
Member

bk138 commented Feb 7, 2022

@Parakleta wait, you're talking about x11vnc or libvncserver?

@Parakleta
Copy link
Author

Parakleta commented Feb 7, 2022

Both. There is functionality documented in x11vnc which doesn’t work due to code missing from libvncserver.

There is no reasonable way that I can see to fix the issue in x11vnc because it relies on information that libvncserver doesn’t collect or provide, so this has to be resolved here by integrating somehow the code sample I provide from 0.9.13.

@bk138
Copy link
Member

bk138 commented Feb 8, 2022

Ah so you mean x11vnc 0.9.13 and not LibVNCServer 0.9.13? I propose you simply file a pull request and we can discuss there or if the code was in LibVNCServer but is now broken, flag the commit that caused the change so we can revert/change.

@Parakleta
Copy link
Author

The relevant code is in the file I linked above but unfortunately I cannot flag the relevant commit because the code was never in this fork and I cannot find a commit for it in the sourceforge repository either. The only copy of it I can find is in the tarball.

@Parakleta
Copy link
Author

Ideally if you can find the person who made the 0.9.13 tarball and get a copy of their (possibly) unpublished repository you may be able to cherrypick the commits onwards from 0.9.12 and try to rebase them on top this repo. Failing that you’ll probably need to rebase this repo on top of the 0.9.13 code and diff it against HEAD to generate a patch which can replace the missing functionality (which would then be applied against the current HEAD).

Both of these tasks are way to complex and dangerous for me to undertake with my lack of knowledge of the design and history of this project.

@bk138
Copy link
Member

bk138 commented Feb 15, 2022

TBH, I'm a bit confused as the LibVNCServer and x11vnc version numbers are somewhat close by. Are you referring to 0.9.12 and 0.9.13 libvncserver or x11vnc?

Is the point you're making maybe that there's an older x11vnc release that had it's own internal copy of libvncserver and this deviates from this here repository regarding the Unix sockets support?

@Parakleta
Copy link
Author

Is the point you're making maybe that there's an older x11vnc release that had it's own internal copy of libvncserver and this deviates from this here repository regarding the Unix sockets support?

Exactly this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants