Skip to content

Commit

Permalink
Make gtls_session_ticket always go through unique_ptr
Browse files Browse the repository at this point in the history
The copy and move constructors/operators are a decent amount of
complexity keeping track of pointers that really isn't needed because we
nearly always want these held inside a unique_ptr, and so it simplifies
things to just enforce that everywhere and fix the two occurances that
held in a value and then sort of mutated into a unique_ptr.

This also lets us make the key data const, which gives us better safety
assurances for using it as the key of a map (as we currently are).

Also makes the private fields actually private, along with the
constructors (to enforce all usage now go through make).

Also dropped some unused comparison & hashing functions.
  • Loading branch information
jagerman committed Nov 21, 2024
1 parent 11a3b4c commit 3fb037b
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 76 deletions.
4 changes: 2 additions & 2 deletions include/oxen/quic/endpoint.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ namespace oxen::quic

bool stateless_reset_enabled() const { return _stateless_reset_enabled; }

int validate_anti_replay(gtls_session_ticket ticket, time_t exp);
void store_session_ticket(gtls_session_ticket ticket);
int validate_anti_replay(gtls_ticket_ptr ticket, time_t exp);
void store_session_ticket(gtls_ticket_ptr ticket);
gtls_ticket_ptr get_session_ticket(const ustring_view& remote_pk);

private:
Expand Down
89 changes: 26 additions & 63 deletions include/oxen/quic/gnutls_crypto.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,21 +260,21 @@ namespace oxen::quic
}
};

struct gtls_session_ticket;
using gtls_ticket_ptr = std::unique_ptr<gtls_session_ticket>;
struct gtls_session_ticket
{
std::vector<unsigned char> _key;
private:
const std::vector<unsigned char> _key;
std::vector<unsigned char> _ticket;
gnutls_datum_t _data;

explicit gtls_session_ticket(
const unsigned char* key, unsigned int keysize, const unsigned char* ticket, unsigned int ticketsize) :
_key(keysize), _ticket(ticketsize)
{
std::memcpy(_key.data(), key, keysize);
std::memcpy(_ticket.data(), ticket, ticketsize);
_data.data = _ticket.data();
_data.size = _ticket.size();
}
_key{key, key + keysize},
_ticket{ticket, ticket + ticketsize},
_data{.data = _ticket.data(), .size = ticketsize}
{}

explicit gtls_session_ticket(ustring_view key, ustring_view ticket) :
gtls_session_ticket{
Expand All @@ -284,56 +284,33 @@ namespace oxen::quic
static_cast<unsigned int>(ticket.size())}
{}

gtls_session_ticket(const gnutls_datum_t* key, const gnutls_datum_t* ticket) :
gtls_session_ticket{key->data, key->size, ticket->data, ticket->size}
{}

gtls_session_ticket(const gtls_session_ticket& t) :
gtls_session_ticket{
t._key.data(),
static_cast<unsigned int>(t._key.size()),
t._ticket.data(),
static_cast<unsigned int>(t._ticket.size())}
{}

gtls_session_ticket& operator=(gtls_session_ticket&& other)
{
_key = std::move(other._key);
_ticket = std::move(other._ticket);
_data.data = _ticket.data();
_data.size = _ticket.size();
return *this;
}
public:
gtls_session_ticket() = delete;
gtls_session_ticket(gtls_session_ticket&& t) = delete;
gtls_session_ticket(const gtls_session_ticket& t) = delete;
gtls_session_ticket& operator=(gtls_session_ticket&&) = delete;
gtls_session_ticket& operator=(const gtls_session_ticket&) = delete;

static std::unique_ptr<gtls_session_ticket> make(ustring_view key, ustring_view ticket)
static gtls_ticket_ptr make(ustring_view key, ustring_view ticket)
{
return std::make_unique<gtls_session_ticket>(key, ticket);
return gtls_ticket_ptr(new gtls_session_ticket{key, ticket});
}

static std::unique_ptr<gtls_session_ticket> make(gtls_session_ticket&& g)
static gtls_ticket_ptr make(const gnutls_datum_t* key, const gnutls_datum_t* ticket)
{
return std::make_unique<gtls_session_ticket>(std::move(g));
return make({key->data, key->size}, {ticket->data, ticket->size});
}

// Returns a view of the key for this ticket. The view is valid as long as this
// gtls_session_ticket object remains alive, and so can be used (for example) as the key of
// a map containing the object in the value.
ustring_view key() const { return {_key.data(), _key.size()}; }

bool operator==(const gtls_session_ticket& other) const { return _ticket == other._ticket; }

bool operator==(const gnutls_datum_t* other) const
{
return gnutls_memcmp(_data.data, other->data, _data.size) == 0;
}
// Returns a view of the ticket data.
ustring_view ticket() const { return {_ticket.data(), _ticket.size()}; }

template <concepts::gtls_datum_type T>
operator T*()
{
return &_data;
}
template <concepts::gtls_datum_type T>
operator const T*() const
{
return &_data;
}
// Accesses the ticket data pointer as needed by gnutls API
const gnutls_datum_t* datum() const { return &_data; }
gnutls_datum_t* datum() { return &_data; }
};

