Comment 3 Henrik Andersson 2016-06-17 12:40:32 CEST

Tester should verify that:

 - ...our buildserver builds using version libtasn1 4.8
 - ...tl-crl-tool and tl-certtool works as expected
 - ...tlclient.bin work as expected when using smart card

Comment 4 Henrik Andersson 2016-06-17 12:42:19 CEST

(In reply to comment #3)
> Tester should verify that:
> 
>  - ...our buildserver builds using version libtasn1 4.8
>  - ...tl-crl-tool and tl-certtool works as expected
>  - ...tlclient.bin work as expected when using smart card

Consider to test on armhf and i686 archs and not x86_64 which dev. tested.

Comment 5 Pierre Ossman 2016-06-20 13:27:14 CEST

Chavez is indeed updated.
Tested smart card auth in tlclient on x86_64 and win32.
Tested i686 tl-certtool and tl-crltool.

However, tl-crltool has taken a nose-dive when it comes to performance. It now takes more than twice as long to process things compared to 4.6.0. i686 or x86_64 doesn't seem to matter.

(tl-certtool is probably affected as well, but it's so fast that you cannot really measure it)

Comment 6 Pierre Ossman 2016-06-20 14:40:55 CEST

Found the issue. Upstream has reverted an optimisation, thinking it doesn't have any effect:

commit 62cb6fab75ee9a4fc2925104896980d40862a89f
Author: Nikos Mavrogiannopoulos <nmav@gnutls.org>
Date:   Sun Apr 3 10:10:49 2016 +0200

    Revert "optimized _asn1_find_up()."
    
    This reverts commit 4010bb04588fca86a9f6d683b637c05b4cec24e0.
    This optimization did not offer much benefit and there may be
    corner cases in the internal structure handling that may not
    be possibly to handle with this optimization.

Before this commit: (56b9f13cff50861bc0660dcf5b8d4a41c2bd5289)

~/devel/ctc/tlmisc
[ossman@ossman]$ cat ~/devel/ctc/tlmisc/ca003.crl | time ./tl-crltool --update/tmp/pkix.asn.NG1b5k:343: Warning: VisibleString is a built-in ASN.1 type.
/tmp/pkix.asn.NG1b5k:345: Warning: NumericString is a built-in ASN.1 type.
/tmp/pkix.asn.NG1b5k:347: Warning: IA5String is a built-in ASN.1 type.
/tmp/pkix.asn.NG1b5k:349: Warning: TeletexString is a built-in ASN.1 type.
/tmp/pkix.asn.NG1b5k:351: Warning: PrintableString is a built-in ASN.1 type.
/tmp/pkix.asn.NG1b5k:353: Warning: UniversalString is a built-in ASN.1 type.
/tmp/pkix.asn.NG1b5k:356: Warning: BMPString is a built-in ASN.1 type.
/tmp/pkix.asn.NG1b5k:360: Warning: UTF8String is a built-in ASN.1 type.
110621080035Z 110622080047Z
14.62user 0.06system 0:14.69elapsed 99%CPU (0avgtext+0avgdata 175820maxresident)k
0inputs+0outputs (0major+43654minor)pagefaults 0swaps

After this commit: (80accf6e21259e2f97e62dc6d557f47ad6f695fc)

~/devel/ctc/tlmisc
[ossman@ossman]$ cat ~/devel/ctc/tlmisc/ca003.crl | time ./tl-crltool --update/tmp/pkix.asn.gzgG2t:343: Warning: VisibleString is a built-in ASN.1 type.
/tmp/pkix.asn.gzgG2t:345: Warning: NumericString is a built-in ASN.1 type.
/tmp/pkix.asn.gzgG2t:347: Warning: IA5String is a built-in ASN.1 type.
/tmp/pkix.asn.gzgG2t:349: Warning: TeletexString is a built-in ASN.1 type.
/tmp/pkix.asn.gzgG2t:351: Warning: PrintableString is a built-in ASN.1 type.
/tmp/pkix.asn.gzgG2t:353: Warning: UniversalString is a built-in ASN.1 type.
/tmp/pkix.asn.gzgG2t:356: Warning: BMPString is a built-in ASN.1 type.
/tmp/pkix.asn.gzgG2t:360: Warning: UTF8String is a built-in ASN.1 type.
110621080035Z 110622080047Z
61.71user 0.09system 1:01.81elapsed 99%CPU (0avgtext+0avgdata 160464maxresident)k
0inputs+0outputs (0major+39838minor)pagefaults 0swaps


We should probably inform upstream that it was a very useful optimisation.

Comment 7 Henrik Andersson 2016-06-20 15:44:22 CEST

(In reply to comment #6)

> 
> We should probably inform upstream that it was a very useful optimisation.

I have sent a mail with information to their mailing list.

Comment 8 Henrik Andersson 2016-06-23 12:27:42 CEST

(In reply to comment #7)
> (In reply to comment #6)
> 
> > 
> > We should probably inform upstream that it was a very useful optimisation.
> 
> I have sent a mail with information to their mailing list.

Hi,
 The reason I dropped the _asn1_find_up optimization is because I
wasn't convinced that there were not any unintended side-effects from
it (e.g, when modifying a structure) that could lead to crash. I had
no time to verify that so I decided to drop it instead. However, if
you could make a convincing point that this optimization doesn't cause
any side-effects on read-write structure or can provide a better fix,
I'm all for including it in a future release.

regards,
Nikos

Comment 9 Pierre Ossman 2016-06-28 13:01:39 CEST

We've decided to accept the performance regression so that we can stay in sync with upstream.

Comment 10 Pierre Ossman 2016-06-28 13:02:08 CEST

Already tested on comment 5.