credssp: improve ber parsing/peeking and add support for new messages - #121

Open

hardening wants to merge 1 commit into

future from

dft/ber_parsing

Open

credssp: improve ber parsing/peeking and add support for new messages #121
hardening wants to merge 1 commit into
future from
dft/ber_parsing

Conversation

@hardening

@hardening hardening commented Feb 9, 2021

Copy link

Copy Markdown

Contributor

No description provided.

@hardening


 credssp: improve ber parsing/peeking and add support for new messages

267c054

jonathanpoelen

jonathanpoelen reviewed

Feb 22, 2021

View reviewed changes

src/core/RDP/nla/nla_client_kerberos.hpp

Comment on lines +150 to +152

this->User.assign(user_utf16_av.data(),user_utf16_av.data()+user_utf16_av.size());

this->Domain.assign(domain_utf16_av.data(),domain_utf16_av.data()+domain_utf16_av.size());

this->Password.assign(password_utf16_av.data(),password_utf16_av.data()+password_utf16_av.size());

@jonathanpoelen jonathanpoelen Feb 22, 2021

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

This can be simplified with

this->User.assign(user_utf16_av.begin(), user_utf16_av.end());

src/core/RDP/nla/credssp.hpp

inline std::vector<uint8_t> mkOid(bytes_view oid)

{

std::vector<uint8_t> ret = mkOidHeader(oid.size());

ret << std::vector<uint8_t>(oid.data(), oid.data()+oid.size());

@jonathanpoelen jonathanpoelen Feb 22, 2021

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

<< should take a bytes_view/array_view. I also think <<= to be more explicit than <<.

But is it really better than insert() ?

ret.insert(ret.end(), oid.begin(), oid.end());

src/core/RDP/nla/credssp.hpp

backward_push_tagged_field_header(head, oid.size() + head.size(), tag);

std::reverse(head.begin(), head.end());

head << std::vector<uint8_t>(oid.begin(), oid.end());

@jonathanpoelen jonathanpoelen Feb 22, 2021

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

same

src/core/RDP/nla/credssp.hpp

Comment on lines +333 to +334

if (verbose)

LOG(LOG_ERR, "%s: Ber parse error", message);

@jonathanpoelen jonathanpoelen Feb 22, 2021

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

There is a macro LOG_IF(verbose, ....)

src/core/RDP/nla/credssp.hpp

Comment on lines +344 to +345

if (verbose)

LOG(LOG_ERR, "%s: not enough byte for length on 2 bytes", message);

@jonathanpoelen jonathanpoelen Feb 22, 2021

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

same

src/core/RDP/nla/credssp.hpp

Comment on lines +353 to +354

if (verbose)

LOG(LOG_ERR, "%s: not enough byte for length on 3 bytes", message);

@jonathanpoelen jonathanpoelen Feb 22, 2021

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

same

src/core/RDP/nla/credssp.hpp

Comment on lines +361 to +362

if (verbose)

LOG(LOG_ERR, "%s: Ber parse error, length=0x%lx", message, length);

@jonathanpoelen jonathanpoelen Feb 22, 2021

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

same

src/core/RDP/nla/credssp.hpp

Comment on lines +368 to +370

if (verbose)

LOG(LOG_ERR, "%s: Ber Not enough data for length on %lu bytes(need=%lu have=%lu)",

message, extraSize, extraSize + length, s.size());

@jonathanpoelen jonathanpoelen Feb 22, 2021

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

same

src/core/RDP/nla/credssp.hpp

Comment on lines +417 to +418

if (verbose)

LOG(LOG_ERR, "%s: Ber unexpected tag", message);

@jonathanpoelen jonathanpoelen Feb 22, 2021

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

same

src/core/RDP/nla/credssp.hpp

Comment on lines +564 to +565

BerOID ret = s.in_skip_bytes(bytes).as<std::vector<uint8_t>>();

return ret;

@jonathanpoelen jonathanpoelen Feb 22, 2021

Copy link

Copy Markdown

Contributor

Choose a reason for hiding this comment

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

return s.in_skip_bytes(bytes).as<BerOID>();

Labels

None yet

2 participants

@hardening @jonathanpoelen

Uh oh!

Conversation

@hardening hardening commented Feb 9, 2021

Uh oh!

@jonathanpoelen jonathanpoelen Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

@jonathanpoelen jonathanpoelen Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

@jonathanpoelen jonathanpoelen Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

@jonathanpoelen jonathanpoelen Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

@jonathanpoelen jonathanpoelen Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

@jonathanpoelen jonathanpoelen Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

@jonathanpoelen jonathanpoelen Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

@jonathanpoelen jonathanpoelen Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

@jonathanpoelen jonathanpoelen Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

@jonathanpoelen jonathanpoelen Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants