Mir

Merge lp:~mir-team/mir/160-bit-finally into lp:mir

Proposed by Brandon Schaefer
Status: Merged
Approved by: Brandon Schaefer
Approved revision: no longer in the source branch.
Merged at revision: 3270
Proposed branch: lp:~mir-team/mir/160-bit-finally
Merge into: lp:mir
Prerequisite: lp:~mir-team/mir/public-cookie-api
Diff against target: 210 lines (+90/-14)
7 files modified
debian/copyright (+16/-0)
src/cookie/CMakeLists.txt (+1/-0)
src/cookie/authority.cpp (+13/-12)
src/cookie/const_memcmp.cpp (+33/-0)
src/cookie/const_memcmp.h (+26/-0)
src/include/cookie/mir/cookie/blob.h (+1/-1)
src/server/input/default_event_builder.cpp (+0/-1)
To merge this branch: bzr merge lp:~mir-team/mir/160-bit-finally
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Kevin DuBois (community) Approve
Mir CI Bot continuous-integration Needs Fixing
Alberto Aguirre (community) Approve
Review via email: mp+283751@code.launchpad.net

This proposal supersedes a proposal from 2016-01-24.

Commit message

mir-cookie: Use a constant time memcmp to verify cookie contents

Additionally use the full SHA1 size of 160 bits for the cookie MAC.

Description of the change

mir-cookie: Use a constant time memcmp to verify cookie contents

Additionally use the full SHA1 size of 160 bits for the cookie MAC.

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3270
https://mir-jenkins.ubuntu.com/job/mir-ci/146/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/147/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/146/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

E: Unable to locate package libsodium-dev
apt download failed. Exit value: 100

That ought to be there. Restarting

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

Is it too difficult to write an equivalent function to sodium_memcmp()? Or, perhaps, are we looking to use more functions from this library in the near future? I'd rather have an internal memcmp (if its not to difficult to write) than add an additional dependency.

review: Needs Information
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Decided to just grab the function we will be using from sodium (since its an MIT/ISC)

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3271
https://mir-jenkins.ubuntu.com/job/mir-ci/154/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/154/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/154/rebuild

