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

Move ZypperAuth.verify_instance to InstanceVerification engine #1149

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
71 changes: 69 additions & 2 deletions engines/instance_verification/lib/instance_verification/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ def self.update_cache(remote_ip, system_login, product_id, is_byos: false, regis
unless registry
InstanceVerification.write_cache_file(
Rails.application.config.repo_cache_dir,
[remote_ip, system_login, product_id].join('-')
InstanceVerification.build_cache_key(remote_ip, system_login, base_product_id: product_id)
)
end

InstanceVerification.write_cache_file(
Rails.application.config.registry_cache_dir,
[remote_ip, system_login].join('-')
InstanceVerification.build_cache_key(remote_ip, system_login)
)
end

Expand All @@ -22,6 +22,73 @@ def self.write_cache_file(cache_dir, cache_key)
FileUtils.touch(File.join(cache_dir, cache_key))
end

def self.verify_instance(request, logger, system)
return false unless request.headers['X-Instance-Data']

instance_data = Base64.decode64(request.headers['X-Instance-Data'].to_s)
base_product = system.products.find_by(product_type: 'base')
return false unless base_product

# check the cache for the system (20 min)
cache_path = File.join(
Rails.application.config.repo_cache_dir,
InstanceVerification.build_cache_key(request.remote_ip, system.login, base_product_id: base_product.id)
)
if File.exist?(cache_path)
# only update registry cache key
InstanceVerification.update_cache(request.remote_ip, system.login, nil, is_byos: system.proxy_byos, registry: true)
Copy link
Member

Choose a reason for hiding this comment

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

The is_byos key was removed with ccb4e6e ???

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The PR was created without the removal of BYOS changes in place

I have merged master branch into this branch so the removal of BYOS param is in place

return true
end

verification_provider = InstanceVerification.provider.new(
logger,
request,
base_product.attributes.symbolize_keys.slice(:identifier, :version, :arch, :release_type),
instance_data
)

is_valid = verification_provider.instance_valid?
# update repository and registry cache
InstanceVerification.update_cache(request.remote_ip, system.login, base_product.id, is_byos: system.proxy_byos)
is_valid
rescue InstanceVerification::Exception => e
message = ''
if system.proxy_byos
result = SccProxy.scc_check_subscription_expiration(request.headers, system.login, system.system_token, logger)
if result[:is_active]
InstanceVerification.update_cache(request.remote_ip, system.login, base_product.id, is_byos: system.proxy_byos)
return true
end

message = result[:message]
else
message = e.message
end
details = [ "System login: #{system.login}", "IP: #{request.remote_ip}" ]
details << "Instance ID: #{verification_provider.instance_id}" if verification_provider.instance_id
details << "Billing info: #{verification_provider.instance_billing_info}" if verification_provider.instance_billing_info

ZypperAuth.auth_logger.info <<~LOGMSG
Access to the repos denied: #{message}
#{details.join(', ')}
LOGMSG
false
rescue StandardError => e
logger.error('Unexpected instance verification error has occurred:')
logger.error(e.message)
logger.error("System login: #{system.login}, IP: #{request.remote_ip}")
logger.error('Backtrace:')
logger.error(e.backtrace)
false
end

def self.build_cache_key(remote_ip, login, base_product_id: nil)
Copy link
Member

Choose a reason for hiding this comment

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

This seems more complicated than it needs to be. Is there any reason something simpler couldn't be used, since there's no check on the assigned values anyway?

def self.build_cache_key(*args)
  args.compact.join('-')
end

cache_key = [remote_ip, login]
cache_key.append(base_product_id) unless base_product_id.nil?

cache_key.join('-')
end

class Engine < ::Rails::Engine
isolate_namespace InstanceVerification
config.generators.api_only = true
Expand Down
2 changes: 1 addition & 1 deletion engines/registry/lib/registry/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class Engine < ::Rails::Engine
before_action :handle_auth_cache, only: %w[index]

def handle_auth_cache
unless ZypperAuth.verify_instance(request, logger, @system)
unless InstanceVerification.verify_instance(request, logger, @system)
render(xml: { error: 'Instance verification failed' }, status: :forbidden)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
context 'without valid repository cache' do
before do
headers['X-Instance-Data'] = 'IMDS'
allow(ZypperAuth).to receive(:verify_instance).and_return(true)
allow(InstanceVerification).to receive(:verify_instance).and_return(true)
end

context 'without X-Instance-Data headers or hw_info' do
Expand All @@ -31,7 +31,7 @@
# allow(File).to receive(:exist?).with("repo/cache/127.0.0.1-#{system.login}-#{system.products.first.id}").and_return(true)
# allow(File).to receive(:exist?)
allow(InstanceVerification).to receive(:update_cache)
allow(ZypperAuth).to receive(:verify_instance).and_call_original
allow(InstanceVerification).to receive(:verify_instance).and_call_original
headers['X-Instance-Data'] = 'IMDS'
end

Expand Down
64 changes: 2 additions & 62 deletions engines/zypper_auth/lib/zypper_auth/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,66 +5,6 @@ def auth_logger
Thread.current[:logger].reopen
Thread.current[:logger]
end

def verify_instance(request, logger, system)
return false unless request.headers['X-Instance-Data']

instance_data = Base64.decode64(request.headers['X-Instance-Data'].to_s)

base_product = system.products.find_by(product_type: 'base')
return false unless base_product

# check the cache for the system (20 min)
cache_key = [request.remote_ip, system.login, base_product.id].join('-')
cache_path = File.join(Rails.application.config.repo_cache_dir, cache_key)
if File.exist?(cache_path)
# only update registry cache key
InstanceVerification.update_cache(request.remote_ip, system.login, nil, is_byos: system.proxy_byos, registry: true)
return true
end

verification_provider = InstanceVerification.provider.new(
logger,
request,
base_product.attributes.symbolize_keys.slice(:identifier, :version, :arch, :release_type),
instance_data
)

is_valid = verification_provider.instance_valid?
# update repository and registry cache
InstanceVerification.update_cache(request.remote_ip, system.login, base_product.id, is_byos: system.proxy_byos)
is_valid
rescue InstanceVerification::Exception => e
message = ''
if system.proxy_byos
result = SccProxy.scc_check_subscription_expiration(request.headers, system.login, system.system_token, logger)
if result[:is_active]
InstanceVerification.update_cache(request.remote_ip, system.login, base_product.id, is_byos: system.proxy_byos)
return true
end

message = result[:message]
else
message = e.message
end
details = [ "System login: #{system.login}", "IP: #{request.remote_ip}" ]
details << "Instance ID: #{verification_provider.instance_id}" if verification_provider.instance_id
details << "Billing info: #{verification_provider.instance_billing_info}" if verification_provider.instance_billing_info

ZypperAuth.auth_logger.info <<~LOGMSG
Access to the repos denied: #{message}
#{details.join(', ')}
LOGMSG

false
rescue StandardError => e
logger.error('Unexpected instance verification error has occurred:')
logger.error(e.message)
logger.error("System login: #{system.login}, IP: #{request.remote_ip}")
logger.error('Backtrace:')
logger.error(e.backtrace)
false
end
end

class Engine < ::Rails::Engine
Expand Down Expand Up @@ -126,7 +66,7 @@ def make_repo_url(base_url, repo_local_path, service_name = nil)
# additional validation for zypper service XML controller
before_action :verify_instance
def verify_instance
unless ZypperAuth.verify_instance(request, logger, @system)
unless InstanceVerification.verify_instance(request, logger, @system)
render(xml: { error: 'Instance verification failed' }, status: 403)
end
end
Expand All @@ -138,7 +78,7 @@ def verify_instance
# additional validation for strict_authentication auth subrequest
def path_allowed?(path)
return false unless original_path_allowed?(path)
ZypperAuth.verify_instance(request, logger, @system)
InstanceVerification.verify_instance(request, logger, @system)
end
end
end
Expand Down