struct Packet;
Expand Down Expand Up @@ -449,17 +426,3 @@ namespace oxen::quic
Connection* get_connection_from_gnutls(gnutls_session_t g_session);

} // namespace oxen::quic

namespace std
{
template <>
struct hash<oxen::quic::gtls_session_ticket>
{
size_t operator()(const oxen::quic::gtls_session_ticket& t) const noexcept
{
auto h = hash<oxen::quic::ustring_view>{}(oxen::quic::ustring_view{t._ticket.data(), t._ticket.size()});
h ^= hash<oxen::quic::ustring_view>{}(t.key()) + oxen::quic::inverse_golden_ratio + (h << 7) + (h >> 3);
return h;
}
};
} // namespace std
2 changes: 0 additions & 2 deletions include/oxen/quic/opt.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,6 @@ namespace oxen::quic
};
} // namespace opt

using gtls_ticket_ptr = std::unique_ptr<gtls_session_ticket>;

using gtls_db_validate_cb = std::function<bool(gtls_ticket_ptr, time_t)>;
using gtls_db_get_cb = std::function<gtls_ticket_ptr(ustring_view)>;
using gtls_db_put_cb = std::function<void(gtls_ticket_ptr, time_t)>;
Expand Down
10 changes: 5 additions & 5 deletions src/endpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ namespace oxen::quic

if (auto it = session_tickets.find(key); it != session_tickets.end())
{
if (auto exp = gnutls_db_check_entry_expire_time(*it->second); current < exp)
if (auto exp = gnutls_db_check_entry_expire_time(it->second->datum()); current < exp)
{
log::debug(log_cat, "Found existing anti-replay ticket for incoming connection; rejecting...");
return GNUTLS_E_DB_ENTRY_EXISTS;
Expand Down Expand Up @@ -550,15 +550,15 @@ namespace oxen::quic
}
}

int Endpoint::validate_anti_replay(gtls_session_ticket ticket, time_t current)
int Endpoint::validate_anti_replay(gtls_ticket_ptr ticket, time_t current)
{
return _validate_0rtt_ticket(gtls_session_ticket::make(std::move(ticket)), current) ? 0 : GNUTLS_E_DB_ENTRY_EXISTS;
return _validate_0rtt_ticket(std::move(ticket), current) ? 0 : GNUTLS_E_DB_ENTRY_EXISTS;
}

void Endpoint::store_session_ticket(gtls_session_ticket ticket)
void Endpoint::store_session_ticket(gtls_ticket_ptr ticket)
{
log::trace(log_cat, "Storing session ticket...");
return _put_session_ticket(gtls_session_ticket::make(std::move(ticket)), 0);
return _put_session_ticket(std::move(ticket), 0);
}

gtls_ticket_ptr Endpoint::get_session_ticket(const ustring_view& remote_pk)
Expand Down
8 changes: 4 additions & 4 deletions src/gnutls_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace oxen::quic
if (not ep->zero_rtt_enabled())
throw std::runtime_error{"Anti-replay DB hook should not be called on 0rtt-disabled endpoint!"};

return ep->validate_anti_replay({key, data}, exp_time);
return ep->validate_anti_replay(gtls_session_ticket::make(key, data), exp_time);
}

int gtls_session_callbacks::client_session_cb(
Expand Down Expand Up @@ -53,8 +53,8 @@ namespace oxen::quic
return rv;
}

ep.store_session_ticket(gtls_session_ticket{
remote_key.data(), static_cast<unsigned int>(remote_key.size()), encoded.data, encoded.size});
ep.store_session_ticket(gtls_session_ticket::make(
remote_key, {encoded.data, encoded.size}));
}

return 0;
Expand Down Expand Up @@ -248,7 +248,7 @@ namespace oxen::quic
{
gnutls_datum_t d;

if (auto rv = gnutls_pem_base64_decode2(SESSION_TICKET_HEADER, &maybe_ticket->_data, &d); rv != 0)
if (auto rv = gnutls_pem_base64_decode2(SESSION_TICKET_HEADER, maybe_ticket->datum(), &d); rv != 0)
{
log::warning(log_cat, "Failed to decode session ticket: {}", ngtcp2_strerror(rv));
throw std::runtime_error("gnutls_pem_base64_decode2 failed");
Expand Down

0 comments on commit 3fb037b

Please sign in to comment.