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.

Sign up for GitHub

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

Fix for issue #1074 #1132

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix for issue #1074 #1132

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c7af70d
Initial fix for #1074
colin-stubbs Feb 2, 2023
aabbcac
Add CloudFlare public DNS server IP list
colin-stubbs Feb 2, 2023
022f8f9
Add Google public DNS IP list
colin-stubbs Feb 2, 2023
1878883
Correct logic fail when testing RFC-5737 addresses
colin-stubbs Feb 2, 2023
b5d8c1a
Replace spaces with tabs
colin-stubbs Feb 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions internal/fields/_static/allowed_geo_ips.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,23 @@
89.160.20.128/25
67.43.156.0/24
2a02:cf40::/29
1.1.1.1/32
1.1.1.2/32
1.1.1.3/32
1.0.0.1/32
1.0.0.2/32
1.0.0.3/32
2606:4700:4700::1111/128
2606:4700:4700::1112/128
2606:4700:4700::1113/128
2606:4700:4700::1001/128
2606:4700:4700::1002/128
2606:4700:4700::1003/128
2606:4700:4700::64/128
2606:4700:4700::6400/128
8.8.8.8/32
8.8.4.4/32
2001:4860:4860::8888/128
2001:4860:4860::8844/128
2001:4860:4860::64/128
2001:4860:4860::6464/128
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these IPs included in the current GeoIP database? At least some of them are not related to documentation.

The idea of this list is to have a reference of IP ranges that are included in the GeoIP database, so package developers can know when GeoIP enrichment is working.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, from: https://www.maxmind.com/en/geoip-demo

IP Address Country Code Location Network Postal Code Approximate Coordinates* Accuracy Radius (km) ISP Organization Domain Metro Code
2001:4860:4860::8888 US United States,North America 2001:4860:4860::/46   37.751,-97.822 100 Google Google    
2001:4860:4860::8844 US United States,North America 2001:4860:4860::/46   37.751,-97.822 100 Google Google    
2001:4860:4860::64 US United States,North America 2001:4860:4860::/46   37.751,-97.822 100 Google Google    
2001:4860:4860::6464 US United States,North America 2001:4860:4860::/46   37.751,-97.822 100 Google Google    

20 changes: 20 additions & 0 deletions internal/fields/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,25 @@ func initializeAllowedCIDRsList() (cidrs []*net.IPNet) {
return cidrs
}

