Implement host lookups #49

flokli · 2022-10-20T11:11:38Z

This adds support for the following request types:

GETAI
GETHOSTBYADDR
GETHOSTBYADDRv6
GETHOSTBYNAME
GETHOSTBYNAMEv6

Cherry-picked into one PR due to #39 (comment):

Note that they both have to be merged in the same release because glibc's NSCD client disables lookups for the whole hosts family of operations if one operation comes back unimplemented (see also #26, where it disabled lookups for the whole groups family of operations), but that shouldn't be hard to do. I don't see any other host operations I think.

Fixes #37.
Supersedes #39 and #40.

flokli · 2022-10-20T11:24:48Z

It looks like this doesn't seem to build with the rustc version you're using in the build-debian CI step:

error: attributes are not yet allowed on `if` expressions
   --> /github/home/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/socket2-0.4.7/src/sys/unix.rs:906:5
    |
906 |     #[cfg(not(any(target_os = "haiku", target_os = "openbsd")))]
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I've seen a bunch of work in #41 and #45, where you used older versions of crates to get this still to build. That might be a tactic to get stuff to work here as well, but is there a reason you can't switch to a more recent version of rustc to build stuff on Debian 10?

bbenne10 · 2023-03-03T16:31:25Z

Is there any update on using a newer rustc rather than downgrading crates for Debian 10 compatibility? I'd be interested in helping shepherd this along so that consumption is a bit easier for myself and some coworkers looking to use nsncd in a non-nixos environment where our login infrastrcture is based around sssd.

leifwalsh · 2023-03-04T00:01:30Z

Long story short: yeah, we think we can get there. Right now we build and run on Debian 10, but the only real constraint we have is to run on Debian 10 (and, honestly, we run the rest of nix that we use there too, and it seems fine so far).

I think if we can get to the point where we're building with a newer toolchain but verifying that we still run on Debian 10, we should be good to go.

If you want to take a crack at modifying the CI to do that, I think that's a great way to help us unblock the rustc upgrade. I might try to but I don't really know what I'm doing :)

bbenne10 · 2023-03-04T01:57:18Z

So I forked and started having a go at getting everything built and verified on Debian 10, but I don't fully understand ci/test.sh. The current tests only seem to be run against ubuntu:latest and it is not successfully running without modification on Debian 10. It does run and does not immediately crash or anything, but I'm not sure what I'm doing.

leifwalsh · 2023-03-05T01:01:48Z

Yeah, I poked around aimlessly too. I think we need @geofft's expertise here.

leifwalsh · 2023-03-05T01:08:42Z

I have a question for @flokli and @NinjaTrappeur: You're all presumably distributing a build of nsncd that isn't built with nix (because then it would depend on the nix libc, defeating the point), right? How are you doing that?

flokli · 2023-03-05T07:49:40Z

No, we are distributing a build of nsncd that's built with Nix.

This is used as a "NSS RPC protocol", so that we don't need to bake paths to "external nss modules" (like systemd, LDAP, avahi, ...) into all binaries, but can keep this a per-host-config thing. This is explained with a bit more context in https://flokli.de/posts/2022-11-18-nsncd/

bbenne10 · 2023-03-05T15:26:10Z

So I started to rework the workflow of the CI actions quite a bit in my fork, but I am running low on time this weekend (lots of chores around the house need doing). The general idea is as follows (everything after the debian package is somewhat speculative):

Run the tests via cargo test (which also populates ./target/ with built dependencies)
Run the build via cargo build (which builds a debug build into ./target/ but reuses the built dependencies)
Build a debian package via singingwolfboy/build-dpkg-buster and cache that for reuse in integration tests
Build the whatami nss module and cache that too
Build an RPM suitable for CentOS and RHEL (this is selfishly setting up our consumer use case) and cache it
On ubuntu, run a truncated version of ci/test.sh using the cached nss module and the cached dpkg
On debian 10, run a truncated version of ci/test.sh using the cached nss module and the cached dpkg
On CentOS, run a truncated version of ci/test.sh use cached nss module (may need another build here? Not sure) and the cached RPM