review: Approve (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3272
https://mir-jenkins.ubuntu.com/job/mir-ci/155/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/155/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/155/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

LGTM.

review: Approve
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3272
https://mir-jenkins.ubuntu.com/job/mir-ci/156/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/156/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/156/rebuild

review: Approve (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3273
https://mir-jenkins.ubuntu.com/job/mir-ci/157/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/157/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/157/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

alright, thanks for removing the dependency for the one function. lgtm too.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3273
http://jenkins.qa.ubuntu.com/job/mir-ci/6126/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5682
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4589
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5638
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/334
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/450
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/450/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/450
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/450/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5635
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5635/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8070
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27027
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/330
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/330/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/186
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27030

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/6126/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/copyright'
--- debian/copyright 2015-02-22 07:46:25 +0000
+++ debian/copyright 2016-01-25 18:06:54 +0000
@@ -153,3 +153,19 @@
153 THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT153 THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
154 (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE154 (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
155 OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.155 OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
156
157 Files: src/cookie/const_memcmp.h src/cookie/const_memcmp.cpp
158 Copyright: Copyright (c) 2013-2016
159 Frank Denis <j at pureftpd dot org>
160
161 Permission to use, copy, modify, and/or distribute this software for any
162 purpose with or without fee is hereby granted, provided that the above
163 copyright notice and this permission notice appear in all copies.
164
165 THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
166 WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
167 MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
168 ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
169 WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
170 ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
171 OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
156172
=== modified file 'src/cookie/CMakeLists.txt'
--- src/cookie/CMakeLists.txt 2016-01-25 18:06:53 +0000
+++ src/cookie/CMakeLists.txt 2016-01-25 18:06:54 +0000
@@ -24,6 +24,7 @@
2424
25 ${symbol_map}25 ${symbol_map}
26 authority.cpp26 authority.cpp
27 const_memcmp.cpp
27 hmac_cookie.cpp28 hmac_cookie.cpp
28)29)
2930
3031
=== modified file 'src/cookie/authority.cpp'
--- src/cookie/authority.cpp 2016-01-25 18:06:53 +0000
+++ src/cookie/authority.cpp 2016-01-25 18:06:54 +0000
@@ -20,6 +20,7 @@
20#include "mir/cookie/authority.h"20#include "mir/cookie/authority.h"
21#include "mir/cookie/blob.h"21#include "mir/cookie/blob.h"
22#include "hmac_cookie.h"22#include "hmac_cookie.h"
23#include "const_memcmp.h"
23#include "format.h"24#include "format.h"
2425
25#include <algorithm>26#include <algorithm>
@@ -28,6 +29,7 @@
28#include <system_error>29#include <system_error>
2930
30#include <nettle/hmac.h>31#include <nettle/hmac.h>
32
31#include <linux/random.h>33#include <linux/random.h>
32#include <sys/types.h>34#include <sys/types.h>
33#include <sys/stat.h>35#include <sys/stat.h>
@@ -41,7 +43,8 @@
41{43{
42std::string const random_device_path{"/dev/random"};44std::string const random_device_path{"/dev/random"};
43std::string const urandom_device_path{"/dev/urandom"};45std::string const urandom_device_path{"/dev/urandom"};
44int const wait_seconds{30};46uint32_t const wait_seconds{30};
47uint32_t const mac_byte_size{20};
4548
46size_t cookie_size_from_format(mir::cookie::Format const& format)49size_t cookie_size_from_format(mir::cookie::Format const& format)
47{50{
@@ -141,9 +144,9 @@
141 {144 {
142 /*145 /*
143 SHA_1 Format:146 SHA_1 Format:
144 1 byte = FORMAT147 1 byte = FORMAT
145 8 btyes = TIMESTAMP148 8 bytes = TIMESTAMP
146 8 BYTES = MAC149 20 bytes = MAC
147 */150 */
148151
149 if (raw_cookie.size() != cookie_size_from_format(mir::cookie::Format::hmac_sha_1_8))152 if (raw_cookie.size() != cookie_size_from_format(mir::cookie::Format::hmac_sha_1_8))
@@ -162,11 +165,10 @@
162 auto ptr = raw_cookie.data();165 auto ptr = raw_cookie.data();
163 ptr++;166 ptr++;
164167
165 memcpy(&timestamp, ptr, 8);168 memcpy(&timestamp, ptr, sizeof(uint64_t));
166 ptr += sizeof(timestamp);169 ptr += sizeof(timestamp);
167170
168 // FIXME Soon to be 20 bytes171 std::vector<uint8_t> mac(mac_byte_size);
169 std::vector<uint8_t> mac(8);
170 memcpy(mac.data(), ptr, mac.size());172 memcpy(mac.data(), ptr, mac.size());
171173
172 std::unique_ptr<mir::cookie::Cookie> cookie =174 std::unique_ptr<mir::cookie::Cookie> cookie =
@@ -183,10 +185,9 @@
183private:185private:
184 std::vector<uint8_t> calculate_cookie(uint64_t const& timestamp)186 std::vector<uint8_t> calculate_cookie(uint64_t const& timestamp)
185 {187 {
186 // FIXME Soon to change to 160bits, for now uint64_t188 std::vector<uint8_t> mac(mac_byte_size);
187 std::vector<uint8_t> mac(sizeof(uint64_t));
188 hmac_sha1_update(&ctx, sizeof(timestamp), reinterpret_cast<uint8_t const*>(&timestamp));189 hmac_sha1_update(&ctx, sizeof(timestamp), reinterpret_cast<uint8_t const*>(&timestamp));
189 hmac_sha1_digest(&ctx, sizeof(uint64_t), reinterpret_cast<uint8_t*>(mac.data()));190 hmac_sha1_digest(&ctx, mac.size(), mac.data());
190191
191 return mac;192 return mac;
192 }193 }
@@ -198,8 +199,8 @@
198 auto const this_stream = cookie->serialize();199 auto const this_stream = cookie->serialize();
199 auto const other_stream = calculated_cookie->serialize();200 auto const other_stream = calculated_cookie->serialize();
200201
201 // FIXME Need to do a constant memcmp here!202 return this_stream.size() == other_stream.size() &&
202 return std::equal(std::begin(this_stream), std::end(this_stream), std::begin(other_stream));203 mir::cookie::const_memcmp(this_stream.data(), other_stream.data(), this_stream.size()) == 0;
203 }204 }
204205
205 struct hmac_sha1_ctx ctx;206 struct hmac_sha1_ctx ctx;
206207
=== added file 'src/cookie/const_memcmp.cpp'
--- src/cookie/const_memcmp.cpp 1970-01-01 00:00:00 +0000
+++ src/cookie/const_memcmp.cpp 2016-01-25 18:06:54 +0000
@@ -0,0 +1,33 @@
1/*
2 * Copyright (c) 2013-2016
3 * Frank Denis <j at pureftpd dot org>
4 *
5 * Permission to use, copy, modify, and/or distribute this software for any
6 * purpose with or without fee is hereby granted, provided that the above
7 * copyright notice and this permission notice appear in all copies.
8 *
9 * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
10 * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
11 * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
12 * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
13 * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
14 * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
15 * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
16 */
17
18#include "const_memcmp.h"
19
20int mir::cookie::const_memcmp(void const* const b1_, void const* const b2_, size_t len)
21{
22 volatile unsigned char const* b1 = (volatile unsigned char const*) b1_;
23 volatile unsigned char const* b2 = (volatile unsigned char const*) b2_;
24
25 size_t i;
26 unsigned char d = (unsigned char) 0U;
27
28 for (i = 0U; i < len; i++)
29 {
30 d |= b1[i] ^ b2[i];
31 }
32 return (1 & ((d - 1) >> 8)) - 1;
33}
034
=== added file 'src/cookie/const_memcmp.h'
--- src/cookie/const_memcmp.h 1970-01-01 00:00:00 +0000
+++ src/cookie/const_memcmp.h 2016-01-25 18:06:54 +0000
@@ -0,0 +1,26 @@
1/*
2 * Copyright (c) 2013-2016
3 * Frank Denis <j at pureftpd dot org>
4 *
5 * Permission to use, copy, modify, and/or distribute this software for any
6 * purpose with or without fee is hereby granted, provided that the above
7 * copyright notice and this permission notice appear in all copies.
8 *
9 * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
10 * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
11 * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
12 * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
13 * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
14 * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
15 * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
16 */
17
18#include "stddef.h"
19
20namespace mir
21{
22namespace cookie
23{
24int const_memcmp(void const* const b1_, void const* const b2_, size_t len);
25}
26}
027
=== modified file 'src/include/cookie/mir/cookie/blob.h'
--- src/include/cookie/mir/cookie/blob.h 2016-01-25 18:06:53 +0000
+++ src/include/cookie/mir/cookie/blob.h 2016-01-25 18:06:54 +0000
@@ -25,7 +25,7 @@
25{25{
26namespace cookie26namespace cookie
27{27{
28size_t const default_blob_size = 17;28size_t const default_blob_size = 29;
29using Blob = std::array<uint8_t, default_blob_size>;29using Blob = std::array<uint8_t, default_blob_size>;
30}30}
31}31}
3232
=== modified file 'src/server/input/default_event_builder.cpp'
--- src/server/input/default_event_builder.cpp 2016-01-25 18:06:53 +0000
+++ src/server/input/default_event_builder.cpp 2016-01-25 18:06:54 +0000
@@ -69,7 +69,6 @@
69 const float x_axis_value = 0;69 const float x_axis_value = 0;
70 const float y_axis_value = 0;70 const float y_axis_value = 0;
71 std::vector<uint8_t> vec_cookie{};71 std::vector<uint8_t> vec_cookie{};
72 // FIXME Moving to 160bits soon
73 if (action == mir_pointer_action_button_up || action == mir_pointer_action_button_down)72 if (action == mir_pointer_action_button_up || action == mir_pointer_action_button_down)
74 {73 {
75 auto const cookie = cookie_authority->make_cookie(timestamp.count());74 auto const cookie = cookie_authority->make_cookie(timestamp.count());

Subscribers

People subscribed via source and target branches