// IsDocumentation reports whether ip is a reserved address for documentation,
// according to RFC 5737 (IPv4 Address Blocks Reserved for Documentation) and
// RFC 3849 (IPv6 Address Prefix Reserved for Documentation).
func IsDocumentation(ip net.IP) (bool) {
if ip4 := ip.To4(); ip4 != nil {
// Following RFC 5737, Section 3. Documentation Address Blocks which says:
// The blocks 192.0.2.0/24 (TEST-NET-1), 198.51.100.0/24 (TEST-NET-2),
// and 203.0.113.0/24 (TEST-NET-3) are provided for use in
// documentation.
return ((ip4[0] == 192 && ip4[1] == 0 && ip4[2] == 2) ||
(ip4[0] == 198 && ip4[1] == 51 && ip4[2] == 100) ||
(ip4[0] == 203 && ip4[1] == 0 && ip4[2] == 113))
}
// Following RFC 3849, Section 2. Documentation IPv6 Address Prefix which
// says:
// The prefix allocated for documentation purposes is 2001:DB8::/32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation looks wrong here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can run make check-static for linting.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make build / make works fine... I'm already using a locally build version of elastic-agent

make lint reports no errors.

make check-static is whinging but unhelpful...

user@box elastic-package % make check-static
go build -ldflags "-X github.com/elastic/elastic-package/internal/version.CommitHash=`git describe --always --long --dirty` -X github.com/elastic/elastic-package/internal/version.BuildTime=`date +%s` -X github.com/elastic/elastic-package/internal/version.Tag=`(git describe --exact-match --tags 2>/dev/null || echo '') | tr -d '\n'`" -o elastic-package
go run golang.org/x/tools/cmd/goimports -local github.com/elastic/elastic-package/ -w .
go run honnef.co/go/tools/cmd/staticcheck ./...
go run github.com/elastic/go-licenser -license Elastic
go mod tidy
cd tools/readme; go run main.go
2023/02/03 12:23:54 generating command doc for benchmark...
2023/02/03 12:23:54 generating command doc for build...
2023/02/03 12:23:54 generating command doc for changelog...
2023/02/03 12:23:54 generating command doc for check...
2023/02/03 12:23:54 generating command doc for clean...
2023/02/03 12:23:54 generating command doc for create...
2023/02/03 12:23:54 generating command doc for dump...
2023/02/03 12:23:54 generating command doc for export...
2023/02/03 12:23:54 generating command doc for format...
2023/02/03 12:23:54 generating command doc for install...
2023/02/03 12:23:54 generating command doc for lint...
2023/02/03 12:23:54 generating command doc for profiles...
2023/02/03 12:23:54 generating command doc for promote...
2023/02/03 12:23:54 generating command doc for publish...
2023/02/03 12:23:54 generating command doc for report...
2023/02/03 12:23:54 generating command doc for service...
2023/02/03 12:23:54 generating command doc for stack...
2023/02/03 12:23:54 generating command doc for status [package]...
2023/02/03 12:23:54 generating command doc for test...
2023/02/03 12:23:54 generating command doc for uninstall...
2023/02/03 12:23:54 generating command doc for version...
README.md successfully written
git update-index --really-refresh
internal/fields/validate.go: needs update
make: *** [check-git-clean] Error 1
user@box elastic-package % make build
go build -ldflags "-X github.com/elastic/elastic-package/internal/version.CommitHash=`git describe --always --long --dirty` -X github.com/elastic/elastic-package/internal/version.BuildTime=`date +%s` -X github.com/elastic/elastic-package/internal/version.Tag=`(git describe --exact-match --tags 2>/dev/null || echo '') | tr -d '\n'`" -o elastic-package
user@box elastic-package %

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation looks wrong here.

Ewww, tabs in use... I've replaced the spaces and pushed another commit.

return len(ip) == net.IPv6len && ip[0] == 32 && ip[1] == 1 && ip[2] == 13 && ip[3] == 184
}

func loadFieldsFromDir(fieldsDir string) ([]FieldDefinition, error) {
files, err := filepath.Glob(filepath.Join(fieldsDir, "*.yml"))
if err != nil {
Expand Down Expand Up @@ -648,6 +667,7 @@ func (v *Validator) isAllowedIPValue(s string) bool {

if ip.IsUnspecified() ||
ip.IsPrivate() ||
IsDocumentation(ip) ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am ok with allowing documentation IPs for this purpose, but I would prefer if apart of allowing them, at least any of them are also included in the GeoIP database, so package developers can use them to check that geoip enrichment works.

Copy link
Author

@colin-stubbs colin-stubbs Feb 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsoriano not possible, the whole point point of documentation addresses is that they are used in place of real addresses in documentation, the test events and sample events that wind up in the integration are a form of documentation and should be permitted. There is no GeoIP data to be associated with them.

For anyone who is generating test events in a lab environment as part of Elastic integration testing, they can/should and likely will use the RFC addresses allocated for this purpose.

They likely will not, and should not, take say... 89.160.20.128/25 - which is a real IP block belonging to someone in Swedend, and use that in their lab environment to generate test events.

Summary: allowing the documentation IP's is important to support test events generated in a lab environment. It's not to support GeoIP lookups in any way.

The documentation IP's are complementary to RFC-1918 private addresses and should be permitted.

ip.IsLoopback() ||
ip.IsLinkLocalUnicast() ||
ip.IsLinkLocalMulticast() ||
Expand Down