Add TOTP support, resolves #80 by weslly · Pull Request #380 · keepassxreboot/keepassxc

weslly · 2017-03-06T00:24:52Z

Description

This PR add basic support for TOTP code generation (as described by rfc6238) using KeepassXC.

How it Works

Right now it supports the format used by the 2 TOTP plugins listed on Keepass plugins page: KeeOtp and Tray TOTP.

The following formats are supported:

	Attribute name	Value
Tray TOTP	`TOTP Seed`	`XXXXXXXXXXXXXXXX`
KeeOtp	`otp`	`key=XXXXXXXXXXXXXXXX`

If your entry has one of those attributes, the "Timed One Time Password" context menu will be available.

Motivation and Context

A lot of people avoid using 2FA with TOTP because having to get your phone and opening Google Authenticator app every time you need to login is a hassle. There's also the risk of losing access to your accounts if you lose your phone.

Most modern password managers have support for generating TOTP codes and even Keepass 2.x have a couple of plugins that add this feature so it would be very useful for people who use KeepassXC to have access to their TOTP codes right from Keepass interface.

resolves #80

How Has This Been Tested?

I tested about 10 different sites which I had setup 2FA with TOTP. All of them worked fine with the generated codes.

Screenshots (if appropriate):

Types of changes

✅ New feature (non-breaking change which adds functionality)

Checklist:

✅ I have read the CONTRIBUTING document.
✅ My code follows the code style of this project.
✅ All new and existing tests passed.

droidmonkey · 2017-03-06T02:51:58Z

This will be great to add to the new HTTP protocol to specifically be able to request and fill in the current totp code.

weslly · 2017-03-06T11:24:07Z

The dialog looks a lot better now. 😄

ping @TheZ3ro

TheZ3ro · 2017-03-06T11:36:36Z

I love it! Testing this on windows RN

phoerious · 2017-03-06T12:05:04Z

Hell, yeah. It does. 😊

phoerious · 2017-03-06T19:11:08Z

src/gui/DatabaseWidget.cpp

+
+    TotpDialog* totpDialog = new TotpDialog(this, currentEntry);
+    totpDialog->setWindowFlags(totpDialog->windowFlags() | Qt::WindowStaysOnTopHint);
+    totpDialog->show();


I would prefer opening this as a modal dialog (sheet on OS X) with open(). Then you also don't need Qt::WindowStaysOnTopHint.

Actually I was trying to do this before but couldn't find how since I have no experience with QT. Looks even better now :)

phoerious · 2017-03-06T19:11:15Z

src/gui/DatabaseWidget.cpp

+    TotpDialog* totpDialog = new TotpDialog(this, currentEntry);
+    totpDialog->setWindowFlags(totpDialog->windowFlags() | Qt::WindowStaysOnTopHint);
+    totpDialog->show();
+    return;


return here does nothing.

phoerious · 2017-03-06T19:13:22Z

src/gui/TotpDialog.cpp

+
+    updateProgressBar();
+
+    QTimer *timer=new QTimer(this);


Spaces around operator missing.

phoerious · 2017-03-06T19:13:40Z

src/gui/TotpDialog.cpp

