Add a buffer reader/writer for NTLM.

net/http/ntlm_buffer_reader.h

+// Copyright (c) 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef NET_BASE_NTLM_BUFFER_READER_H_
+#define NET_BASE_NTLM_BUFFER_READER_H_
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include <string>
+
+#include "base/strings/string_piece.h"
+#include "net/base/net_export.h"
+#include "net/http/ntlm.h"
+
+namespace net {
+
+// Supports various bounds-checked low level buffer operations required by an
+// NTLM implementation.
+//
+// The class supports sequential read of a provided buffer. All reads perform
+// bounds checking to ensure enough space is remaining in the buffer.
+//
+//

[Ryan Sleevi, 2017/06/08 18:36:34]

Delete newline?

[zentaro, 2017/06/12 23:12:06]

Done.

+// Read* methods read from the buffer at the current cursor position and
+// perform any necessary type conversion and provide the data in out params.
+// After a successful read the cursor position is advanced past the read
+// field.
+//
+// Failed reads leave the internal cursor at the same position as before the
+// call.
+//
+//

[Ryan Sleevi, 2017/06/08 18:36:33]

Delete newline?

[zentaro, 2017/06/12 23:12:06]

Done.

+// Read*Payload methods first read a security buffer see |ReadSecurityBuffer|

[Ryan Sleevi, 2017/06/08 18:36:34]

nit:
s/see |ReadSecurityBuffer|/(see ReadSecurityBuffer())|

That is, grammatically, it doesn't flow - so we should either integrate into the
sentence, such as:

Read*Payload methods first read a security buffer, using ReadSecurityBuffer(),
and then read the requested payload from ...

[zentaro, 2017/06/12 23:12:06]

Done.

+// then read the requested payload from the offset and length stated in the
+// security buffer.
+//
+// If the length and offset in the security buffer would cause a read outside
+// the message buffer the payload will not be read and the function will
+// return false.
+//
+// The cursor will remain as it was before the call as if the security buffer
+// had not been read.

[Ryan Sleevi, 2017/06/08 18:36:34]

From looking at your documentatino for Read*Payload methods, they seem to
adequately cover this - I think you can delete this block?

[zentaro, 2017/06/12 23:12:05]

Done.

+//

[Ryan Sleevi, 2017/06/08 18:36:33]

Delete newline?

[zentaro, 2017/06/12 23:12:05]

Done.

+//
+// Skip* methods skip the cursor over that same number of bytes that the
+// equivalent Read method would without reading or returning the values.

[Ryan Sleevi, 2017/06/08 18:36:33]

Deletable? :)

[zentaro, 2017/06/12 23:12:06]

Done.

+//
+//

[Ryan Sleevi, 2017/06/08 18:36:33]

Delete newline?

[zentaro, 2017/06/12 23:12:06]

Done.

+// Match* methods are used to validate fields in the buffer that should have
+// expected values, such as signatures or message identifiers.

[Ryan Sleevi, 2017/06/08 18:36:34]

Deletable?

[zentaro, 2017/06/12 23:12:06]

Done.

+//
+//

[Ryan Sleevi, 2017/06/08 18:36:33]

Delete newline?

[zentaro, 2017/06/12 23:12:05]

Done.

