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

Fix NTLMv2 hash when username contains non-ASCII characters #56

Merged
merged 4 commits into from
Jun 12, 2024
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
13 changes: 12 additions & 1 deletion lib/net/ntlm.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,18 @@ def ntlmv2_hash(user, password, target, opt={})
else
ntlmhash = ntlm_hash(password, opt)
end
userdomain = user.upcase + target

if opt[:unicode]
# Uppercase operation on username containing non-ASCI characters
# after behing unicode encoded with `EncodeUtil.encode_utf16le`
# doesn't play well. Upcase should be done before encoding.
user_upcase = EncodeUtil.decode_utf16le(user).upcase
user_upcase = EncodeUtil.encode_utf16le(user_upcase)
else
user_upcase = user.upcase
end
userdomain = user_upcase + target

unless opt[:unicode]
userdomain = EncodeUtil.encode_utf16le(userdomain)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/net/ntlm/encode_util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def self.decode_utf16le(str)
# the function will convert the string bytes to UTF-16LE and note the encoding as UTF-8 so that byte
# concatination works seamlessly.
def self.encode_utf16le(str)
str.dup.force_encoding('UTF-8').encode(Encoding::UTF_16LE, Encoding::UTF_8).force_encoding('UTF-8')
str.dup.force_encoding('UTF-8').encode(Encoding::UTF_16LE, Encoding::UTF_8).force_encoding('ASCII-8BIT')
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this change makes sense and behaves more like the comment block describes, since its supposed to be safe to perform byte concatenation on the result

end
end
end
Expand Down
23 changes: 22 additions & 1 deletion spec/lib/net/ntlm/message/type3_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,28 @@

end

describe '.serialize' do
describe '#serialize' do
context 'when the username contains non-ASCI characters' do
let(:t3) {
t2 = Net::NTLM::Message::Type2.new
t2.response(
{
:user => 'Hélène',
:password => '123456',
:domain => ''
},
{
:ntlmv2 => true,
:workstation => 'testlab.local'
}
)
}

it 'serializes without error' do
expect { t3.serialize }.not_to raise_error
end
end

subject(:message) { described_class.create(opts) }
context 'with the UNICODE flag set' do
let(:opts) { {lm_response: "\x00".b, ntlm_response: '', domain: '', workstation: '', user: '', flag: Net::NTLM::DEFAULT_FLAGS[:TYPE3] | Net::NTLM::FLAGS[:UNICODE] } }
Expand Down
8 changes: 8 additions & 0 deletions spec/lib/net/ntlm_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@
end
end

context 'when the username contains non-ASCI characters' do
let(:user) { 'юзер' }

it 'should return the correct ntlmv2 hash' do
expect(Net::NTLM::ntlmv2_hash(user, passwd, domain, { unicode: true })).to eq(["a0f4b914a37faeaee884b6b04a20faf0"].pack("H*"))
end
end

it 'should generate an lm_response' do
expect(Net::NTLM::lm_response(
{
Expand Down