+
+void TotpDialog::updateProgressBar()
+{
+    if(uCounter <= 100) {


Space after if missing.

phoerious · 2017-03-06T19:14:14Z

src/gui/TotpDialog.cpp

+
+void TotpDialog::updateSeconds()
+{
+    m_ui->timerLabel->setText(tr("Expires in <b>") + QString::number(30 - (uCounter*30/100)) + tr("</b> seconds")); 


I don't think <b>…</b> should be part of the translatable string. Also add spaces around operators.

phoerious · 2017-03-06T19:14:48Z

src/gui/TotpDialog.cpp

+
+    if (curSeconds >= 30)
+        curSeconds = curSeconds - 30;
+    return curSeconds/30*100;


phoerious · 2017-03-06T19:18:09Z

src/gui/TotpDialog.cpp

+
+void TotpDialog::copyToClipboard()
+{
+    clipboard()->setText(m_entry->totp());


Some "copy and close" functionality could be useful (just a suggestion).

phoerious · 2017-03-06T19:19:32Z

src/CMakeLists.txt

                      ${GPGERROR_LIBRARIES}
-                      ${ZLIB_LIBRARIES})
+                      ${ZLIB_LIBRARIES}
+                      ${OATH_LIBRARIES})


Since this adds a new dependency, adding a -DWITH_XC_TOTP CMake option would be useful to optionally compile KeePassXC without TOTP support.

Should I set it ON by default?

@droidmonkey @TheZ3ro What do you think?

Are the libraries readily available on both MinGW and OSX? If so I say leave it as-is and update the wiki. This should be treated like core capability and not a plugin.

phoerious · 2017-03-06T19:22:45Z

share/translations/keepassx_en.ts

        <source>&amp;Clone entry</source>
        <translation type="unfinished"></translation>
    </message>
+    <message>


Shouldn't this be spelled "Timed one-time password"?

louib · 2017-03-06T19:41:26Z

src/CMakeLists.txt

    gui/ChangeMasterKeyWidget.cpp
    gui/Clipboard.cpp
    gui/CloneDialog.cpp
+    gui/TotpDialog.cpp


@weslly This list is sorted alphabetically. This line should go further down.

louib · 2017-03-06T19:41:50Z

src/CMakeLists.txt

    gui/AboutDialog.ui
    gui/ChangeMasterKeyWidget.ui
    gui/CloneDialog.ui
+    gui/TotpDialog.ui


@weslly this list is also sorted alphabetically.

weslly · 2017-03-06T19:48:02Z

Just one thing before I submit the fixes, I'm getting a segfault crash when the progressbar/counter resets but it only happens on newly added entries, if you close the program and open again it works fine. I think it crashes when m_entry->totp() is called to get the new code on reset at TotpDialog.cpp but I can't find what is exactly wrong since it works with old entries. Can you guys take a look at it?

~~Edit: Seems like it happens only when using the cmd+T shortcut. When I use the mouse to click on the menu it works fine.~~

Edit 2: For some reason the segfault was happening because of "Cmd+T" as the shortcut so maybe it was a conflict with OSX native key bindings? I changed the shortcut to "cmd+shift+T" and now it doesn't crash anymore.

TheZ3ro

Before merging this I would like to compile liboath on windows msys2-env. Right now it's not working. I've opened an issue here https://gitlab.com/oath-toolkit/oath-toolkit/issues/1

droidmonkey · 2017-03-06T23:52:01Z

With this library are we able to also use it to setup a timed password for the database itself? I could see this "solving" our keylogger issue on entry of the master password since two-level authentication would be required. This would be similar to the Yubikey implementation.

droidmonkey · 2017-03-06T23:54:57Z

src/core/Entry.cpp

+        time_t t_now = time(NULL);
+        char *b_secret = NULL;
+        size_t b_secret_len;
+        int rc;


This variable name is ambiguous, recommend renaming to "result"

droidmonkey · 2017-03-06T23:56:30Z

src/core/Entry.cpp

+            rc = oath_totp_generate(b_secret, b_secret_len, t_now, 30, 0, 6, output);
+        }
+
+        if (rc != OATH_OK) {


Is the intention of having this as a separate if statement to also catch errors from oath_totp_generate? Seems like a different error message should be displayed, no?

phoerious · 2017-03-08T16:46:25Z

src/core/Entry.cpp

+        }
+
+        const char *secret = qPrintable(seed.toLatin1());
+        char output[20];


One more thing I just noticed here: isn't this over-allocated? According to the docs, this should have a size of the number of digits + 1 (for the \0 character).

I would also much prefer if you saved the intended number of digits in a constant numDigits (const unsigned), used a C99 dynamic array here to allocate the buffer with length numDigits + 1 and then passed this constant as a parameter to oath_totp_generate(). That way we avoid possible buffer overflows if someone changes the number of output digits, but doesn't adjust the buffer size or the like.

You may also want to use names for the other parameters ofoath_totp_generate() instead of magic numbers.

phoerious · 2017-03-08T16:52:27Z

src/core/Entry.cpp

+                                &result, &result_len);
+
+        if (rc == OATH_OK) {
+            rc = oath_totp_generate(result, result_len, t_now, 30, 0, 6, output);


Does this even work? The documentation of oath_base32_decode() says

If out is NULL, then *outlen will be set to what would have been the length of *out on successful encoding.

So as I understand it, result would still be NULL. If for some reason, the docs are wrong, though, it would mean that result had been allocated dynamically and would need to be deallocated again.

I'm also not a huge fan of using the NULL define in C++ code (even when using a C API). You should instead use nullptr.

I believe this is interested in the address of the pointer container itself (hence the reference to a pointer). After this call, it will be pointing to the allocated data made in the function.

There may be a memory leak, however, because result is never deleted.

Which is not what the docs says. That's why I'm asking. But if it gets allocated, there is a memory leak for sure.

If out is NULL, then *outlen will be set to what would have been the length of *out on successful encoding.

out contains the address of the variable result and not null...

@rockihack Yes, you're right. oath_base32_decode() is being given &result as parameter, not result. So there is no dynamic allocation. However, result is a nullptr and not an allocated char array. This means we have a nice multi-byte buffer overflow here.

Yes. Correct way to convert to char array is:

QByteArray seedBytes = seed.toLatin1(); const char* secret = seedBytes.constData();

This is not the fault of C code, the developer didn't read the documentation.

It kind of is. With a clean C++ API, we wouldn't need to convert to raw C strings at all. But the statement was also more targeted towards the memory leak. C does not do RAII.

C++ std strings and different character encodings are even worse.
BTW you should replace strlen() with seed.length().

The encoding should be pretty fixed in this case. That would be a non-issue here.

seed.length() and strlen are equivalent. Since what goes into the C function is actually secret and not seed, I find usage of strlen(seed) consistent, although using the length of the QByteArray directly is probably not the worst idea. But then I would prefer size() instead of length().

secret or seed whatever the name of the variable is.
strlen() runs in time O(n) and var.length() in O(1), so they are not equivalent.

Alright, that's an argument, although basically irrelevant with a fixed string length of 6 bytes. Still would prefer size() over length(), though.

droidmonkey · 2017-03-11T20:54:28Z

src/core/Entry.cpp

+                                &result, &result_len);
+
+        if (rc == OATH_OK) {
+            rc = oath_totp_generate(result, result_len, t_now, 30, 0, 6, output);


I believe this is interested in the address of the pointer container itself (hence the reference to a pointer). After this call, it will be pointing to the allocated data made in the function.

There may be a memory leak, however, because result is never deleted.

droidmonkey · 2017-03-11T20:55:36Z

src/core/Entry.cpp

+        const char *secret = qPrintable(seed.toLatin1());
+        char output[20];
+        time_t t_now = time(NULL);
+        char *result = NULL;


This variable should be renamed to: secret_decoded

phoerious · 2017-03-13T20:18:22Z

src/core/Entry.cpp

+        } else {
+            return QString(tr("Invalid TOTP Seed."));
+        }
+