We could theoretically publish the dpkg and rpm packages as releases every so often too. Please see my fork for the current state of this work.

Note that I have not yet signed the CLA, so any contributions should not be upstreamed until I do that. I do intend to do this, but I have yet to get around to it.

EDIT: While researching for this, I came across cargo-deb. My new plan is to jettison build-dpkg-buster (It takes a huge time to set up during runs, which seems silly) and use it. Additionally, this allows a more "cargo-centric" workflow whereby your metadata largely comes from the Cargo.toml. I have started translation of the debian/ subfolder into the Cargo.toml on my fork if you'd like to play along. I'm putting this down for the day and may come back tonight or tomorrow morning for anoher run. Currently, everything builds (including nss_whatami.so.2) and the cargo test tests pass, but I'm having trouble restoring cache on a different worker and running the relevant section of ci/test.sh (...still). Not sure what is wrong on that front. See logs action logs on my fork for the error messages (Something about a cache key not being set on restore, which is wrong - I am setting it, but maybe not in a way it understands?)

bbenne10 · 2023-03-07T13:41:26Z

I have opened #58 with what I believe to be the minimum required changes to get ci building with a new Rust toolchain. There's maybe more work to do with the CI layout, but I think that that PR will get us unblocked on getting this particular PR merged (once rebased).

flokli · 2023-04-25T20:05:37Z

I'm happy to rebase this once #58 is in 👍

leifwalsh · 2023-05-05T02:00:50Z

@flokli please go ahead!

Co-Authored-By: Félix Baylac-Jacqué <felix@alternativebit.fr>

Also add some error codes used in the response header. Co-Authored-By: Félix Baylac-Jacqué <felix@alternativebit.fr>

flokli · 2023-05-10T12:40:09Z

I rebased things and addressed conflicts, but now need to also address #51.

I might not get to it this week, but I didn't forget about it!

Co-Authored-By: Félix Baylac-Jacqué <felix@alternativebit.fr>

Co-Authored-By: Florian Klink <flokli@flokli.de>

We implement the GETAI (aka. get address info) operation. Most of the glibc applications will rely on this operation to perform the hostname -> addrs resolutions. We add some unit tests checking that we're correctly serializing the address info header and its payload. The expected data used in these tests has been extracted from various Nscd socket dumps. We also add an integration test checking that getaddrinfo is able to resolve localhost. Co-Authored-By: Florian Klink <flokli@flokli.de>

codecov-commenter · 2023-05-19T12:16:27Z

Codecov Report

Patch coverage: 69.33% and project coverage change: +9.93 🎉

Comparison is base (125cb88) 41.60% compared to head (1a4bdc3) 51.53%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #49      +/-   ##
==========================================
+ Coverage   41.60%   51.53%   +9.93%     
==========================================
  Files           6        6              
  Lines         274      423     +149     
==========================================
+ Hits          114      218     +104     
- Misses        160      205      +45

