Support for Certificate Identity Mapping #398

flo-renaud · 2017-01-18T08:31:05Z

See design http://www.freeipa.org/page/V4/Certificate_Identity_Mapping

https://fedorahosted.org/freeipa/ticket/6542

MartinBasti · 2017-01-18T08:56:32Z

install/share/73certmap.ldif

@@ -0,0 +1,17 @@
+## IPA Base OID:
+##
+## Attributes:          2.16.840.1.113730.3.8.22.x


Those OIDs are not registered, please register them

Sure, my plan was to wait for final agreement on the PR before registering the OIDs.

MartinBasti · 2017-01-18T08:59:29Z

ipaserver/plugins/baseuser.py

@@ -18,15 +18,17 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.

 import six
+from ldap.dn import dn2str, str2dn


Would be possible to use ipapython.dn.DN instead of importing ldap.dn ?

Respectively implement reverse() to ipapython.dn.DN if it is not already there

I will implement reverse() in ipapython.dn.DN and add unit tests.

MartinBasti · 2017-01-18T09:05:05Z

ipaserver/plugins/baseuser.py

+    times, or in conjunction with --subject --issuer.
+    """
+    data = []
+    if 'ipacertmapdata' in options:


This can be shorter:

for item in options.get('ipacertmapdata', ()): data.append(item)

MartinBasti · 2017-01-18T09:05:53Z

ipaserver/plugins/baseuser.py

+        subject = options.get('subject')
+        data.append(_build_mapdata(subject, issuer))
+
+    if 'usercertificate' in options:


you can use get(key, default) too here to save one level of indentation

MartinBasti · 2017-01-18T09:08:50Z

ipaserver/plugins/baseuser.py

+        # --certificate
+        # --data
+        # Check that at least one of the 3 formats is used
+        if 'issuer' not in options and \


this is ugly :)

more pythonic way is to use all()

# from top of my head untested if all(key not in options for key in ('issuer', subject', ....)):

MartinBasti · 2017-01-18T09:09:43Z

ipaserver/plugins/baseuser.py

+
+    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
+                     **options):
+        if 'issuer' not in options and \


all() can be use here too

MartinBasti · 2017-01-18T09:20:06Z

ipaserver/plugins/certmap.py

+            'default_privileges': {
+                'Certificate Identity Mapping Administrators'},
+        },
+        'System: Read Certmap Rules': {


I have a question related to ACI, if System: Read Certmap Configuration is allowed for ldap://all, shouldn't be this System: Read Certmap Rules accessible to ldap://all? What/who is the consumer of this rules? SSSD?

You are right. SSSD will be the consumer of the rules and there's no point in hiding the rules.

MartinBasti · 2017-01-18T09:28:28Z

ipaserver/plugins/user.py

@@ -366,6 +368,13 @@ class user(baseuser):
            },
            'default_privileges': {'PassSync Service'},
        },
+        'System: Manage User Certificate Mappings': {


IMHO the similar permission should be added to stageuser implementation, something like System: Manage Stage User Certificate Mappings

The permissions for Users are finer-grained than the permissions for stage users. For instance, for users, there is "System: Modify Users" (which does not grant write on usercertificate) and "System: Manage User Certificates".
For stage user, there is only "System: Modify stage User" which applies to all the attributes.

I applied the same strategy (finer-grained for users) but if it is a mistake I can create a similar permission for cert mapping on stage users.

MartinBasti · 2017-01-18T09:38:35Z

I put some inline commets, @flo-renaud if you don't know where to register OIDs feel free to ping me

MartinBasti · 2017-01-18T17:19:07Z

ipaserver/plugins/baseuser.py

+    Converts a (ipa) DN into a string representation following X500 order
+    """
+    if name:
+        name.reverse()


I'm always scared by in-place methods. Do you count with that the object where the original name reference was pointing to, changed also outside the _convert_to_x500 method?

maybe we should rather implement get_reverse() method that returns a new DN and keeps the original reference untouched

HonzaCholasta · 2017-02-07T07:44:55Z

ipapython/dn.py