You still need to free() secret_decoded.

phoerious · 2017-03-13T20:19:41Z

src/core/Entry.cpp

+        const char* secret = seedBytes.constData();
+        char output[numDigits + 1];
+        time_t t_now = time(NULL);
+        char *secret_decoded = nullptr;


The * belongs to the type, not the identifier.

rockihack · 2017-03-13T20:55:42Z

src/core/Entry.cpp

+        char* secret_decoded = nullptr;
+        size_t secret_decoded_len;
+
+        int rc = oath_init();


Return value not checked.

rockihack · 2017-03-13T20:56:09Z

src/core/Entry.cpp

+                OATH_TOTP_DEFAULT_START_TIME, // 0
+                numDigits, output);
+        } else {
+            return QString(tr("Invalid TOTP Seed."));


Missing a call to oath_done().

rockihack · 2017-03-13T20:56:34Z

src/core/Entry.cpp

+        }
+
+        if (rc != OATH_OK) {
+            return QString(tr("Invalid or unsupported OTP format."));


Missing calls to free(secret_decoded) and oath_done().

phoerious · 2017-03-14T01:29:34Z

src/core/Entry.cpp

                OATH_TOTP_DEFAULT_START_TIME, // 0
                numDigits, output);
        } else {
+            oath_done();


Redundant. oath_done() is called twice when rc != OATH_OK.

oath_done is never called twice, however its called even if oath_init fails.
This method should be refactored.

True. I shouldn't review code so late in the night.

phoerious · 2017-03-14T01:30:26Z

src/core/Entry.cpp

@@ -325,25 +325,28 @@ QString Entry::totp() const{

        int rc = oath_init();


You should check the return value after this or not save it all and wrap the call in Q_UNUSED().

You are wrong. Q_UNUSED should only wrap unused parameters and not return values.

Q_UNUSED only adds (void) in front of the line which can also be used to suppress "unchecked return value" warnings.

It doesn't matter what Q_UNUSED does. Don't try to be smarter than the qt developers if you use their api. Implementations are prone to change.
https://doc.qt.io/qt-5/qtglobal.html#Q_UNUSED

The Qt guys themselves sometimes use it for non-parameter expressions.

Do you have an example? Where do they use it for return values?

This seems like the easiest, most cross-platform way to accomplish a true "ignore return value"

template<typename T> inline void ignore_result(T /* unused result */) {}

Allegedly (void) casting is going out of vogue...

I've seen it being used directly on a static_cast expression (e.g. here https://github.com/qt/qtbase/blob/6bceb4a8a9292ce9f062a38d6fe143460b54370e/tests/auto/corelib/tools/qscopedpointer/tst_qscopedpointer.cpp#L200) and what you find extremely often is this:

int foo = bar(); Q_UNUSED(foo);

IMHO you can shorten that by directly using it on the rvalue of the function return, but in either case, that's not a function parameter.

@droidmonkey That creates unnecessary stack operations and pollutes the namespace, though. You also still need some extra mechanism inside ignore_result() (which would probably be void casting again or some useless operation on the parameter).

Alright they use it to prevent unused parameter AND unused local variable warnings, but I doubt that they use it for return values.

There is virtually no difference. If you really want to be sure that it doesn't break if they change it to something that doesn't work with rvalues (highly unlikely, would also break their own static_cast code), you can always make an lvalue from it before using Q_UNUSED. The point is just that we don't want compiler warnings about ignored return values and Q_UNUSED can get rid of them. Whether you apply it directly to the function call or create an lvalue first, I don't care.

phoerious · 2017-03-14T01:31:37Z

src/core/Entry.cpp

+        QByteArray seedBytes = seed.toLatin1();
+        const char* secret = seedBytes.constData();
+        char output[numDigits + 1];
+        time_t t_now = time(NULL);


Use nullptr instead of NULL.

phoerious · 2017-03-14T01:35:30Z

src/core/Entry.cpp

+        }
+
+        free(secret_decoded);
+        oath_done();


Should also be wrapped in Q_UNUSED(), since it returns an integer.

No it shouldn't, see above.

Throne3d · 2017-03-14T16:09:31Z

src/gui/TotpDialog.cpp

 void TotpDialog::copyToClipboard()
 {
-    clipboard()->setText(m_entry->totp());
+    clipboard()->setText(m_entry->totp()); 


Is this trailing whitespace accidental?

Yes, it is. Thanks, I'll fix on next commit

liboath has now been removed

Gargauth · 2017-03-23T11:10:46Z

Will this support more than 6 digits long codes? They can be specified like otpauth://totp/Authenticator-Test:?secret=RANDOMSECRET&digits=8. Some services I use have 8 digits long codes, so support for it would be greatly appreciated. Thank you.

weslly · 2017-03-23T11:29:45Z

@Gargauth not yet, but I'll implement it as soon as I have some time.

phoerious · 2017-03-25T12:36:25Z

src/totp/totp.cpp

+    const unsigned numDigits = 6;
+
+    int secretLen = (key.length() + 7) / 8 * 5;
+    quint8 secret[100];


One last thing: hard-coded array size. Were does this come from and can't we use a dynamic array here? Or maybe also use QVector instead of a plain C-Array.

Other than that: good job. Code is a lot cleaner now without the C library.

@phoerious The 100 length is pretty much just a guess of a reasonable secret string maximum size (including spaces).

https://github.com/google/google-authenticator-libpam/blob/master/src/google-authenticator.c#L61

I can change the type to something like QVector but secret is used as an argument to base32_decode() which is basically C code from google-auth-libpam and expects a plain C-Array anyways.

Maybe I should change it to be a dynamic array using secretLen + 1 as the size?

Don't guess any sizes. Use the size the string will actually have.

@phoerious Updated

Might be a dumb question, but why use quint8 and not char?

Doesn't make any practical difference in this case, but the source base32_decode() function from google-auth-libpam already used uint8_t instead of char.

rockihack · 2017-03-26T14:15:14Z

src/gui/DatabaseWidget.cpp

+        return;
+    }
+
+    TotpDialog* totpDialog = new TotpDialog(this, currentEntry);


Memory leak?

Shouldn't be a memory leak. The instance is parented to this.

And this refers to a DatabaseWidget, that probably is a long lived object.
Possible fix: totpDialog->setAttribute(Qt::WA_DeleteOnClose);

Yeah, good point.

@rockihack @phoerious
I had Qt::WA_DeleteOnClose already set at TotpDialog.cpp line 34 before, doesn't it have the same effect?

rockihack · 2017-03-26T14:17:53Z

src/totp/totp.cpp

+}
+
+
+QByteArray QTotp::hmacSha1(QByteArray key, QByteArray baseString)


