Skip to content

Commit ee82637

Browse files
renemereneme
committed
Refactor: Cipher_Mode::finish_msg() using std::span
Previously, the internal customization point finish_msg() did take a std::vector and re-allocated it to accomodate or discard encryption overhead such as paddings and/or AEAD tags. This forces downstream users into using a std::vector and potentially copy data, even when they could pre-allocate enough memory out-of-band in their application. Additionally, an offset was provided to deal with prefix bytes within the vector that should not be processed. The handling of this offset was duplicated in each concrete Cipher_Mode implementation. This is now factored out into the base class. The changes in this commit are not public-facing as they just affect the internal customization point.
1 parent 2244285 commit ee82637

File tree

27 files changed

+726
-522
lines changed

27 files changed

+726
-522
lines changed

‎src/fuzzer/mode_padding.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ void fuzz(std::span<const uint8_t> in) {
161161
FUZZER_ASSERT_EQUAL(ct_esp, ref_esp);
162162
}
163163

164-
const uint16_t ct_cbc = Botan::TLS::check_tls_cbc_padding(in.data(), len);
164+
const uint16_t ct_cbc = Botan::TLS::check_tls_cbc_padding(in);
165165
const uint16_t ref_cbc = ref_tls_cbc_unpad(in);
166166
FUZZER_ASSERT_EQUAL(ct_cbc, ref_cbc);
167167
}

‎src/lib/modes/aead/ccm/ccm.cpp‎

Lines changed: 63 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <botan/internal/ct_utils.h>
1212
#include <botan/internal/fmt.h>
1313
#include <botan/internal/loadstor.h>
14+
#include <botan/internal/stl_util.h>
1415