+// Based on [MS-NLMP]: NT LAN Manager (NTLM) Authentication Protocol
+// Specification version 28.0 [1]
+//
+// [1] https://msdn.microsoft.com/en-us/library/cc236621.aspx
+class NET_EXPORT_PRIVATE NtlmBufferReader {
+ public:
+  explicit NtlmBufferReader(base::StringPiece buffer);

[asanka, 2017/06/08 18:44:58]

Obvious, but add a short note here and below that the reader shouldn't outlive
the buffer.

[zentaro, 2017/06/12 23:12:05]

Done.

+  NtlmBufferReader(const uint8_t* ptr, size_t len);
+  ~NtlmBufferReader();
+
+  size_t GetLength() const { return buffer_.length(); }
+  size_t GetCursor() const { return cursor_; }
+  bool IsEndOfBuffer() const { return cursor_ >= GetLength(); }
+
+  // Returns true if there are |len| more bytes between the current cursor
+  // position and the end of the buffer.
+  bool CanRead(size_t len) const;
+
+  // Returns true if there are |len| more bytes between |offset| and the end
+  // of the buffer. The cursor position is not used or modified.
+  bool CanReadFrom(size_t offset, size_t len) const;
+
+  // Returns true if it would be possible to read the payload described by the
+  // security buffer.
+  bool CanReadFrom(ntlm::SecurityBuffer sec_buf) const {
+    return CanReadFrom(sec_buf.offset, sec_buf.length);
+  }
+
+  // Reads a 16 bit value (little endian) as a uint16_t. If there are not 16
+  // more bits available it returns false.
+  bool ReadUInt16(uint16_t* value);
+
+  // Reads a 32 bit value (little endian) as a uint32_t. If there are not 32
+  // more bits available it returns false.
+  bool ReadUInt32(uint32_t* value);
+
+  // Reads a 64 bit value (little endian) as a uint64_t. If there are not 64
+  // more bits available it returns false.
+  bool ReadUInt64(uint64_t* value);
+
+  // Reads |len| bytes and copies them into |buffer|.
+  bool ReadBytes(uint8_t* buffer, size_t len);
+
+  // A security buffer is an 8 byte structure that defines the offset and
+  // length of a payload (string, struct or byte array) that appears after the
+  // fixed part of the message.
+  //
+  // The structure is (little endian fields):
+  //     uint16 - |length| Length of payload
+  //     uint16 - Allocation (this is always ignored and not returned)
+  //     uint32 - |offset| Offset from start of message
+  bool ReadSecurityBuffer(ntlm::SecurityBuffer* sec_buf);
+
+  // Reads |len| bytes into the std::string value. Technically these are
+  // actually in the OEM character set. In practice the implementation
+  // just treats any of these strings as an opaque byte sequence.
+  bool ReadAsciiString(std::string* value, size_t len);
+
+  // Reads |len| bytes as a UTF-16 string with little-endian byte order.
+  bool ReadUnicodeString(base::string16* value, size_t len);
+
+  // There are 3 message types Negotiate (sent by client), Challenge (sent by
+  // server), and Authenticate (sent by client).
+  //
+  // This reads the message type from the header and will return false if the
+  // value is invalid.
+  bool ReadMessageType(ntlm::MessageType* message_type);
+
+  // First reads a security buffer (see |ReadSecurityBuffer|), then verifies

[Ryan Sleevi, 2017/06/08 18:36:33]

s/First reads/Reads/

[zentaro, 2017/06/12 23:12:06]

Done.

+  // that the offset and length it states are within the message, then
+  // reads those bytes from the payload part of the message into a
+  // std::string.
+  //
+  // If the security buffer can't be read or the values in the security buffer
+  // are outside the bounds of the message then false will be returned.
+  //
+  // In the case of failure the cursor will remain as if it never read the
+  // security buffer. In the case of success the cursor will point after the
+  // security buffer.
+  bool ReadAsciiPayload(std::string* value);
+
+  // First reads a security buffer (see |ReadSecurityBuffer|), then verifies
+  // that the offset and length it states are within the message, then
+  // reads those bytes from the payload part of the message into a
+  // base::string16. The 16 bit characters within the string are read
+  // little endian.
+  //
+  // If the security buffer can't be read or the values in the security buffer
+  // are outside the bounds of the message then false will be returned.
+  //
+  // In the case of failure the cursor will remain as if it never read the
+  // security buffer. In the case of success the cursor will point after the
+  // security buffer.
+  bool ReadUnicodePayload(base::string16* value);
+
+  // First reads a security buffer (see |ReadSecurityBuffer|), then verifies
+  // that the offset and length it states are within the message, then
+  // reads those bytes from the payload and copies them into the provided
+  // |buffer|.
+  //
+  // If the security buffer can't be read or the values in the security buffer
+  // are outside the bounds of the message or |buffer_len| is less than the
+  // size of the payload then false will be returned.
+  //
+  // In the case of failure the cursor will remain as if it never read the
+  // security buffer. In the case of success the cursor will point after the
+  // security buffer.
+  bool ReadBytesPayload(uint8_t* buffer, size_t buffer_len);
+
+  // Skips over a security buffer field without reading the fields. This is
+  // the equivalent of advancing the cursor 8 bytes. Returns false if there
+  // are less than 8 bytes left in the buffer.
+  bool SkipSecurityBuffer();
+
+  // Skips over the security buffer without returning the values, but fails if
+  // the values would cause a read outside the buffer if the payload was
+  // actually read.
+  bool SkipSecurityBufferWithValidation();
+
+  // Skips over |count| bytes in the buffer. Returns false if there is not
+  // |count| bytes left in the buffer.
+  bool SkipBytes(size_t count);
+
+  // Reads and returns true if the next 8 bytes matches the signature in an
+  // NTLM message "NTLMSSP\0"
+  bool MatchSignature();

[Ryan Sleevi, 2017/06/08 18:36:33]

Do Match() methods update the cursor or not? From the lack of explicitly
mentioning it, I would assume "no"

[zentaro, 2017/06/12 23:12:06]

They do. Made more explicit.

+
+  // Reads the message type from the cursor and returns true if it is
+  // |message_type|.
+  bool MatchMessageType(ntlm::MessageType message_type);
+
+  // Performs |MatchSignature| then |MatchMessageType|.
+  bool MatchMessageHeader(ntlm::MessageType message_type);
+
+  // Reads and returns true if there are |count| bytes of zeros.
+  bool MatchZeros(size_t count);
+
+  // Reads and returns true if the next 8 bytes contain an empty ie. all zero
+  // security buffer.

[asanka, 2017/06/08 18:44:58]

It's not clear to me whether the an empty security buffer is all zeros. Some
parts of the spec say that an empty buffer should be encoded with length=0, and
offset=<where the buffer would be if the length was not zero>.

Have you looked at messages generated from a Windows box to see what they
currently do?

[zentaro, 2017/06/12 23:12:06]

I've seen the part of the spec that says that but IMO it's ambiguous for several
reasons. (1) Do multiple empty buffers point to the same place? Do they all
point to the start of the payload section? (2) Do they point to the end of the
payload that preceded them in the header section? (3) Do they point to the end
of the payload section? Payloads can be in any order. They could even overlap I
assume. So any of the those alternatives could be considered correct.

It seems like that is just a description of how it is implemented not a
meaningful requirement since no read will happen if it is zero length anyway. I
assume they just have a payload_upto variable in their implementation and write
it unconditionally.

I haven't looked at a windows machine though (I suspect it does (2)). The
mozilla implementation of v1 and 2 write zero.

So I don't think it matters and at least the current way it's implemented  it
would need additional work to do option (2).

[asanka, 2017/06/16 03:21:28]

It seems the spec if pretty consistent about requiring that the offset SHOULD
point to where the buffer would've been if it were not empty. E.g.:

"WorkstationBufferOffset field SHOULD be set to the offset from the beginning of
the
NEGOTIATE_MESSAGE to where the WorkstationName would be in Payload if it was
present."

[zentaro, 2017/06/21 00:38:46]

Well it says the length MUST be zero but the offset SHOULD be 'where it would
have been'. But it also says payloads can be in any order (and different
implementations do use different orders) and that they can have arbitrary
amounts of padding before and after each payload. So any value >=
OLDEST_VERSION_HEADER_LENGTH could be 'where it would have been' depending on
your implementation.