You can use libgcrypt instead of implementing your own hmac construction.

Note: This file and function is part of QTotp library

We should still fix those issues. And yes, I was also thinking about using libgcrypt.

@phoerious If we need to fix those "issues" so we should open PR to QTotp, and if we think about using libgcrypt we should drop QTotp and implement our Totp interface directly

Actually Qt has its own hmac class (QMessageAuthenticationCode) so I'll use that instead of libgcrypt. Which is kinda a relief, because I really don't want to deal with external C libraries again, especially something as low level as libgcrypt seems to be.

rockihack · 2017-03-26T14:21:13Z

src/totp/totp.cpp

+    const unsigned numDigits = 6;
+
+    int secretLen = (key.length() + 7) / 8 * 5;
+    quint8 secret[secretLen];


This does not work as expected. You need to use new[] and delete[] for dynamic arrays.

Only true for VCpp. GCC support dynamic (variable-length) stack arrays.

Ok seems to be valid c code, c++11 standard mentions array size as a constant-expression.

Note: This file and function is part of QTotp library

It's a C99 feature.

requires tests

droidmonkey

Tests of the totp code need to be included. Especially tests of the custom written decode32 function.

rockihack · 2017-03-26T14:40:07Z

src/gui/TotpDialog.cpp

+
+    updateProgressBar();
+
+    QTimer *timer = new QTimer(this);


