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

v4 ips in v6 database inserted incorrectly #1

Open
Kellel opened this issue Sep 6, 2024 · 3 comments
Open

v4 ips in v6 database inserted incorrectly #1

Kellel opened this issue Sep 6, 2024 · 3 comments

Comments

@Kellel
Copy link

Kellel commented Sep 6, 2024

geoip2 databases contain both v4 and v6 entries.

If we look at the C implementation https://github.com/maxmind/libmaxminddb/blob/main/src/maxminddb.c#L926-L931

We can see that the bits are transposed to the last 4 octets of a v6 address before lookup.

I believe something similar should be done here.

Currently when I write a v4 ip to a v6 database I see something like the following:

2.2.4.5/32 -> 202:405::/32

Which is wrong for multiple reasons
I believe the bits should be written like ::2:2:3:4

@Kellel
Copy link
Author

Kellel commented Sep 6, 2024

Ok I figured out a workaround:

let v6_ip = v4.addr().to_ipv6_compatible();
// add 96 to translate between v4 and v6 subnet prefix lengths
let db_ip = IpAddrWithMask::new(v6_ip, iv4.prefix_len()+96);

This translates v4 ips into compatible v6 ips and all the tooling seems to be happy with it

@pierd
Copy link
Owner

pierd commented Sep 15, 2024

Thanks for pointing this out.

According to the spec the IPv4 addresses are stored as-is (and then one can also include pointers to handle different prefix mappings).

As for the workaround, I guess the most important bit is to use the same approach on both sides (when creating the db and when doing the lookup).

@Kellel
Copy link
Author

Kellel commented Sep 18, 2024

Yeah I saw the same in the spec.
This allows me to be compatible with other pre-existing GeoIP2 database readers.
Also if we look at the official maxminddb-writer go library they do something similar
https://github.com/maxmind/mmdbwriter/blob/main/tree.go#L637-L641
https://github.com/maxmind/mmdbwriter/blob/main/tree.go#L287-L290

I find the spec hard to parse in this area, especially the bits about them including pointers to different mapped sections.

If you don't find this a bug perhaps either:
add a database option or a short block in the docs could be cool.

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

No branches or pull requests

2 participants