"Payload data can be present in any order within the Payload field, with
variable-length padding before or after the data. The data that can be present
in the Payload field of this message, in no particular order,
are:"

I've changed this function to just match that the length is zero and the offset
is within the message. This is only really used to validate during testing. We
never have a reason to try and enforce that a field is empty for a real message.

I'll change the writing code to always point to the payload.

+  bool MatchEmptySecurityBuffer();
+
+ private:
+  // Reads |sizeof(T)| bytes of an integer type from a little-endian buffer.
+  template <typename T>
+  bool ReadUInt(T* value);
+
+  // Sets the cursor position. This is used when reading payloads to move the
+  // cursor to the start of the payload indicated by the security buffer.
+  bool SetCursor(size_t cursor);
+
+  // Returns a constant pointer to the start of the buffer.
+  const uint8_t* GetBufferPtr() const {
+    return reinterpret_cast<const uint8_t*>(buffer_.data());
+  }
+
+  // Returns a pointer to the underlying buffer at the current cursor
+  // position.
+  const uint8_t* GetBufferAtCursor() const { return GetBufferPtr() + cursor_; }
+
+  // Returns the byte at the current cursor position.
+  uint8_t GetByteAtCursor() const {
+    DCHECK(!IsEndOfBuffer());
+    return *(GetBufferAtCursor());
+  }
+
+  const base::StringPiece buffer_;
+  size_t cursor_;
+
+  DISALLOW_COPY_AND_ASSIGN(NtlmBufferReader);
+};
+
+}  // namespace net
+
+#endif  // NET_BASE_NTLM_BUFFER_READER_H_