Memory leak?

This should be a QScopedPointer as a member variable. But it shouldn't leak anything, as it's parented to this.

droidmonkey · 2017-03-26T15:09:52Z

src/totp/totp.cpp

+    int secretLen = (key.length() + 7) / 8 * 5;
+    quint8 secret[secretLen];
+    int res = QTotp::base32_decode(reinterpret_cast<const quint8 *>(key.constData()), secret, secretLen);
+    QByteArray hmac = QTotp::hmacSha1(QByteArray(reinterpret_cast<const char *>(secret), res), QByteArray(reinterpret_cast<char*>(&current), sizeof(current)));


How can you convert a quint64 value (current) into a char*??

A pointer to char is effectively a 64-bit integer on 64-bit systems. I'm not quite sure about the reason for these cast stunts.

Dont forget the ampersand...

That's generally not the problem. I see how these casts work. It's rather that this part is utterly confusing. Both casts are just explicit casts of pointers. Once from a quint8 array to a char array and once from a pointer to quint64 to a pointer to char (why?). I'm really not sure why we don't do it more explicitly: specify the template parameter directly for the QByteArray parameter and use the correct types for the rest right from the start.

Once from a quint8 array to a char array and once from a pointer to quint64 to a pointer to char (why?).

To convert quint64 to QByteArray.

Yeah, but why not use something like QByteArray::number()?

Because it doesn't work that way.

Returns a byte array containing the string equivalent of the number

Also you need to pay attention to endianness here.

Hmm. Okay. Seems like there is really no dedicated interface for converting numbers to byte arrays. Weird. In that case…

droidmonkey · 2017-03-26T15:36:19Z

@weslly I rewrote the base32decode function using QByteArrays. Please consider incorporating this into your next changeset: https://gist.github.com/droidmonkey/091ed6a5b69a5dfe2725a0ae3dd9f1c9

I also included a simple test case.

droidmonkey · 2017-03-27T02:54:52Z

I recommend that once this PR gets finalized that the history be cleaned up. There are a ton of minor commits with very short summaries that can be easily squashed together.

ibrokemypie · 2017-03-27T10:47:20Z

@weslly would you consider adding in support for steam auth codes? keetraytotp has got it working with the totp settings syntax of symply 30;S;http://steampowered.com

weslly · 2017-03-29T21:21:05Z

@ibrokemypie I'll take a look at it as soon as the basic functionality is finished and merged.

TheZ3ro · 2017-04-01T12:17:49Z

News about this? @weslly

Report has been fixed

weslly · 2017-04-03T22:42:36Z

@TheZ3ro I still need a couple of days to finish the fixes from last review.

ArchangeGabriel · 2017-04-05T03:32:12Z