Impacted Files	Coverage Δ
src/handlers.rs	`54.58% <69.01%> (+21.25%)`	⬆️
src/protocol.rs	`50.00% <75.00%> (+9.09%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Previously IPv6-only results would return an empty successful result message. This doesn't seem to be a valid response from the perspective of Glibc. In my case it caused Firefox to crash.

We don't have to use a lambda to return a value from a sub-expression. Rust takes the last value in a scope as return value even if it isn't a function.

The function belongs to the data format and not to the actual processing logic. This isn't a clean cut just yet all over the codebase but moving this very fragile piece into its own function will enable us to test it in isolation.

This makes the result filtering similar and thus should be less confusing.

This is as efficient as doing the manual pre-allocation as the `to_vec` method will use the length of the slice for the initial size allocation of the vector.

Previously this would produce a malformed packet as far as glibc is concerned. If there are no addresses the "found" field should not be one and the other lookup fields can also be left blank.

This moves the "well-known" wire format structs to constants that can be reused without having to duplicate their definition all the time.

bbenne10 · 2023-08-07T23:42:09Z

Ping :)

flokli · 2023-08-20T22:04:54Z

This isn't fully correct yet, see nix-community#4 (comment). I guess we'll update this PR once fixed.

andir · 2023-08-27T13:13:19Z

Just to clarify: The changes in this PR that are authored by me are not cleared by any CLA. I accept the Apache License 2.0, but I'll not allow arbitrary re-/sublicensing and distribution outside what the license already grants.

We internally used getai to restpond to the gethostbyname/gethostbyaddr operations. Sadly, it does not behave as expected and breaks some tools (like hostname --fqdn, see #4. We FFI the right Glibc gethostbyname_2r/gethostbyaddr_2r (now deprecated) functions and use it to back the GETHOSTBYNAME, GETHOSTBYNAME6, GETHOSTBYADDR, and GETHOSTBYADDR6 Nscd interfaces. Using sockburp, we realized the hostent serialization function was bogus: we totally forgot to serialize the aliases. This commit fixes this and makes sure we're producing bit-to-bit identical results with Nscd for gethostbyname/getaddrinfo. Took me three try to get this right. This is actually the third full rewrite. The Nscd behaviour for these two legacy functions is *really* confusing. We're supposed to ignore the herrno (herrno != errno!!) and set it to 0 if gethostbyaddr/name returns a non-null hostent. If we end up with a null hostent, we return the herrno together with a dummy hostent header. I tried to keep things as safe as possible by extracting the glibc hostent to a proper rust structure. This structure mirrors the libc hostent in a Rust idiomatic way. We should probably try to upstream the FFI part of this commit to the Nix crate at some point. Fixes #4

Gethostbyname: use proper glibc function

flokli · 2023-10-24T13:37:15Z

We needed to change this significantly, due to nix-community#4 (comment), as can also be seen in nix-community#9.

We'll open a new PR with this rebased on top of your latest main branch soon.

This was referenced Oct 20, 2022

provide host lookups #37

Closed

implement GETHOSTBY{ADDR,NAME}[v6] #40

Closed

picnoir mentioned this pull request Oct 20, 2022

Implement the GETAI operation #39

Closed

flokli mentioned this pull request Nov 20, 2022

add simple runtime configuration #51

Merged

de11n mentioned this pull request Apr 14, 2023

Nsncd: 23.05 regression compared to 22.11 NixOS/nixpkgs#218813

Open

flokli and others added 3 commits May 10, 2023 15:26

Cargo: add dns-lookup crate

23289bd

Co-Authored-By: Félix Baylac-Jacqué <felix@alternativebit.fr>

protocol: add HstResponseHeader and serialization test

a045349

Co-Authored-By: Félix Baylac-Jacqué <felix@alternativebit.fr>

src/handlers: implement serialize_host

ef805a4

Also add some error codes used in the response header. Co-Authored-By: Félix Baylac-Jacqué <felix@alternativebit.fr>

flokli force-pushed the host-lookups branch from 47e580f to bc777b2 Compare May 10, 2023 12:40

wkral mentioned this pull request May 18, 2023

NixOS: can't get machine FQDN when resolved is enabled NixOS/nixpkgs#132646

Open

flokli and others added 4 commits May 19, 2023 14:46

src/handlers.rs: handle GETHOSTBYADDR, GETHOSTBYADDRv6

ee60daa

Co-Authored-By: Félix Baylac-Jacqué <felix@alternativebit.fr>

src/handlers: implement GETHOSTBYNAME*

2e6ffe6

Co-Authored-By: Félix Baylac-Jacqué <felix@alternativebit.fr>

src/protocol: add AiResponse{Header} structs and serialization test

de9e5f0

Co-Authored-By: Florian Klink <flokli@flokli.de>

flokli force-pushed the host-lookups branch from bc777b2 to 1a4bdc3 Compare May 19, 2023 12:13

andir added 2 commits May 19, 2023 15:25

Handle IPv6-only results even in GETHOSTBYNAME

96237d8

Previously IPv6-only results would return an empty successful result message. This doesn't seem to be a valid response from the perspective of Glibc. In my case it caused Firefox to crash.

Remove useless lambda and lambda call in serialize_host

f881aac

We don't have to use a lambda to return a value from a sub-expression. Rust takes the last value in a scope as return value even if it isn't a function.

andir added 6 commits May 19, 2023 15:25

Move the host serialize code to a associated function

f0a162b

The function belongs to the data format and not to the actual processing logic. This isn't a clean cut just yet all over the codebase but moving this very fragile piece into its own function will enable us to test it in isolation.

Make the GETHOSTBYNAMEv6 handling similar to the v4 equivalent

1f386d7

This makes the result filtering similar and thus should be less confusing.

Use as_slice().to_vec() instead of manually pre-allocated buffers

c6773d6

This is as efficient as doing the manual pre-allocation as the `to_vec` method will use the length of the slice for the initial size allocation of the vector.

Make Host::serialize able to serialize empty address lists

384182e

Previously this would produce a malformed packet as far as glibc is concerned. If there are no addresses the "found" field should not be one and the other lookup fields can also be left blank.

Move common HostResponseHeader to const structs

22dda36

This moves the "well-known" wire format structs to constants that can be reused without having to duplicate their definition all the time.

Simplify the serialize_host nested Result<Some<Option<..>> handling

94f54bb

flokli force-pushed the host-lookups branch from 1a4bdc3 to 94f54bb Compare May 19, 2023 12:25

picnoir and others added 2 commits October 20, 2023 15:51

Merge pull request #9 from nix-community/nin/ghbn

77fd58d

Gethostbyname: use proper glibc function

flokli closed this Oct 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement host lookups #49

Implement host lookups #49

flokli commented Oct 20, 2022

flokli commented Oct 20, 2022

bbenne10 commented Mar 3, 2023

leifwalsh commented Mar 4, 2023

bbenne10 commented Mar 4, 2023 •

edited

Loading

leifwalsh commented Mar 5, 2023

leifwalsh commented Mar 5, 2023

flokli commented Mar 5, 2023

bbenne10 commented Mar 5, 2023 •

edited

Loading

bbenne10 commented Mar 7, 2023

flokli commented Apr 25, 2023

leifwalsh commented May 5, 2023

flokli commented May 10, 2023

codecov-commenter commented May 19, 2023

bbenne10 commented Aug 7, 2023

flokli commented Aug 20, 2023

andir commented Aug 27, 2023

flokli commented Oct 24, 2023

Implement host lookups #49

Implement host lookups #49

Conversation

flokli commented Oct 20, 2022

flokli commented Oct 20, 2022

bbenne10 commented Mar 3, 2023

leifwalsh commented Mar 4, 2023

bbenne10 commented Mar 4, 2023 • edited Loading

leifwalsh commented Mar 5, 2023

leifwalsh commented Mar 5, 2023

flokli commented Mar 5, 2023

bbenne10 commented Mar 5, 2023 • edited Loading

bbenne10 commented Mar 7, 2023

flokli commented Apr 25, 2023

leifwalsh commented May 5, 2023

flokli commented May 10, 2023

codecov-commenter commented May 19, 2023

Codecov Report

bbenne10 commented Aug 7, 2023

flokli commented Aug 20, 2023

andir commented Aug 27, 2023

flokli commented Oct 24, 2023

bbenne10 commented Mar 4, 2023 •

edited

Loading

bbenne10 commented Mar 5, 2023 •

edited

Loading