1516
namespace Botan {
1617

@@ -168,24 +169,28 @@ secure_vector<uint8_t> CCM_Mode::format_c0() {
168169
return C;
169170
}
170171

171-
void CCM_Encryption::finish_msg(secure_vector<uint8_t>& buffer, size_t offset) {
172-
BOTAN_ARG_CHECK(buffer.size() >= offset, "Offset is out of range");
172+
size_t CCM_Encryption::finish_msg(std::span<uint8_t> buffer, size_t input_bytes) {
173+
const auto tag_length = tag_size();
174+
const auto& buffered = msg_buf();
173175

174-
buffer.insert(buffer.begin() + offset, msg_buf().begin(), msg_buf().end());
176+
BOTAN_ASSERT_NOMSG(buffered.size() + input_bytes + tag_length == buffer.size());
175177

176-
const size_t sz = buffer.size() - offset;
177-
uint8_t* buf = buffer.data() + offset;
178+
const auto entire_payload = buffer.first(buffered.size() + input_bytes);
179+
const auto tag = buffer.last(tag_length);
178180

179181
const secure_vector<uint8_t>& ad = ad_buf();
180182
BOTAN_ARG_CHECK(ad.size() % CCM_BS == 0, "AD is block size multiple");
181183

182184
const BlockCipher& E = cipher();
183185

186+
// TODO: consider using std::array<> for all those block-size'ed buffers
187+
// (this requires adapting more helper functions like `format_b0`, ...)
184188
secure_vector<uint8_t> T(CCM_BS);
185-
E.encrypt(format_b0(sz), T);
189+
E.encrypt(format_b0(entire_payload.size()), T);
186190

187-
for(size_t i = 0; i != ad.size(); i += CCM_BS) {
188-
xor_buf(T.data(), &ad[i], CCM_BS);
191+
BufferSlicer ad_bs(ad);
192+
while(!ad_bs.empty()) {
193+
xor_buf(T, ad_bs.take(CCM_BS));
189194
E.encrypt(T);
190195
}
191196

@@ -196,48 +201,61 @@ void CCM_Encryption::finish_msg(secure_vector<uint8_t>& buffer, size_t offset) {
196201

197202
secure_vector<uint8_t> X(CCM_BS);
198203

199-
const uint8_t* buf_end = &buf[sz];
204+
// copy all buffered input into the in/out buffer if needed
205+
if(!buffered.empty()) {
206+
copy_mem(entire_payload.last(input_bytes), entire_payload.first(input_bytes));
207+
copy_mem(entire_payload.first(buffered.size()), buffered);
208+
}
209+
210+
// TODO: Use BufferTransformer, once it is available
211+
// See https://github.com/randombit/botan/pull/4151
212+
BufferSlicer payload_slicer(entire_payload);
213+
BufferStuffer payload_stuffer(entire_payload);
200214

201-
while(buf != buf_end) {
202-
const size_t to_proc = std::min<size_t>(CCM_BS, buf_end - buf);
215+
while(!payload_slicer.empty()) {
216+
const size_t to_proc = std::min<size_t>(CCM_BS, payload_slicer.remaining());
217+
const auto in_chunk = payload_slicer.take(to_proc);
218+
const auto out_chunk = payload_stuffer.next(to_proc);
203219

204-
xor_buf(T.data(), buf, to_proc);
220+
xor_buf(std::span{T}.first(in_chunk.size()), in_chunk);
205221
E.encrypt(T);
206222

207223
E.encrypt(C, X);
208-
xor_buf(buf, X.data(), to_proc);
224+
xor_buf(out_chunk, std::span{X}.first(out_chunk.size()));
209225
inc(C);
210-
211-
buf += to_proc;
212226
}
213227

214228
T ^= S0;
215229

216-
buffer += std::make_pair(T.data(), tag_size());
230+
BOTAN_DEBUG_ASSERT(tag.size() <= T.size());
231+
copy_mem(tag, std::span{T}.first(tag.size()));
217232

218233
reset();
219-
}
220234

221-
void CCM_Decryption::finish_msg(secure_vector<uint8_t>& buffer, size_t offset) {
222-
BOTAN_ARG_CHECK(buffer.size() >= offset, "Offset is out of range");
235+
return buffer.size();
236+
}
223237

224-
buffer.insert(buffer.begin() + offset, msg_buf().begin(), msg_buf().end());
238+
size_t CCM_Decryption::finish_msg(std::span<uint8_t> buffer, size_t input_bytes) {
239+
const auto tag_length = tag_size();
240+
const auto& buffered = msg_buf();
225241

226-
const size_t sz = buffer.size() - offset;
227-
uint8_t* buf = buffer.data() + offset;
242+
BOTAN_ASSERT_NOMSG(buffer.size() >= tag_length);
243+
BOTAN_ASSERT_NOMSG(buffered.size() + input_bytes == buffer.size());
228244

229-
BOTAN_ARG_CHECK(sz >= tag_size(), "input did not include the tag");
245+
const auto entire_payload = buffer.first(buffer.size() - tag_length);
246+
const auto tag = buffer.last(tag_length);
230247

231248
const secure_vector<uint8_t>& ad = ad_buf();
232249
BOTAN_ARG_CHECK(ad.size() % CCM_BS == 0, "AD is block size multiple");
233250

234251
const BlockCipher& E = cipher();
235252

236253
secure_vector<uint8_t> T(CCM_BS);
237-
E.encrypt(format_b0(sz - tag_size()), T);
254+
E.encrypt(format_b0(entire_payload.size()), T);
238255

239-
for(size_t i = 0; i != ad.size(); i += CCM_BS) {
240-
xor_buf(T.data(), &ad[i], CCM_BS);
256+
BufferSlicer ad_bs(ad);
257+
while(!ad_bs.empty()) {
258+
xor_buf(T, ad_bs.take<CCM_BS>());
241259
E.encrypt(T);
242260
}
243261

@@ -249,30 +267,39 @@ void CCM_Decryption::finish_msg(secure_vector<uint8_t>& buffer, size_t offset) {
249267

250268
secure_vector<uint8_t> X(CCM_BS);
251269

252-
const uint8_t* buf_end = &buf[sz - tag_size()];
270+
// copy all buffered input into the in/out buffer if needed
271+
if(!buffered.empty()) {
272+
copy_mem(buffer.last(input_bytes), buffer.first(input_bytes));
273+
copy_mem(buffer.first(buffered.size()), buffered);
274+
}
275+
276+
// TODO: Use BufferTransformer, once it is available
277+
// See https://github.com/randombit/botan/pull/4151
278+
BufferSlicer payload_slicer(entire_payload);
279+
BufferStuffer payload_stuffer(entire_payload);
253280

254-
while(buf != buf_end) {
255-
const size_t to_proc = std::min<size_t>(CCM_BS, buf_end - buf);
281+
while(!payload_slicer.empty()) {
282+
const size_t to_proc = std::min<size_t>(CCM_BS, payload_slicer.remaining());
283+
const auto in_chunk = payload_slicer.take(to_proc);
284+
const auto out_chunk = payload_stuffer.next(to_proc);
256285

257286
E.encrypt(C, X);
258-
xor_buf(buf, X.data(), to_proc);
287+
xor_buf(out_chunk, std::span{X}.first(out_chunk.size()));
259288
inc(C);
260289

261-
xor_buf(T.data(), buf, to_proc);
290+
xor_buf(std::span{T}.first(in_chunk.size()), in_chunk);
262291
E.encrypt(T);
263-
264-
buf += to_proc;
265292
}
266293

267294
T ^= S0;
268295

269-
if(!CT::is_equal(T.data(), buf_end, tag_size()).as_bool()) {
296+
if(!CT::is_equal(T.data(), tag.data(), tag.size()).as_bool()) {
270297
throw Invalid_Authentication_Tag("CCM tag check failed");
271298
}
272299

273-
buffer.resize(buffer.size() - tag_size());
274-
275300
reset();
301+
302+
return entire_payload.size();
276303
}
277304

278305
} // namespace Botan

‎src/lib/modes/aead/ccm/ccm.h‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ class CCM_Mode : public AEAD_Mode {
6363

6464
secure_vector<uint8_t>& msg_buf() { return m_msg_buf; }
6565

66+
const secure_vector<uint8_t>& msg_buf() const { return m_msg_buf; }
67+
6668
secure_vector<uint8_t> format_b0(size_t msg_size);
6769
secure_vector<uint8_t> format_c0();
6870

@@ -96,10 +98,14 @@ class CCM_Encryption final : public CCM_Mode {
9698

9799
size_t output_length(size_t input_length) const override { return input_length + tag_size(); }
98100

101+
size_t bytes_needed_for_finalization(size_t final_input_length) const override {
102+
return output_length(msg_buf().size() + final_input_length);
103+
}
104+
99105
size_t minimum_final_size() const override { return 0; }
100106

101107
private:
102-
void finish_msg(secure_vector<uint8_t>& final_block, size_t offset = 0) override;
108+
size_t finish_msg(std::span<uint8_t> final_block, size_t input_bytes) override;
103109
};
104110

105111
/**
@@ -122,10 +128,15 @@ class CCM_Decryption final : public CCM_Mode {
122128
return input_length - tag_size();
123129
}
124130

131+
size_t bytes_needed_for_finalization(size_t final_input_length) const override {
132+
BOTAN_ARG_CHECK(final_input_length >= tag_size(), "Sufficient input");
133+
return msg_buf().size() + final_input_length;
134+
}
135+
125136
size_t minimum_final_size() const override { return tag_size(); }
126137

127138
private:
128-
void finish_msg(secure_vector<uint8_t>& final_block, size_t offset = 0) override;
139+
size_t finish_msg(std::span<uint8_t> final_block, size_t input_bytes) override;
129140
};
130141

131142
} // namespace Botan

‎src/lib/modes/aead/chacha20poly1305/chacha20poly1305.cpp‎

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -102,21 +102,29 @@ size_t ChaCha20Poly1305_Encryption::process_msg(uint8_t buf[], size_t sz) {
102102
return sz;
103103
}
104104

105-
void ChaCha20Poly1305_Encryption::finish_msg(secure_vector<uint8_t>& buffer, size_t offset) {
106-
update(buffer, offset);
105+
size_t ChaCha20Poly1305_Encryption::finish_msg(std::span<uint8_t> buffer, size_t input_bytes) {
106+
const auto tag_length = tag_size();
107+
108+
BOTAN_ASSERT_NOMSG(input_bytes + tag_length == buffer.size());
109+
110+
const auto payload = buffer.first(input_bytes);
111+
const auto tag = buffer.last(tag_length);
112+
113+
process(payload);
107114
if(cfrg_version()) {
108115
if(m_ctext_len % 16) {
109-
const uint8_t zeros[16] = {0};
110-
m_poly1305->update(zeros, 16 - m_ctext_len % 16);
116+
std::array<uint8_t, 16> zeros{};
117+
m_poly1305->update(std::span{zeros}.first(16 - m_ctext_len % 16));
111118
}
112119
update_len(m_ad.size());
113120
}
114121
update_len(m_ctext_len);
115122

116-
buffer.resize(buffer.size() + tag_size());
117-
m_poly1305->final(&buffer[buffer.size() - tag_size()]);
123+
m_poly1305->final(tag);
118124
m_ctext_len = 0;
119125
m_nonce_len = 0;
126+
127+
return buffer.size();
120128
}
121129

122130
size_t ChaCha20Poly1305_Decryption::process_msg(uint8_t buf[], size_t sz) {
@@ -126,43 +134,40 @@ size_t ChaCha20Poly1305_Decryption::process_msg(uint8_t buf[], size_t sz) {
126134
return sz;
127135
}
128136

129-
void ChaCha20Poly1305_Decryption::finish_msg(secure_vector<uint8_t>& buffer, size_t offset) {
130-
BOTAN_ARG_CHECK(buffer.size() >= offset, "Offset is out of range");
131-
const size_t sz = buffer.size() - offset;
132-
uint8_t* buf = buffer.data() + offset;
137+
size_t ChaCha20Poly1305_Decryption::finish_msg(std::span<uint8_t> buffer, size_t input_bytes) {
138+
const auto tag_length = tag_size();
133139

134-
BOTAN_ARG_CHECK(sz >= tag_size(), "input did not include the tag");
140+
BOTAN_ASSERT_NOMSG(buffer.size() == input_bytes);
141+
BOTAN_ASSERT_NOMSG(buffer.size() >= tag_length);
135142

136-
const size_t remaining = sz - tag_size();
143+
const auto remaining = buffer.first(buffer.size() - tag_length);
144+
const auto tag = buffer.last(tag_length);
137145

138-
if(remaining) {
139-
m_poly1305->update(buf, remaining); // poly1305 of ciphertext
140-
m_chacha->cipher1(buf, remaining);
141-
m_ctext_len += remaining;
146+
if(!remaining.empty()) {
147+
process(remaining);
142148
}
143149

144150
if(cfrg_version()) {
145151
if(m_ctext_len % 16) {
146-
const uint8_t zeros[16] = {0};
147-
m_poly1305->update(zeros, 16 - m_ctext_len % 16);
152+
std::array<uint8_t, 16> zeros{};
153+
m_poly1305->update(std::span{zeros}.first(16 - m_ctext_len % 16));
148154
}
149155
update_len(m_ad.size());
150156
}
151157

152158
update_len(m_ctext_len);
153159

154-
uint8_t mac[16];
160+
std::array<uint8_t, 16> mac;
155161
m_poly1305->final(mac);
156162

157-
const uint8_t* included_tag = &buf[remaining];
158-
159163
m_ctext_len = 0;
160164
m_nonce_len = 0;
161165

162-
if(!CT::is_equal(mac, included_tag, tag_size()).as_bool()) {
166+
if(!CT::is_equal(mac.data(), tag.data(), tag.size()).as_bool()) {
163167
throw Invalid_Authentication_Tag("ChaCha20Poly1305 tag check failed");
164168
}
165-
buffer.resize(offset + remaining);
169+
170+
return remaining.size();
166171
}
167172

168173
} // namespace Botan

‎src/lib/modes/aead/chacha20poly1305/chacha20poly1305.h‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,13 @@ class ChaCha20Poly1305_Encryption final : public ChaCha20Poly1305_Mode {
7777

7878
size_t minimum_final_size() const override { return 0; }
7979

80+
size_t bytes_needed_for_finalization(size_t final_input_length) const override {
81+
return output_length(final_input_length);
82+
}
83+
8084
private:
8185
size_t process_msg(uint8_t buf[], size_t size) override;
82-
void finish_msg(secure_vector<uint8_t>& final_block, size_t offset = 0) override;
86+
size_t finish_msg(std::span<uint8_t> final_block, size_t input_bytes) override;
8387
};
8488

8589
/**
@@ -94,9 +98,14 @@ class ChaCha20Poly1305_Decryption final : public ChaCha20Poly1305_Mode {
9498

9599
size_t minimum_final_size() const override { return tag_size(); }
96100

101+
size_t bytes_needed_for_finalization(size_t final_input_length) const override {
102+
BOTAN_ARG_CHECK(final_input_length >= tag_size(), "Sufficient input");
103+
return final_input_length;
104+
}
105+
97106
private:
98107
size_t process_msg(uint8_t buf[], size_t size) override;
99-
void finish_msg(secure_vector<uint8_t>& final_block, size_t offset = 0) override;
108+
size_t finish_msg(std::span<uint8_t> final_block, size_t input_bytes) override;
100109
};
101110

102111
} // namespace Botan

0 commit comments

Comments
 (0)