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

Implement the GETAI operation #39

Closed
wants to merge 1 commit into from

Conversation

picnoir
Copy link
Contributor

@picnoir picnoir commented Oct 11, 2022

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.

The Nix crate we rely on for the glibc FFI bindings does not wrap getaddrinfo yet. We had to pull the dns-lookup package to get those bindings.

Part of #37.

Note: on top of the automated tests, this PR has also been manually tested via a test VM.

src/handlers.rs Outdated Show resolved Hide resolved
src/handlers.rs Outdated Show resolved Hide resolved
src/handlers.rs Outdated Show resolved Hide resolved
src/handlers.rs Outdated Show resolved Hide resolved
src/handlers.rs Outdated Show resolved Hide resolved
src/handlers.rs Outdated Show resolved Hide resolved
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.

The Nix crate we rely on for the glibc FFI bindings does not wrap
getaddrinfo yet. We had to pull the `dns-lookup` package to get those
bindings.
@leifwalsh
Copy link
Collaborator

@NinjaTrappeur can you describe your manual test? I wonder if that's something we can get working in Actions.

@flokli
Copy link
Contributor

flokli commented Oct 15, 2022

can you describe your manual test? I wonder if that's something we can get working in Actions.

We use a NixOS VM test, using a custom version of getent. This test makes use of the fact that external glibc modules only work via nscd protocol on NixOS, and uses the nss-systemd module too. I'm not sure this will be something trivial to port to a container.

We could add the VM test code to this repository, and tweak it so that it uses the nsncd code from here, but running it in GH actions might not work (nested virt).

See https://github.com/flokli/nixpkgs/blob/e26519527879774b6a2551b1704598039cdbcb6e/nixos/tests/nscd.nix for the test code.

@geofft
Copy link
Collaborator

geofft commented Oct 15, 2022

On the actions front - you can reproduce the issue with Nix (nixpkgs) on a non-NixOS OS (in fact that's our use case). There is a host-wide libc, but Nix-built software doesn't use it. So I don't think we need NixOS specifically; we can just install Nix inside the actions VM running Ubuntu (or whatever) and make sure a getent call from nixpkgs.glibc works. That's probably worth doing anyway, since that's a more end-to-end test than our current one (e.g., it will catch if the NSCD protocol changes in some unfortunate way). I can give that a shot.

@geofft
Copy link
Collaborator

geofft commented Oct 16, 2022

Both this and #40 seem reasonable.

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.

I'd like to add an end-to-end test and we should sort out the CLA stuff (#44) but I'm actually curious to run this internally on my dev box (i.e. without yet writing config to let you disable this) and see how it holds up. I also want to stare a little bit harder at some of the _nss_gethostbyname4 stuff that isn't super well-documented to make sure we're indeed calling the right NSS functions, but I think this code does (We can test that in the end-to-end test.)

Thank you for the contributions!

@flokli flokli mentioned this pull request Oct 20, 2022
@picnoir
Copy link
Contributor Author

picnoir commented Oct 20, 2022

Superseded by #49

@picnoir picnoir closed this Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants