diff --git a/lib/mongo/error/invalid_uri.rb b/lib/mongo/error/invalid_uri.rb index 860ed2569e..ce1943ae6e 100644 --- a/lib/mongo/error/invalid_uri.rb +++ b/lib/mongo/error/invalid_uri.rb @@ -23,12 +23,16 @@ class Error class InvalidURI < Error # Instantiate the new exception. # + # The URI is redacted via {Mongo::URI.redact} before being interpolated + # so that any cleartext credentials in the original input do not end up + # in logs, error reporters, or backtraces. + # # @example Instantiate the exception. # Mongo::Error::InvalidURI.new(uri, details, format) # # @since 2.0.0 def initialize(uri, details, format = nil) - message = "Bad URI: #{uri}\n" + + message = "Bad URI: #{Mongo::URI.redact(uri)}\n" + "#{details}\n" message += "MongoDB URI must be in the following format: #{format}\n" if format message += "Please see the following URL for more information: #{Mongo::URI::HELP}\n" diff --git a/lib/mongo/uri.rb b/lib/mongo/uri.rb index 0f959cd0fd..f17f1acc16 100644 --- a/lib/mongo/uri.rb +++ b/lib/mongo/uri.rb @@ -141,6 +141,19 @@ class URI # @since 2.5.0 SCHEME_DELIM = '://' + # Placeholder used in place of cleartext credentials when a URI is + # rendered for display, logging, or error reporting. + CREDENTIALS_PLACEHOLDER = '' + + # Pattern matching the userinfo portion of a MongoDB connection string. + # Anchors at the start and is bounded to the authority component (stops at + # the first '/', '?', or '#'), but matches greedily up to the last '@' in + # that component so passwords containing an unescaped '@' are still fully + # redacted. Case-insensitive so an unusual scheme like 'MongoDB://' is + # redacted too — the parser will reject it, and the redactor must not be + # the thing that leaks the credentials in the resulting error. + USERINFO_REDACTION_REGEX = %r{\A(mongodb(?:\+srv)?://)[^/?#]*@}i.freeze + # Error details for an invalid options format. # # @since 2.1.0 @@ -215,6 +228,24 @@ class URI # @since 2.1.0 REPEATABLE_OPTIONS = %i[tag_sets ssl] + # Replace the userinfo portion of a MongoDB connection string with a + # placeholder so the result can safely be logged, displayed, or embedded + # in an exception message. + # + # The input is matched as a string, not parsed, so this is safe to call + # on malformed URIs (which is exactly when {InvalidURI} is raised). + # + # @param [ String ] string The raw URI string. + # + # @return [ String ] The URI with any userinfo replaced by + # {CREDENTIALS_PLACEHOLDER}, or the input unchanged if it is not a + # string or has no userinfo. + def self.redact(string) + return string unless string.is_a?(String) + + string.sub(USERINFO_REDACTION_REGEX, "\\1#{CREDENTIALS_PLACEHOLDER}@") + end + # Get either a URI object or a SRVProtocol URI object. # # @example Get the uri object. @@ -320,20 +351,33 @@ def database @database || Database::ADMIN end - # Get the uri as a string. + # Get the uri as a string with any credentials redacted. + # + # Credentials are replaced with {CREDENTIALS_PLACEHOLDER} so the result is + # safe to log or display. Use {#credentials} to recover the original user + # and password. # # @example Get the uri as a string. # uri.to_s # - # @return [ String ] The uri string. + # @return [ String ] The redacted uri string. def to_s reconstruct_uri end + # Return a redacted, human-readable representation of the URI. The + # default {Object#inspect} would dump {@string} and {@password} as + # instance variables, leaking credentials. + # + # @return [ String ] The redacted inspect string. + def inspect + "#<#{self.class.name}: #{reconstruct_uri}>" + end + private - # Reconstruct the URI from its parts. Invalid options are dropped and options - # are converted to camelCase. + # Reconstruct the URI from its parts with credentials redacted. Invalid + # options are dropped and options are converted to camelCase. # # @return [ String ] the uri. def reconstruct_uri @@ -349,9 +393,7 @@ def reconstruct_uri end.compact.join('&') uri = "#{scheme}#{SCHEME_DELIM}" - uri += @user.to_s if @user - uri += "#{AUTH_USER_PWD_DELIM}#{@password}" if @password - uri += '@' if @user || @password + uri += "#{CREDENTIALS_PLACEHOLDER}@" if @user || @password uri += @query_hostname || servers uri += '/' if @database || !options.empty? uri += @database.to_s if @database diff --git a/spec/mongo/error/invalid_uri_spec.rb b/spec/mongo/error/invalid_uri_spec.rb new file mode 100644 index 0000000000..78cfbffa61 --- /dev/null +++ b/spec/mongo/error/invalid_uri_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'lite_spec_helper' + +describe Mongo::Error::InvalidURI do + describe '#initialize' do + let(:details) { 'Invalid port' } + + context 'when the uri has cleartext credentials' do + let(:uri) { 'mongodb://alice:s3cret@host:bad-port/admin' } + + let(:error) { described_class.new(uri, details) } + + it 'does not include the password in the message' do + expect(error.message).not_to include('s3cret') + end + + it 'does not include the username in the message' do + expect(error.message).not_to include('alice') + end + + it 'replaces the userinfo with the credentials placeholder' do + expect(error.message).to include('mongodb://@host:bad-port/admin') + end + + it 'still includes the supplied details' do + expect(error.message).to include(details) + end + end + + context 'when the uri is a mongodb+srv URI with credentials' do + let(:uri) { 'mongodb+srv://alice:s3cret@cluster.example.com' } + + let(:error) { described_class.new(uri, details) } + + it 'does not include the password' do + expect(error.message).not_to include('s3cret') + end + + it 'redacts the userinfo' do + expect(error.message).to include('mongodb+srv://@cluster.example.com') + end + end + + context 'when the uri has no credentials' do + let(:uri) { 'mongodb://host:27017' } + + it 'does not alter the uri' do + error = described_class.new(uri, details) + expect(error.message).to include(uri) + end + end + + context 'when the uri is nil' do + it 'does not raise when constructing the message' do + expect { described_class.new(nil, details) }.not_to raise_error + end + end + end +end diff --git a/spec/mongo/uri/srv_protocol_spec.rb b/spec/mongo/uri/srv_protocol_spec.rb index 8615642a6c..67244978ad 100644 --- a/spec/mongo/uri/srv_protocol_spec.rb +++ b/spec/mongo/uri/srv_protocol_spec.rb @@ -17,7 +17,8 @@ shared_examples 'roundtrips string' do it 'returns the correct string for the uri' do - expect(uri.to_s).to eq(URI::DEFAULT_PARSER.unescape(string)) + expected = Mongo::URI.redact(URI::DEFAULT_PARSER.unescape(string)) + expect(uri.to_s).to eq(expected) end end @@ -352,8 +353,8 @@ expect(uri.credentials[:user]).to eq(user) end - it 'drops the colon in to_s' do - expect(uri.to_s).to eq('mongodb+srv://tyler@test5.test.build.10gen.cc') + it 'redacts the credentials in to_s' do + expect(uri.to_s).to eq('mongodb+srv://@test5.test.build.10gen.cc') end end @@ -734,8 +735,8 @@ expect(client.options[:auth_mech]).to eq(expected) end - it 'roundtrips the string' do - expect(uri.to_s).to eq('mongodb+srv://tyler:s3kr4t@test5.test.build.10gen.cc/?authSource=$external&authMechanism=GSSAPI') + it 'roundtrips the string with credentials redacted' do + expect(uri.to_s).to eq('mongodb+srv://@test5.test.build.10gen.cc/?authSource=$external&authMechanism=GSSAPI') end end @@ -778,8 +779,8 @@ expect(client.options[:auth_mech]).to eq(expected) end - it 'roundtrips the string' do - expect(uri.to_s).to eq('mongodb+srv://tyler@test5.test.build.10gen.cc/?authSource=$external&authMechanism=MONGODB-X509') + it 'roundtrips the string with credentials redacted' do + expect(uri.to_s).to eq('mongodb+srv://@test5.test.build.10gen.cc/?authSource=$external&authMechanism=MONGODB-X509') end context 'when a username is not provided' do diff --git a/spec/mongo/uri_spec.rb b/spec/mongo/uri_spec.rb index ef6dcba290..42dc9c8e12 100644 --- a/spec/mongo/uri_spec.rb +++ b/spec/mongo/uri_spec.rb @@ -5,7 +5,8 @@ describe Mongo::URI do shared_examples 'roundtrips string' do it 'returns the correct string for the uri' do - expect(uri.to_s).to eq(URI::DEFAULT_PARSER.unescape(string)) + expected = Mongo::URI.redact(URI::DEFAULT_PARSER.unescape(string)) + expect(uri.to_s).to eq(expected) end end @@ -325,14 +326,94 @@ end end + describe '.redact' do + it 'returns nil unchanged' do + expect(described_class.redact(nil)).to be_nil + end + + it 'leaves a uri without userinfo unchanged' do + expect(described_class.redact('mongodb://localhost:27017')) + .to eq('mongodb://localhost:27017') + end + + it 'replaces user and password with the placeholder' do + expect(described_class.redact('mongodb://alice:s3cret@host:27017/admin')) + .to eq('mongodb://@host:27017/admin') + end + + it 'redacts userinfo for mongodb+srv:// URIs' do + expect(described_class.redact('mongodb+srv://alice:s3cret@host')) + .to eq('mongodb+srv://@host') + end + + it 'redacts user-only userinfo' do + expect(described_class.redact('mongodb://alice@host')) + .to eq('mongodb://@host') + end + + it 'does not redact an @ that appears past the authority component' do + expect(described_class.redact('mongodb://host/db?opt=a@b')) + .to eq('mongodb://host/db?opt=a@b') + end + + it 'redacts a password containing an unescaped @' do + expect(described_class.redact('mongodb://alice:p@ss@host')) + .to eq('mongodb://@host') + end + + it 'redacts when the scheme uses mixed case' do + expect(described_class.redact('MongoDB://alice:s3cret@host')) + .to eq('MongoDB://@host') + end + + it 'does not redact strings with an unrelated scheme' do + expect(described_class.redact('http://alice:s3cret@host')) + .to eq('http://alice:s3cret@host') + end + end + describe '#to_s' do - context 'string is a uri' do + context 'string is a uri without credentials' do let(:string) { 'mongodb://localhost:27017' } it 'returns the original string' do expect(uri.to_s).to eq(string) end end + + context 'string includes credentials' do + let(:string) { 'mongodb://alice:s3cret@localhost:27017' } + + it 'replaces the credentials with a placeholder' do + expect(uri.to_s).to eq('mongodb://@localhost:27017') + end + + it 'does not include the password' do + expect(uri.to_s).not_to include('s3cret') + end + + it 'does not include the username' do + expect(uri.to_s).not_to include('alice') + end + end + end + + describe '#inspect' do + context 'string includes credentials' do + let(:string) { 'mongodb://alice:s3cret@localhost:27017' } + + it 'does not include the password' do + expect(uri.inspect).not_to include('s3cret') + end + + it 'does not include the username' do + expect(uri.inspect).not_to include('alice') + end + + it 'includes the credentials placeholder' do + expect(uri.inspect).to include('') + end + end end describe '#servers' do @@ -448,8 +529,8 @@ expect(uri.credentials[:user]).to eq(user) end - it 'roundtrips string without the colon' do - expect(uri.to_s).to eq('mongodb://tyler@localhost') + it 'redacts the credentials in to_s' do + expect(uri.to_s).to eq('mongodb://@localhost') end end @@ -895,8 +976,8 @@ expect(client.options[:auth_mech_properties]).to eq({ 'service_name' => 'mongodb' }) end - it 'roundtrips the string' do - expect(uri.to_s).to eq('mongodb://tyler:s3kr4t@localhost/?authMechanism=GSSAPI') + it 'roundtrips the string with credentials redacted' do + expect(uri.to_s).to eq('mongodb://@localhost/?authMechanism=GSSAPI') end end @@ -909,8 +990,8 @@ expect(client.options[:auth_mech_properties]).to eq({ 'service_name' => 'foo' }) end - it 'roundtrips the string' do - expect(uri.to_s).to eq('mongodb://tyler:s3kr4t@localhost/?authMechanism=GSSAPI&authMechanismProperties=SERVICE_NAME:foo') + it 'roundtrips the string with credentials redacted' do + expect(uri.to_s).to eq('mongodb://@localhost/?authMechanism=GSSAPI&authMechanismProperties=SERVICE_NAME:foo') end end end