ECDSA signing & public key exporting #2

AndrzejKurek · 2019-05-21T09:46:19Z

This PR implements hardware ECDSA signing and public key exporting using ATECC608A or ATECC508A.
Prerequisite - a cryptoauth device (508A or 608A) comissioned, with a locked configuration and a private key generated in slot 0 (warning - configuration locking is irreversible!). An example application doing this with a quite safe config can be seen here.

This PR has a prerequisite: ARMmbed/mbed-os-atecc608a#3

Patater · 2019-05-21T10:07:49Z

"playin' round" is not a good commit message...

AndrzejKurek · 2019-05-21T10:09:12Z

"playin' round" is not a good commit message...

Sorry, couldn't rewrite the history on your repository (due to permissions), so I opted for doing it here once it gets reviewed :)

Patater · 2019-05-21T10:09:30Z

atecc608a_se.c

+    size_t key_data_len = 65;
+    if(data_size < key_data_len)
+    {
+        printf("FAILED! Key buffer too small.\n");


I don't think we want printf in the driver.

Patater · 2019-05-21T10:12:11Z

atecc608a_se.c

+
+    /* XXX how to check the curve? */
+    //PSA_KEY_TYPE_ECC_KEYPAIR(PSA_ECC_CURVE_SECP256R1);
+    //PSA_KEY_TYPE_ECC_KEYPAIR(PSA_ECC_CURVE_SECP256K1);


We should check that the curve being requested is what the SE supports (only PSA_ECC_CURVE_SECP256R1).

Patater · 2019-05-21T10:13:33Z

main.c

-    status = psa_allocate_key(&sign_handle);
-    ASSERT_STATUS(status, PSA_SUCCESS);
+    /* Register first slot */
+    status = psa_register_se_slot(atecc608a_key_slot_psa, atecc608a_key_slot_device,


I don't think we want to register the SE or any of its slots, yet. We just want to call the driver directly for now from the example.

Patater · 2019-05-21T10:14:47Z

atecc608a_se.c

@@ -0,0 +1,224 @@
+#include "atecc608a_se.h"


The actual driver shouldn't be part of this example, but in https://github.com/ARMmbed/mbed-os-atecc608a/tree/mbed-cryptoauthlib instead, which is the repo for the driver. If we don't do this, other applications aside from our example can't use the driver.

Patater · 2019-05-21T10:17:31Z

main.c

    ASSERT_STATUS(status, PSA_SUCCESS);

-    printf("derp\n");


I don't think we need "derp" in the history. It's just playin' around and not serious.

gilles-peskine-arm

I reviewed the code, without knowing the ATAC API. I didn't review the setup instructions.

gilles-peskine-arm · 2019-05-21T10:46:28Z

atecc608a_se.c

+    case ATCA_INVALID_SIZE:
+    case ATCA_SMALL_BUFFER:
+    case ATCA_BAD_OPCODE:
+    case ATCA_ASSERT_FAILURE:


I haven't read the documentation, but I think many of these error codes don't map to INVALID_ARGUMENT. For example ASSERT_FAILURE is CORRUPTION_DETECTED. SMALL_BUFFER, INVALID_SIZE may be BUFFER_TOO_SMALL or an internal error depending on how buffers are managed. BAD_OPCODE is probably an internal error or hardware or communication failure.

Docs are at https://github.com/MicrochipTech/cryptoauthlib/blob/master/lib/atca_status.h#L41-L79 for the statuses. They aren't intuitively named and you need to read the docs to know what to map to.

gilles-peskine-arm · 2019-05-21T11:07:01Z

atecc608a_se.c

+
+    /* We can only do ECDSA on SHA-256 */
+    /* PSA_ALG_ECDSA(PSA_ALG_SHA_256) */
+    if(!PSA_ALG_IS_ECDSA(alg))


I think this should be alg != PSA_ALG_ECDSA(PSA_ALG_SHA_256) && alg != PSA_ALG_ECDSA_ANY. And then you don't need to check for randomized below, but you still need to check the hash length.

gilles-peskine-arm · 2019-05-21T11:07:53Z

atecc608a_se.c

+        return PSA_ERROR_NOT_SUPPORTED;
+    }
+
+    /* XXX how to check the curve? */


The curve doesn't need to be checked here: it's a property of the key, not a parameter of the signature operation. It need to be checked when you create the key. At this point, the only way to find the curve would be to query metadata which isn't directly available to this function. Which raises a design question for the SE HAL: should the key type be made visible to this function?

What if the key is always pre-provisioned and never created by any API call?

We have a function to register a slot as occupied (different from creating the key), so should this fail if we try to say a key slot is occupied by a key the hardware doesn't support? Should the driver be notified when a new key is specified as being pre-provisioned at run-time so that it can error if it doesn't support that type of key? Without involving the driver at slot registration or key creation time, I don't see how Mbed Crypto can know what the hardware is capable of and be able to fail.

The pre-provisioning mechanism needs to set the metadata that the crypto library uses.

A key can't be “specified as being pre-provisioned at run-time”. By definition, a pre-provisioned key is one that exists before the system boots. If a key is provisioned at runtime, this should be done via the normal key creation mechanism.

Mbed Crypto doesn't need to be aware of what the driver is capable to handle pre-provisioned keys: that's the problem of the pre-provisioning mechanism. Mbed Crypto does need to know what slots are capable of storing what kinds of keys (type, size). I don't think we've fully nailed down the mechanism for that yet. This driver hard-codes slot assignments, so it doesn't need this slot capability mechanism.

A key can be “specified as being pre-provisioned at run-time”. You say "Hey Mbed Crypto. I have a key pre-provisioned in this slot, associated with this lifetime. You don't have to generate it or import it. It's already in that slot and the hardware knows about it" via psa_register_se_slot(). This was the proposed API from Derek developed during prototyping. The API may not include such a function yet, but this example assumes that it exists; we may want to change this.

psa_register_se_slot() is a function exposed from Mbed Crypto for setting the metadata that it uses, so that the pre-provisioning implementation doesn't need to know Mbed Crypto internals. It'd anticipate this function only being functional if the PSA Device Security Lifecycle of the device currently allows specifying what is pre-provisioned in your system (e.g. factory mode).

Oh, I see what you mean. The pre-provisioning happened earlier but the specification of it happens at run time. Yes, indeed. But in any case, that mechanism must set all the metadata about the key, including its type. The library core stores the metadata outside the SE.

Yes, and I think that function is the right place to reject the addition of invalid metadata, like an unsupported key type. Which means we need a driver hook for being able to provide the rejection criteria, giving the driver the opportunity to reject metadata.

But no metadata is added in this function! This function doesn't take a key type as argument. When an application performs a signature with a pre-existing key, it never needs to know the key type. This function takes an algorithm as argument, and the core guarantees that this algorithm is compatible with the slot's policy. The place where the key type and the policy can and must be checked against what the slot contains is when the key is registered.

psa_register_se_slot() does take the key type as an argument.

gilles-peskine-arm · 2019-05-21T11:09:32Z

atecc608a_se.c

+
+    /* Signature will be returned here. Format is R and S integers in
+     * big-endian format. 64 bytes for P256 curve. */
+    ret = atcab_sign(key_id, p_hash, p_signature);


Does the PSA crypto SE API guarantee that signature_size >= 64 here?

If not, the driver would have to enforce this. To avoid duplicate checks (every driver having to have identical checks), it might be better to have this checked before we reach the driver.

gilles-peskine-arm · 2019-05-21T11:10:22Z

atecc608a_se.c

+
+    printf("atecc608a_asymmetric_sign - signature size %d:\n", *p_signature_length);
+    atcab_printbin_sp(p_signature, *p_signature_length);
+    return atecc608a_to_psa_error(ret);


This code path is only taken when ret == ATCA_SUCCESS.

gilles-peskine-arm · 2019-05-21T12:06:06Z

atecc608a_se.c

+    if(data_size < key_data_len)
+    {
+        printf("FAILED! Key buffer too small.\n");
+        return PSA_ERROR_INVALID_ARGUMENT;


Suggested change

return PSA_ERROR_INVALID_ARGUMENT;

return PSA_ERROR_BUFFER_TOO_SMALL;

gilles-peskine-arm · 2019-05-21T12:08:54Z

atecc608a_se.c

+{
+    ATCA_STATUS ret = ATCA_SUCCESS;
+    uint16_t slot = key;
+    size_t key_data_len = 65;


This function does not implement the right format. A private EC key is represented as just the private value, a 32-byte string for a 256-bit curve. The public key is not stored in the representation of the private key.

This is the public key exporting function, I just got the name wrong. Fixing.

gilles-peskine-arm · 2019-05-21T12:10:34Z

atecc608a_se.c

+    }
+    ASSERT_STATUS(atcab_init(&atca_iface_config), ATCA_SUCCESS);
+
+    ret = atcab_get_pubkey(slot, &p_data[1]);


export_key only exports the private value. A separate function should implement export_public_key, returning a 65-byte value ({0x04} + xP + yP).

This is the public key exporting function, I just got the name wrong. Fixing.

gilles-peskine-arm · 2019-05-21T12:14:49Z

atecc608a_se.c

+    uint8_t serial[ATCA_SERIAL_NUM_SIZE];
+
+    printf("**********************************************\n");
+    ASSERT_STATUS(atcab_init(&atca_iface_config), ATCA_SUCCESS);


The driver code should not use ASSERT_STATUS, which prints a debugging message and returns -1. It should return an appropriate error code when an error happens, e.g. PSA_ERROR_HARDWARE_FAILURE if the hardware returns an unexpected error code. The driver may contain debug prints, but these should be controlled by an #ifdef.

gilles-peskine-arm · 2019-05-21T12:17:10Z

atecc608a_se.h

+        {                                                           \
+            printf("assertion failed at %s:%d "                     \
+                   "(actual=%d expected=%d)\n", __FILE__, __LINE__, \
+                   (int)actual, (int)expected);                     \


actual and expected are expressions, not values. This line calculates the expressions again: the way this macro is typically used, this calls the atcab_xxx function a second time. Store the values in a local variable and remember to put parentheses around the expression.

int ASSERT_STATUS_actual = (actual); int ASSERT_STATUS_expected = (expected); if (actual != expected) printf(…, actual, expected);

AndrzejKurek · 2019-05-22T11:56:02Z

@gilles-peskine-arm, @Patater - I have updated this PR and created a second one with the driver here: ARMmbed/mbed-os-atecc608a#2. Please re-review.

gilles-peskine-arm

I reviewed the sample code and the instructions. The code is fine, the readme needs work. I did not try to reproduce the instructions.

gilles-peskine-arm · 2019-05-23T11:06:28Z

main.c

+            return -1;                                              \
+        }                                                           \
+    } while(0)
+


Suggestion: define and use #define ASSERT_SUCCESS(expr) ASSERT_STATUS((expr), PSA_SUCCESS)

gilles-peskine-arm · 2019-05-23T11:17:01Z

setup.md

@@ -0,0 +1,92 @@
+### Install mbed-cli to a venv


Is it really useful to have instructions for installing mbed-cli here, rather than linking to https://github.com/ARMmbed/mbed-cli#installing-mbed-cli? See https://github.com/ARMmbed/mbed-os-example-blinky and https://github.com/ARMmbed/mbed-os-example-tls for comparison.

Yes and no. We could relegate the venv trick for coping with the tool to oral tradition. If the tool didn't clobber your environment so much, I'd recommend just linking, sure.

I've changed it so there's an easy setup mentioned first. If you'd like it to be a link to the instructions instead Gilles, let me know.

gilles-peskine-arm · 2019-05-23T11:20:55Z

setup.md

@@ -0,0 +1,92 @@
+### Install mbed-cli to a venv


Assuming these instructions are useful to have here, they aren't easy to follow. Most readers aren't Python experts. A typical reader's reaction here: what's a venv? why should I care? what's a virtual environment?

I recommend to NOT include virtualenv instructions here. Just do pip install --user mbed-cli. If you do, you need to explain that it's about Python and only about Python, that it matters for mbed-cli because it's written in Python, and that if the reader does this then they need to run virtualenv venvs/mbed to open a new shell from which they can then run mbed.

mbed-cli is really horrible to your default python environment. It does way too much installing and even did version downgrading in the past (might still). Even though the title is "venv" and you may not have a clue what that is as a new developer, the why you'd want to is explained.

I've changed this paragraph to have two sets of instructions - one without a venv, and a second - with.

gilles-peskine-arm · 2019-05-23T11:22:03Z

setup.md

+pip install mbed-cli
+```
+
+### Install the GCC_ARM toolchain


Is it really useful to have instructions for installing gcc-arm here, rather than linking to some generic instructions? For example, I wouldn't do any of this because I'm running Ubuntu and I instead do apt-get install gcc-arm-none-eabi.

The Ubuntu supplied version may be out of date and not support ARMv8-M, for instance.

Updated to link to generic download page and external instructions.

gilles-peskine-arm · 2019-05-23T11:23:40Z

setup.md

+```
+
+If you don't like having multiple copies of mbed-crypto or mbed-os lying
+around, feel free to make symlinks or git worktrees as desired.


Last I tried, mbed didn't like git worktrees, and git worktrees are largely incompatible with submodules.

It works for me and how I do things. You can make up your own mind if you want to do the same, which is why there is "feel free" reasoning.

I feel like this tip is not intrusive, and properly noted as a non-obligatory step, so I'd leave it as is.

It didn't work for me when I tried it. @Patater I'd like to know how you do it!

I did mbed deploy once. Then replaced repos with symlinks. Running subsequent mbed deploy will update the symlinked repo.

mbed deploy doesn't know what a worktree is and won't like being symlinked to one of those. It also doesn't like having a real worktree within the project folder (non-symlinked). However, mbed compile works file with worktrees. You can run git commands to update tot he latest lib file, if you want. It'd be nice if mbed recognized worktrees; then we could have worktrees inside every example.

gilles-peskine-arm · 2019-05-23T11:25:09Z

setup.md

+
+Add any driver files (none created so far) to the mbed-os-atecc608a folder. It
+is backed by the
+[mbed-os-atecc608a](https://github.com/ARMmbed/mbed-os-atecc608a/tree/mbed-cryptoauthlib)


This is a link to private repository in a public file. Most people can't follow this link.

I've made the repo public.

gilles-peskine-arm · 2019-05-23T11:26:21Z

setup.md

@@ -0,0 +1,92 @@
+### Install mbed-cli to a venv


Why is this setup.md and not README.md?

It's for developers. README is for users. We could rename to HACKING.md or CONTRIBUTING.md.

Patater · 2019-05-24T13:37:19Z

main.c

+    uint8_t serial[ATCA_SERIAL_NUM_SIZE];
+    size_t buffer_length;
+
+    if(atecc608a_get_serial_number(serial, ATCA_SERIAL_NUM_SIZE,


Pre-existing: add a space after if.

Patater · 2019-05-24T13:38:26Z

main.c

+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+    };


Consider re-using sha256_expected_hash1 for this.

I'd rather go for clarity, since this is just an example.

Patater · 2019-05-24T13:40:00Z

main.c

+
+    atecc608a_print_locked_zones();
+    /* Verify that the device has a locked config before doing anything */
+    ASSERT_STATUS(atecc608a_check_config_locked(), PSA_SUCCESS);


This seems very secure-element specific. There is no PSA driver function to check that the hardware is ready for use. I'd expect this to be done in a probe() function (the per-hardware init function; not the per-driver init function), which we probably need to invent and add to the driver API.

Patater · 2019-05-24T13:40:51Z

main.c

+    ASSERT_STATUS(atecc608a_check_config_locked(), PSA_SUCCESS);
+
+    status = atecc608a_export_public_key(atecc608a_key_slot_device, pubkey,
+                                         sizeof(pubkey), &pubkey_len);


Use the driver struct instead of the secure element directly. This will allow portability between drivers more easily; plus, I'm recommending against having this be a publicly visible function at all in the driver review.

Patater · 2019-05-24T13:41:05Z

main.c

+                                         sizeof(pubkey), &pubkey_len);
+    ASSERT_STATUS(status, PSA_SUCCESS);
+
+    status = atecc608a_asymmetric_sign(atecc608a_key_slot_device, alg, hash,


Use the struct, Luke.

|-o-| [-o-] |-o-|

Patater · 2019-05-24T13:43:27Z

update-crypto.sh

+    make update
+fi
+make
+#cd $curdir


Delete last line here.

Patater · 2019-05-24T13:44:22Z

Please also update the log file so that this test can be automatically tested. https://github.com/ARMmbed/mbed-os-example-atecc608a/blob/master/tests/atecc608a.log

AndrzejKurek · 2019-05-27T11:13:38Z

@Patater - regarding the log file - should we test if the verification was successful, or if the driver was used? If it's the latter - the prints are turned off by default (by a define), so either an additional step has to be added, or the logs turned on by default.

Explain how to set up a development environment to work on this example.

Add a developer convenience script to run the importer.

AndrzejKurek · 2019-05-27T13:22:16Z

I've rebased the PR and added a test log check to ensure that the device has a locked configuration and that verification works. @Patater, @gilles-peskine-arm - please re-review.

gilles-peskine-arm

Other than the code duplication, and the issues I raised previously about setup.md, this looks fine.

gilles-peskine-arm · 2019-05-27T17:27:02Z

atecc608a_utils.c

+    .rx_retries = 20,
+};
+
+psa_status_t atecc608a_to_psa_error(ATCA_STATUS ret)


We don't need two copies of this function in different code bases. ARMmbed/mbed-os-atecc608a#3 should expose this function, since it's convenient to have it outside the driver.

On a platform with secure/non-secure isolation, the application and the driver will be in separate linker namespaces, so the function won't be accessible in the application. But the application wouldn't work anyway, since it wouldn't be able to access the hardware.

Same remark for other support functions and macros like ATCAB_INIT and friends.

Patater

I don't see atca_iface_config referenced here, even though the driver exports it. Are you sure it needs to be exported?

Patater · 2019-05-28T10:57:45Z

main.c

+
+    ASSERT_SUCCESS_PSA((*atecc608a_drv_info.p_asym->p_sign)
+                        (atecc608a_key_slot_device, alg, hash, sizeof(hash),
+                         signature, sizeof(signature), &signature_length));


Your function pointer syntax doesn't look how I'd expect. Does this work?

ASSERT_SUCCESS_PSA(atecc608a_drv_info.p_asym->p_sign( atecc608a_key_slot_device, alg, hash, sizeof(hash), signature, sizeof(signature), &signature_length));

Yep, it works too. I just prefer the first syntax. Would you like me to change it?

I think the explicit parenthesis wrapping and dereferencing makes it less clear, as it implies there is a reason to do those things, which implies something tricky is going on. I'd prefer, and many readers of our code would probably expect, my syntax.

Patater · 2019-05-28T11:07:15Z

Ah, I see atca_iface_config is referenced via the public macro ATCAB_INIT. Could we use a function instead?

AndrzejKurek · 2019-05-28T12:41:29Z

@Patater - I've opted for a macro for better readability (and compactness), as it also wraps error handling. We can have a function, but then the only real gain would be hiding the configuration struct - error handling would be pretty much the same as with the raw atcab_init call.

Patater · 2019-05-28T13:26:33Z

I don't agree a macro is more readable, and hiding the configuration is a benefit. Readability is pretty important; macros can make things less direct and harder to read (even if they may be more compact in some cases).

gilles-peskine-arm · 2019-05-28T13:51:00Z

atecc608a_utils.h

 psa_status_t atecc608a_get_serial_number(uint8_t* buffer, size_t buffer_size,
                                         size_t *buffer_length);
 psa_status_t atecc608a_check_config_locked();
-psa_status_t atecc608a_to_psa_error(ATCA_STATUS ret);
-
-extern ATCAIfaceCfg atca_iface_config;

 #endif /* ATECC608A_SE_H */


Missing trailing newline. Please pass check-files.py.

gilles-peskine-arm

LGTM apart from setup.py.

AndrzejKurek · 2019-05-29T07:32:02Z

@gilles-peskine-arm - which comment about setup.py would you like me to address? I thought that @Patater has replied to your concerns, but if you'd like to insist on introducing any of your suggestions - let me know which.
I have also fixed the whitespace errors, changed the format of calling function pointers and changed the init & deinit macros to functions, to hide the configuration.

gilles-peskine-arm

LGTM apart from setup.md.

I can't mark conversations as resolved here, so I added a 🎉 reaction to the conversations I consider to be resolved. The others are still pending. Even if Jaeden disagrees with what the content should be, I don't find the current content helpful in those places.

Patater

LGTM

Patater · 2019-05-29T14:43:27Z

setup.md

+```
+
+If you don't like having multiple copies of mbed-crypto or mbed-os lying
+around, feel free to make symlinks or git worktrees as desired.


I did mbed deploy once. Then replaced repos with symlinks. Running subsequent mbed deploy will update the symlinked repo.

mbed deploy doesn't know what a worktree is and won't like being symlinked to one of those. It also doesn't like having a real worktree within the project folder (non-symlinked). However, mbed compile works file with worktrees. You can run git commands to update tot he latest lib file, if you want. It'd be nice if mbed recognized worktrees; then we could have worktrees inside every example.

Update the driver in order to gain the ability to perform signature operations.

Patater · 2019-05-29T15:28:18Z

Added a commit to update the driver to the merge result of ARMmbed/mbed-os-atecc608a#3. Tested that the build worked.

AndrzejKurek · 2019-05-30T07:18:33Z

I tested it and it works.

AndrzejKurek changed the title ~~Psa crypto driver~~ ECDSA signing & public key exporting May 21, 2019

Patater suggested changes May 21, 2019

View reviewed changes

gilles-peskine-arm suggested changes May 21, 2019

View reviewed changes

AndrzejKurek force-pushed the psa-crypto-driver branch from 3eb5a8e to fe3c2a0 Compare May 22, 2019 11:40

AndrzejKurek mentioned this pull request May 22, 2019

Add hardware signing and key exporting ARMmbed/mbed-os-atecc608a#2

Closed

gilles-peskine-arm suggested changes May 23, 2019

View reviewed changes

AndrzejKurek mentioned this pull request May 24, 2019

Add hardware signing and key exporting ARMmbed/mbed-os-atecc608a#3

Merged

Patater suggested changes May 24, 2019

View reviewed changes

AndrzejKurek force-pushed the psa-crypto-driver branch from f98c310 to 3111334 Compare May 27, 2019 11:17

Patater and others added 7 commits May 27, 2019 09:11

Name the libary dependency correctly

91bf729

Add developer setup guide

eeb7c01

Explain how to set up a development environment to work on this example.

Add hardware signing and key exporting

5c11b63

Add script to update crypto

5e6070f

Add a developer convenience script to run the importer.

Add a buffer_length parameter when printing serial number

62fe4d7

Reshape the example to use the secure element driver structure

ab7ee36

Remove a comment from script

2d4af00

AndrzejKurek force-pushed the psa-crypto-driver branch from 3111334 to 2d4af00 Compare May 27, 2019 13:12

Adjust test log to ensure that verification works

0450289

gilles-peskine-arm suggested changes May 27, 2019

View reviewed changes

Remove utility functions that are visible in the driver

19724e9

Patater suggested changes May 28, 2019

View reviewed changes

gilles-peskine-arm reviewed May 28, 2019

View reviewed changes

Change init and deinit macro to a function

7b50eb7

gilles-peskine-arm suggested changes May 29, 2019

View reviewed changes

Update instructions in setup.md

703aa54

gilles-peskine-arm previously approved these changes May 29, 2019

View reviewed changes

Patater previously approved these changes May 29, 2019

View reviewed changes

mbed-os-atecc608a: Update driver for signature

9ee704d

Update the driver in order to gain the ability to perform signature operations.

Patater dismissed stale reviews from gilles-peskine-arm and themself via 9ee704d May 29, 2019 15:27

Patater requested a review from gilles-peskine-arm May 29, 2019 15:27

Patater approved these changes May 29, 2019

View reviewed changes

gilles-peskine-arm approved these changes May 29, 2019

View reviewed changes

Patater merged commit 2acf3be into ARMmbed:master May 30, 2019

	return PSA_ERROR_INVALID_ARGUMENT;
	return PSA_ERROR_BUFFER_TOO_SMALL;

ECDSA signing & public key exporting #2

ECDSA signing & public key exporting #2

Uh oh!

Conversation

AndrzejKurek commented May 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Patater commented May 21, 2019

Uh oh!

AndrzejKurek commented May 21, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Patater May 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Patater May 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Patater May 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrzejKurek May 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrzejKurek May 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrzejKurek commented May 22, 2019

Uh oh!

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

AndrzejKurek commented May 21, 2019 •

edited

Loading

Patater May 21, 2019 •

edited

Loading

Patater May 21, 2019 •

edited

Loading

Patater May 21, 2019 •

edited

Loading

AndrzejKurek May 22, 2019 •

edited

Loading

AndrzejKurek May 22, 2019 •

edited

Loading