+        '''
+        reverse_dn = DN(self)
+        reverse_dn.rdns.reverse()
+        return reverse_dn


There is no such thing as reversed DN - it's only the the textural representation which might be in both directions, but the underlying DN is the same. I would prefer if you instead added two new methods ldap_text() and x500_text() which return textual representation of the DN in the LDAP and X.500 order, respectively, and use them as appropriate:

def ldap_text(self): return dn2str([[self.to_openldap()]]) def x500_text(self): return dn2str([[reversed(self.to_openldap()]]) def __str__(self): return self.ldap_text()

HonzaCholasta · 2017-02-07T07:45:58Z

ipaserver/plugins/baseuser.py

@@ -360,6 +362,13 @@ class baseuser(LDAPObject):
            label=_('Certificate'),
            doc=_('Base-64 encoded user certificate'),
        ),
+        Str(
+            'ipacertmapdata*',
+            cli_name='data',


The CLI name should be certmapdata, data is far too generic and disconnected from certificate mapping.

HonzaCholasta · 2017-02-07T07:49:27Z

ipaserver/plugins/baseuser.py

+        subject = DN(cert.subject)
+        data.append(_build_mapdata(subject, issuer))
+
+    return data


Instead of a bunch of global definitions used in both baseuser_add_certmap and baseuser_remove_certmap, create a base class (e.g. ModCertMapData) which baseuser_add_certmap and baseuser_remove_certmap inherit from and encapsulate the common stuff inside it.

By "common stuff" I meant certmap_options and _convert_options_to_certmap as well.

HonzaCholasta · 2017-02-07T07:51:08Z

ipaserver/plugins/baseuser.py

+    return data
+
+
+class baseuser_add_certmap(LDAPAddAttributeViaOption):


This should be called baseuser_add_certmapdata, because the attribute is called certmapdata (actually it is called data now, but see my comment above). Ditto for baseuser_remove_certmap.

HonzaCholasta · 2017-02-07T07:54:47Z

ipaserver/plugins/baseuser.py

+        # ipacertmapdata is not mandatory as it can be built
+        # from the values subject+issuer or from reading usercertificate
+        for option in super(baseuser_add_certmap, self).get_options():
+            if option.name in ['ipacertmapdata']:


I think option.name == 'ipacertmapdata' would be preferrable.

HonzaCholasta · 2017-02-07T07:58:47Z

ipaserver/plugins/baseuser.py

+            entry_attrs_old = ldap.get_entry(dn, ['objectclass'])
+        objclasses_lc = [x.lower() for x in entry_attrs_old['objectclass']]
+        if 'ipacertmapobject' not in objclasses_lc:
+            entry_attrs['objectclass'] = ['ipacertmapobject']


Overwriting the objectclass is wrong and won't work, you should append ipacertmapobject to the existing value. Or, you could use baseldap.add_missing_object_class() which does all of this for you.

HonzaCholasta · 2017-02-07T08:01:41Z

ipaserver/plugins/baseuser.py

+        if 'ipacertmapobject' not in objclasses_lc:
+            entry_attrs['objectclass'] = ['ipacertmapobject']
+
+        entry_attrs[self.attribute] = _convert_options_to_certmap(options)


IMO it would be better to implement this using default_from on the ipacertmapdata, issuer and subject parameters.

I initially considered using default_from, but it would be able to build ipacertmapdata only if both issuer and subject parameters are present (when one key is missing, it does not get called). The ipacertmapdata value can also be extracted from the certificate parameter, and I did not find any method in the framework allowing to take input either from (subject+issuer) or certificate. Would you have any suggestion?

HonzaCholasta · 2017-02-07T08:18:32Z

ipaserver/plugins/certmap.py

+                  ' authentication'),
+        ),
+        Str(
+            'associateddomain*',


I think this should be a DNSNameParam.

flo-renaud · 2017-02-14T11:59:11Z

Hi @HonzaCholasta,
PR updated with most of your comments, except the suggestion to use default_from. Please see my answer inline for this one.

HonzaCholasta · 2017-02-14T13:57:42Z

@flo-renaud, nevermind the default_from suggestion, I was wrong - if e.g. both --certmapdata and --certificate are specified, we want to use both, not throw away --certificate, which is exactly what would happen if --certmapdata had default derived from --certificate.

One more issue, I think the --certmapdata option in user-add-certmapdata and friends should actually be a positional argument, as that would be more consistent with existing commands. The common pattern is that positional arguments are used to specify the literal value of the attribute (such as principal name in user-add-principal), but options need some preprocessing (such as conversion from UID to DN in group-add-member). Currently the only exception to this scheme is user-add-cert and friends, but that's only because the original intent was to add a certificate file positional argument, but it never happened.

flo-renaud · 2017-02-14T18:41:53Z

Hi @HonzaCholasta
PR updated with ipa user-add-certmapdata using positional arg for CERTMAPDATA

HonzaCholasta · 2017-02-15T08:15:10Z

install/updates/73-certmap.update

+default:objectClass: groupofnames
+default:objectClass: nestedgroup
+default:cn: Certificate Identity Mapping Administrators
+default:description: Certificate Identity Mapping Administrators


Note that we usually put new privileges into 40-delegation.update. I'm not saying it's wrong to have it here, just curious why have you done it this way.

Hi @HonzaCholasta
I simply had a look at install/updates/60-trusts.update and did the same, i.e. define new containers and privileges in the same file.
I will keep your note for future reference though.

HonzaCholasta · 2017-02-15T08:17:11Z

ipaserver/plugins/baseuser.py

+            cli_name='issuer',
+            label=_('Issuer'),
+            doc=_('Issuer of the certificate'),
+            flags=['virtual_attribute', 'no_create', 'no_update']


Note that the no_create and no_update flags are meaningless outside of object plugin params.

HonzaCholasta · 2017-02-15T08:19:22Z

ipaserver/plugins/baseuser.py

+        return u'X509:{issuer}{subject}'.format(
+            issuer='<I>{}'.format(issuer.x500_text()) if issuer else '',
+            subject='<S>{}'.format(subject.x500_text()) if subject else '',
+            )


Why not just:

return u'X509:<I>{issuer}<S>{subject}'.format(issuer=issuer, subject=subject)

?

Hi,
issuer or subject can be None. If for instance issuer=None we should return X509:<S>cn=subject but not X509:<I>None<S>cn=subject

When they can be None? I don't see when this can be useful.

Hi @HonzaCholasta
for instance if the user calls ipa user-add-certmapdata LOGIN --subject cn=subject then issuer=None.

I get that, but what is the use case? The subject name is meaningless without context, and the issuer name provides that context. The only use case I can think of is that there is only a single CA (forever), so the issuer name is implied, but I believe that's a corner case which does not need to have special support outside of the generic ipa user-add-certmapdata LOGIN VALUE.

@HonzaCholasta
I'm afraid I disagree here. AD supports both (see Map a Certificate to a User Account) and it is easy for us to provide a function that supports only issuer / only subject / issuer+subject, so why make it more difficult for the administrator? It is more user-friendly to use the issuer or subject options rather than specify the format X509:....

Because it has additional costs for us (in maintenance, testing, ...) for very questionable benefit. Nobody requested it and I seriously doubt anybody would. What does it matter if it's user friendly when there's no user to use it that way?

AD does not support issuer-only AFAICT as it is non-sensical, but it supports other variants which this PR does not implement, so clearly supporting everything AD supports is not the goal here, and I don't think it ever was in the first place.

HonzaCholasta · 2017-02-15T08:35:20Z

ipaserver/plugins/baseuser.py

+        # certificate
+        if len(keys) < 2:
+            valid_options = ('issuer', 'subject', 'usercertificate')
+            if all(key not in options for key in valid_options):


I'm pretty sure just checking for presence of the options is not good enough, you should also check that none of the options are None. IMO the easiest way to achieve this would be to add this at the beginning of pre_callback:

try: certmapdatas = args[1] or [] except IndexError: certmapdatas = [] issuer = options.get('issuer') subject = options.get('subject') certificates = options.get('usercertificate') or []

and work with these variables further in the code.

HonzaCholasta · 2017-02-15T08:37:22Z

ipaserver/plugins/baseuser.py

+        if 'issuer' in options or 'subject' in options:
+            issuer = options.get('issuer')
+            subject = options.get('subject')
+            data.append(ModCertMapData._build_mapdata(subject, issuer))


IMO it would be better to declare _convert_options_to_certmap() as class method, so you can cls._build_mapdata() instead of ModCertMapData._build_mapdata().

HonzaCholasta · 2017-02-15T08:40:53Z

ipaserver/plugins/baseuser.py

+
+        data = entry_attrs.get(self.attribute, list())
+        entry_attrs[self.attribute] = self._convert_options_to_certmap(
+            data, options)


IMO it would be better to hide all this inside _convert_options_to_certmap() and have it called like this here:

self._convert_options_to_certmap(entry_attrs, options)

HonzaCholasta · 2017-02-15T08:45:14Z

ipaserver/plugins/baseuser.py

+        ),
+        Bytes(
+            'usercertificate*', validate_certificate,
+            cli_name='certificate',


Actually, can we name the param certificate instead of usercertificate? It does not refer to the LDAP attribute after all. Speaking of that, it should have the virtual_attribute flag.

HonzaCholasta · 2017-02-15T11:44:00Z

ipaserver/plugins/certmap.py

+    __doc__ = _('Modify a Certificate Identity Mapping Rule.')
+
+    msg_summary = _('Modified Certificate Identity Mapping Rule "%(value)s"')
+


I think we should validate that associateddomain is either the IPA domain or one of the trusted domains in certmaprule_add and certmaprule_mod.

@HonzaCholasta
I discussed this point offline with Sumit and his answer was that any domain name should be accepted. If SSSD cannot make sense of one of the domains it will ignore it and log a message

I can't say I agree, IMHO it does not make sense to allow rules for domains not related to IPA in any way. Also, with validation, we would be able to catch typos.

flo-renaud · 2017-02-15T15:03:13Z

@HonzaCholasta
PR updated according to your comments. Thanks for the detailed review!

flo-renaud · 2017-02-15T21:56:43Z

PR updated with the check on domain in certmaprule-add/mod.

HonzaCholasta · 2017-02-20T08:45:48Z

install/updates/73-certmap.update

+default:objectclass: ipaCertMapConfigObject
+default:cn: certmap
+default:ipaconfigstring: CertMapVersion 1
+default:ipacertmapversion: 1


What is the point of the version? Why is it specified using both ipaConfigString and ipaCertMapVersion?

HonzaCholasta · 2017-02-20T08:53:10Z

install/updates/73-certmap.update

+default:objectclass: nsContainer
+default:objectclass: ipaConfigObject
+default:objectclass: ipaCertMapContainer
+default:objectclass: ipaCertMapConfigObject


You can't have 2 structural object classes (nsContainer and ipaCertMapConfigObject) in an entry. RFC 4512, section 2.4.2 says:

An object or alias entry is characterized by precisely one structural object class superclass chain which has a single structural object class as the most subordinate object class. This structural object class is referred to as the structural object class of the entry.

I would define ipaCertMapConfigObject as auxiliary to fix this, because IMO all containers should have structural object class nsContainer.

HonzaCholasta · 2017-02-20T09:31:06Z

ipaserver/plugins/baseuser.py

+        if not certmapdatas:
+            valid_options = (issuer, subject, certificates)
+            if all(not option for option in valid_options):
+                raise errors.RequirementError(name='CERTMAPDATA')


Nitpick, but IMO this would be a little bit more readable:

if not certmapdatas and not issuer and not subject and not certificates: raise errors.RequirementError(name='ipacertmapdata')

(BTW you should use the name of the param in RequirementError, like above.)

HonzaCholasta · 2017-02-20T09:34:53Z

ipaserver/plugins/baseuser.py

+        if 'ipacertmapdata' not in attrs_list:
+            attrs_list.append('ipacertmapdata')
+
+        self._convert_options_to_certmap(entry_attrs, options)


Nitpick, but I think it would be better to pass certmapdatas, issuer, subject and certificates as arguments rather than options, so you can avoid the redundant options.get() etc. in _convert_options_to_certmap().

HonzaCholasta · 2017-02-20T09:38:31Z

ipaserver/plugins/certmap.py

+        trust_suffix_namespace = set()
+        trust_suffix_namespace.add(api_inst.env.domain.lower())
+
+        trust_objects = api_inst.Command.trust_find(u'', sizelimit=0)['result']


Is the empty string necessary? It shouldn't be.

HonzaCholasta · 2017-02-20T09:41:20Z

ipaserver/plugins/certmap.py

+    def get_dn(self, *keys, **options):
+        rulename = keys[-1]
+        dn = super(certmaprule, self).get_dn(rulename, **options)
+        return dn


It looks like this method does effectively nothing. What is its purpose?

HonzaCholasta · 2017-02-20T10:36:34Z

ipaserver/plugins/baseuser.py

+            flags=['virtual_attribute']
+        ),
+        Bytes(
+            'certificate*', validate_certificate,


Outside of basic certificate validation, we should also validate that the certificate has non-empty subject, otherwise bad certmap data would be generated. This can be fixed later though.

HonzaCholasta · 2017-02-21T06:32:15Z

install/updates/73-certmap.update

+dn: cn=certmap,cn=ipa,cn=etc,$SUFFIX
+default:objectclass: top
+default:objectclass: nsContainer
+default:objectclass: ipaConfigObject


Now that the ipaConfigString attribute is gone, ipaConfigObject is not necessary anymore. This can be fixed later though.

MartinBasti · 2017-02-21T11:09:15Z

ipaserver/plugins/baseuser.py

+
+    @classmethod
+    def _convert_options_to_certmap(cls, entry_attrs, issuer=None,
+                                    subject=None, certificates=[]):


Please don't use mutable python types as default value in options. Yous should use certificates=()

I know that this is on many places in code but we are trying to get rid of it.

HonzaCholasta · 2017-02-21T12:57:33Z

LGTM. @flo-renaud, don't forget to register the new OIDs.

See design http://www.freeipa.org/page/V4/Certificate_Identity_Mapping https://fedorahosted.org/freeipa/ticket/6542

sumit-bose · 2017-02-22T20:12:20Z

It looks like the ACis on the latest version do not allow hosts to access the rules. When I do 'kinit -k' on the IPA server or a client and call

ldapsearch -H ldap://ipa-server.ipa.devel '(&(objectClass=ipaCertMapRule)(ipaEnabledFlag=TRUE))' -Y GSSAPI

I do not get any results. When I call 'kinit admin' and use the same ldapsearch I get my rule returned. Can you confirm this or is my test system broken?

flo-renaud · 2017-02-22T20:37:15Z

Hi @sumit-bose ,
I am not able to reproduce this issue:
`[root@vm-161 ~]# kinit -k
[root@vm-161 ~]# klist
Ticket cache: KEYRING:persistent:0:krb_ccache_h6XRpeK
Default principal: host/vm-161.example.com@DOM-161.EXAMPLE.COM

Valid starting Expires Service principal
02/22/2017 21:30:10 02/23/2017 21:30:10 krbtgt/DOM-161.EXAMPLE.COM@DOM-161.EXAMPLE.COM
[root@vm-161 ~]# ldapsearch -H ldap://vm-161 '(&(objectClass=ipaCertMapRule)(ipaEnabledFlag=TRUE))' -Y GSSAPI -LLL
SASL/GSSAPI authentication started
SASL username: host/vm-161.example.com@DOM-161.EXAMPLE.COM
SASL SSF: 56
SASL data security layer installed.
dn: cn=rule1,cn=certmaprules,cn=certmap,dc=dom-161,dc=example,dc=com
objectClass: ipacertmaprule
objectClass: top
cn: rule1
description: d1
ipaEnabledFlag: TRUE
`
Do you have the ACI "permission:System: Read Certmap Rules" defined on dn: cn=certmaprules,cn=certmap,$BASEDN? It should grant access to ldap:///all

sumit-bose · 2017-02-23T09:24:08Z

Ok, sorry for the noise, I tested on a fresh install again and now it is working as expected. I guess I shouldn't have tried to update from an older version of your patch to a newer one.

ghost · 2017-03-01T10:02:28Z

Works for me.

ghost · 2017-03-02T14:10:13Z

master:

9e24918 Support for Certificate Identity Mapping

MartinBasti suggested changes Jan 18, 2017

View reviewed changes

flo-renaud force-pushed the dmdc branch from 3ccb98a to 068cbf8 Compare January 18, 2017 16:54

MartinBasti suggested changes Jan 18, 2017

View reviewed changes

flo-renaud force-pushed the dmdc branch from 068cbf8 to 65b1b06 Compare January 19, 2017 10:05

MartinBasti approved these changes Jan 19, 2017

View reviewed changes

HonzaCholasta reviewed Feb 7, 2017

View reviewed changes

flo-renaud force-pushed the dmdc branch 3 times, most recently from 5a6a88d to 8e9eeb0 Compare February 13, 2017 18:45

flo-renaud force-pushed the dmdc branch 2 times, most recently from e8a0293 to f26952b Compare February 14, 2017 18:40

HonzaCholasta reviewed Feb 15, 2017

View reviewed changes

flo-renaud force-pushed the dmdc branch from f26952b to ca1e31f Compare February 15, 2017 15:01

flo-renaud force-pushed the dmdc branch from ca1e31f to ab4f03f Compare February 15, 2017 21:51

HonzaCholasta reviewed Feb 20, 2017

View reviewed changes

flo-renaud force-pushed the dmdc branch from ab4f03f to 48a5dbb Compare February 20, 2017 14:22

HonzaCholasta reviewed Feb 21, 2017

View reviewed changes

MartinBasti reviewed Feb 21, 2017

View reviewed changes

flo-renaud force-pushed the dmdc branch from 48a5dbb to 9bc13ef Compare February 21, 2017 12:44

flo-renaud force-pushed the dmdc branch from 9bc13ef to 12234be Compare February 21, 2017 13:52

Support for Certificate Identity Mapping

05f93e1

See design http://www.freeipa.org/page/V4/Certificate_Identity_Mapping https://fedorahosted.org/freeipa/ticket/6542

flo-renaud force-pushed the dmdc branch from 12234be to 05f93e1 Compare February 21, 2017 14:18

ghost self-assigned this Mar 1, 2017

ghost added ack Pull Request approved, can be merged pushed Pull Request has already been pushed labels Mar 1, 2017

ghost closed this Mar 2, 2017

pvomacka mentioned this pull request Mar 7, 2017

WebUI: certmap match #549

Closed

flo-renaud deleted the dmdc branch March 14, 2017 07:46

This pull request was closed.

		return data


		class baseuser_add_certmap(LDAPAddAttributeViaOption):

		__doc__ = _('Modify a Certificate Identity Mapping Rule.')

		msg_summary = _('Modified Certificate Identity Mapping Rule "%(value)s"')

Support for Certificate Identity Mapping #398

Support for Certificate Identity Mapping #398

Conversation

flo-renaud commented Jan 18, 2017 • edited by ghost

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinBasti commented Jan 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HonzaCholasta Feb 14, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flo-renaud commented Feb 14, 2017

HonzaCholasta commented Feb 14, 2017

flo-renaud commented Feb 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flo-renaud Feb 15, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flo-renaud commented Feb 15, 2017

flo-renaud commented Feb 15, 2017

HonzaCholasta Feb 20, 2017 • edited

Choose a reason for hiding this comment

HonzaCholasta Feb 20, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HonzaCholasta Feb 20, 2017 • edited

Choose a reason for hiding this comment

HonzaCholasta Feb 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HonzaCholasta commented Feb 21, 2017

sumit-bose commented Feb 22, 2017

flo-renaud commented Feb 22, 2017

sumit-bose commented Feb 23, 2017

ghost commented Mar 1, 2017

ghost commented Mar 2, 2017

flo-renaud commented Jan 18, 2017 •

edited by ghost

HonzaCholasta Feb 14, 2017 •

edited

flo-renaud Feb 15, 2017 •

edited

HonzaCholasta Feb 20, 2017 •

edited

HonzaCholasta Feb 20, 2017 •

edited

HonzaCholasta Feb 20, 2017 •

edited

HonzaCholasta Feb 21, 2017 •

edited