Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ KEYGEN_HOST=
# Set to -1 to disable pruning.
# KEYGEN_PRUNE_WEBHOOK_BACKLOG_DAYS=30

# A comma-separated list of CIDR ranges that webhook and other configurable
# URLs are allowed to resolve to. By default, private, loopback, link-local,
# and other non-public addresses are rejected to prevent server-side request
# forgery (SSRF). Leave empty to keep that secure default. To permit specific
# internal ranges, add them explicitly, e.g. an internal service subnet like
# 10.0.0.0/24 (prefer the narrowest range possible), or use 0.0.0.0/0,::/0 to
# allow all addresses. Only use this in trusted, self-hosted environments where
# pointing webhooks at internal services is intended.
# KEYGEN_ALLOWED_PRIVATE_ADDRESSES=

# The number of rows pruned per-batch during pruning.
# KEYGEN_PRUNE_BATCH_SIZE=10000

Expand Down
22 changes: 20 additions & 2 deletions app/validators/url_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def validate_each(record, attribute, value)

record.errors.add attribute, :protocol_invalid, message: "must be a valid URL using one of the following protocols: #{protocols.join(", ")}" unless valid_protocol?(uri)
record.errors.add attribute, :host_invalid, message: 'must be a URL with a valid host' unless valid_host?(uri)
record.errors.add attribute, :host_private, message: 'must not resolve to a private address' unless public_host?(uri)
record.errors.add attribute, :address_invalid, message: 'must resolve to a valid address' unless valid_address?(uri)
rescue URI::InvalidURIError,
URI::InvalidComponentError,
URI::BadURIError
Expand Down Expand Up @@ -73,13 +73,19 @@ def blacklisted_host?(host)
end
end

def public_host?(uri)
def valid_address?(uri)
addrs = Resolv.getaddresses(uri.host)
return false if
addrs.empty?

addrs.all? do |addr|
ip = IPAddr.new(addr)

# NB(ezekg) self-hosted deployments may legitimately point webhooks at private or
# internal addresses, so allow explicitly permitted ranges.
next true if
allowed_private_address?(ip)

next false if
ip.loopback? || ip.private? || ip.link_local? ||
ip.ipv4_mapped? || ip.ipv4_compat?
Expand All @@ -98,6 +104,18 @@ def public_host?(uri)
false
end

def allowed_private_address?(ip) = allowed_private_ranges.any? { it.include?(ip) }
def allowed_private_ranges
return [] unless
ENV.key?('KEYGEN_ALLOWED_PRIVATE_ADDRESSES')

# NB(ezekg) a comma-separated CIDR allowlist of otherwise-private ranges to permit,
# e.g. an internal service subnet for webhooks.
ENV.fetch('KEYGEN_ALLOWED_PRIVATE_ADDRESSES') { '' }
.split(',')
.filter_map { IPAddr.new(it.strip) unless it.strip.empty? }
end

def blacklisted_ipv4?(ip) = BLACKLISTED_IPV4.any? { it.include?(ip) }
def blacklisted_ipv6?(ip) = BLACKLISTED_IPV6.any? { it.include?(ip) }
def embedded_ipv4?(ip) = NAT64_PREFIXES.any? { it.include?(ip) }
Expand Down
98 changes: 96 additions & 2 deletions spec/validators/url_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,11 @@ def self.it_resolves_to(*addrs, valid:)
else
it { is_expected.to_not be_valid }

it 'should add a host_private error' do
it 'should add an invalid address error' do
subject.validate

expect(subject.errors.details[:url]).to include(
hash_including(error: :host_private),
hash_including(error: :address_invalid),
)
end
end
Expand Down Expand Up @@ -205,4 +205,98 @@ def self.it_resolves_to(*addrs, valid:)

it { is_expected.to_not be_valid }
end

context 'when all addresses are allowed' do
with_env KEYGEN_ALLOWED_PRIVATE_ADDRESSES: '0.0.0.0/0, ::/0' do
context 'with a host resolving to a private address' do
let(:url) { 'https://webhooks.example' }

before { stub_resolv!('webhooks.example', '10.0.0.1') }

it { is_expected.to be_valid }
end

context 'with a host resolving to a link-local address' do
let(:url) { 'https://webhooks.example' }

before { stub_resolv!('webhooks.example', '169.254.169.254') }

it { is_expected.to be_valid }
end

context 'with an unresolvable host' do
let(:url) { 'https://webhooks.example' }

before { stub_resolv!('webhooks.example') }

it { is_expected.to_not be_valid }
end

context 'with a blacklisted host' do
let(:url) { 'https://api.keygen.sh' }

it { is_expected.to_not be_valid }
end

context 'with an invalid protocol' do
let(:url) { 'ftp://ftp.example' }

it { is_expected.to_not be_valid }
end
end
end

context 'when private addresses are restricted to an allowlist' do
with_env KEYGEN_ALLOWED_PRIVATE_ADDRESSES: '10.0.0.0/8, fd00::/8' do
context 'with a host resolving to an allowed private range' do
let(:url) { 'https://webhooks.example' }

before { stub_resolv!('webhooks.example', '10.1.2.3') }

it { is_expected.to be_valid }
end

context 'with a host resolving to an allowed private IPv6 range' do
let(:url) { 'https://webhooks.example' }

before { stub_resolv!('webhooks.example', 'fd00::1') }

it { is_expected.to be_valid }
end

context 'with a host resolving to a private range outside the allowlist' do
let(:url) { 'https://webhooks.example' }

before { stub_resolv!('webhooks.example', '192.168.1.1') }

it { is_expected.to_not be_valid }
end

context 'with a host resolving to a public address' do
let(:url) { 'https://webhooks.example' }

before { stub_resolv!('webhooks.example', '93.184.215.14') }

it { is_expected.to be_valid }
end

context 'with a mix of allowed and disallowed private addresses' do
let(:url) { 'https://webhooks.example' }

before { stub_resolv!('webhooks.example', '10.1.2.3', '192.168.1.1') }

it { is_expected.to_not be_valid }
end
end

context 'with a malformed allowlist entry' do
with_env KEYGEN_ALLOWED_PRIVATE_ADDRESSES: 'not-a-cidr' do
let(:url) { 'https://webhooks.example' }

before { stub_resolv!('webhooks.example', '93.184.215.14') }

it { is_expected.to_not be_valid }
end
end
end
end
Loading