No one answered this comment from @droidmonkey:

With this library are we able to also use it to setup a timed password for the database itself? I could see this "solving" our keylogger issue on entry of the master password since two-level authentication would be required. This would be similar to the Yubikey implementation.

I’m really interested in the answer to this question. Having my KeePassXC further more secured by a TOTP from my phone would be nice. ;)

TheZ3ro · 2017-04-05T14:47:57Z

@ArchangeGabriel I don't think this is possible.
Like we have done with Yubikey: we should use the TOTP secret directly as key-material when encrypt the database file but then you cannot get the secret from a TOTP token. You can't generate a token from the secret if it's used to encrypt the DB (you don't know the secret) and so you can't do it this way.
We discussed this in the Yubikey PR.

Anyway this is unrelated with this PR that add the ability to generato TOTP token for other services

weslly · 2017-04-13T10:56:50Z

Closing this as I'm working on a new branch with some new features and most of the current code was changed already.

I will submit a new PR as soon as I finish.

phoerious requested changes Mar 6, 2017

View reviewed changes

phoerious added the new feature label Mar 6, 2017

phoerious added this to the v2.2.0 milestone Mar 6, 2017

phoerious reviewed Mar 6, 2017

View reviewed changes

louib reviewed Mar 6, 2017

View reviewed changes

TheZ3ro previously requested changes Mar 6, 2017

View reviewed changes

droidmonkey requested changes Mar 6, 2017

View reviewed changes

phoerious requested changes Mar 8, 2017

View reviewed changes

droidmonkey approved these changes Mar 11, 2017

View reviewed changes

droidmonkey requested changes Mar 11, 2017

View reviewed changes

phoerious requested changes Mar 13, 2017

View reviewed changes

rockihack reviewed Mar 13, 2017

View reviewed changes

phoerious requested changes Mar 14, 2017

View reviewed changes

phoerious reviewed Mar 14, 2017

View reviewed changes

droidmonkey previously approved these changes Mar 14, 2017

View reviewed changes

Throne3d reviewed Mar 14, 2017

View reviewed changes

phoerious approved these changes Mar 25, 2017

View reviewed changes

phoerious previously requested changes Mar 25, 2017

View reviewed changes

rockihack reviewed Mar 26, 2017

View reviewed changes

droidmonkey requested changes Mar 26, 2017

View reviewed changes

rockihack reviewed Mar 26, 2017

View reviewed changes

droidmonkey requested changes Mar 26, 2017

View reviewed changes

TheZ3ro requested a review from phoerious April 1, 2017 12:19

Add TOTP support

0af608d

weslly closed this Apr 13, 2017

weslly mentioned this pull request Apr 21, 2017

Add TOTP support #519

Merged

ArchangeGabriel mentioned this pull request Apr 29, 2017

Add QuickUnlock Feature #488

Closed

droidmonkey modified the milestone: v2.2.0 Jun 19, 2017

weslly added the feature: TOTP label Aug 4, 2017

phoerious added pr: new feature Pull request adds a new feature and removed new feature labels Nov 22, 2024

		@@ -325,25 +325,28 @@ QString Entry::totp() const{

		int rc = oath_init();

		}


		QByteArray QTotp::hmacSha1(QByteArray key, QByteArray baseString)

Uh oh!

Conversation

weslly commented Mar 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How it Works

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

droidmonkey commented Mar 6, 2017

Uh oh!

weslly commented Mar 6, 2017

Uh oh!

TheZ3ro commented Mar 6, 2017

Uh oh!

phoerious commented Mar 6, 2017

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phoerious Mar 6, 2017 • 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!

weslly commented Mar 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheZ3ro left a comment

Choose a reason for hiding this comment

Uh oh!

droidmonkey commented Mar 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phoerious Mar 8, 2017 • 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!

phoerious Mar 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

weslly commented Mar 6, 2017 •

edited

Loading

phoerious Mar 6, 2017 •

edited

Loading

weslly commented Mar 6, 2017 •

edited

Loading

droidmonkey commented Mar 6, 2017 •

edited

Loading

phoerious Mar 8, 2017 •

edited

Loading

phoerious Mar 12, 2017 •

edited

Loading

phoerious Mar 13, 2017 •

edited

Loading

phoerious Mar 13, 2017 •

edited

Loading

rockihack Mar 13, 2017 •

edited

Loading

rockihack Mar 13, 2017 •

edited

Loading