New issue
Advanced search Search tips
Starred by 4 users
Status: Fixed
Owner:
Closed: Sep 2015
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug-Regression



Sign in to add a comment
Unable to use client certificate with MSB=0 bad encoding
Reported by jaan.mur...@gmail.com, Sep 22 2015 Back to list
UserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.33 Safari/537.36

Example URL:

Steps to reproduce the problem:
Steps to reproduce the problem:
1. Chrome 46
2. Estonian ID-card
3. Try to authenticate to testsite for example https://sk.ee/tervitus
4. Authentication fails: certificate selection dialog is displayed but after that it fails without pin entry with error ERR_SSL_PROTOCOL_ERROR
5. Boringssl internal error is BAD_ENCODING.

What is the expected behavior?
Authentication should be success

What went wrong?
https://boringssl.googlesource.com/boringssl/+/c0e245a546b15c6b4219d2f3d5455e417cddc782

Did this work before? Yes chrome 45

Chrome version: 46.0.2490.33  Channel: beta
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: Shockwave Flash 19.0 r0

During the 
https://code.google.com/p/chromium/issues/detail?id=532048 investigation we found this issue.

There is almost same amount of certificates affected as in ticket 532048. 
The temporary workaround should be based on similar grounds as in ticket 532048.
 
37101010021.cer
1.8 KB Download
Labels: -Cr-Internals-Network Cr-Internals-Network-SSL
Labels: -Type-Bug Type-Bug-Regression M-46
Owner: davidben@chromium.org
Status: Started
Fun times. Alright, here's another workaround. Please confirm that newly-issued IDs no longer have this issue and that these too can be resolved in six months.

https://boringssl-review.googlesource.com/#/c/5980/
Project Member Comment 3 by bugdroid1@chromium.org, Sep 23 2015
The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl.git/+/4c60d356a926b68ac20e8090755799e1525874f0

commit 4c60d356a926b68ac20e8090755799e1525874f0
Author: David Benjamin <davidben@chromium.org>
Date: Wed Sep 23 16:23:01 2015

Work around even more Estonian ID card misissuances.

Not content with signing negative RSA moduli, still other Estonian IDs have too
many leading zeros. Work around those too.

This workaround will be removed in six months.

BUG= 534766 

Change-Id: Ica23b1b1499f9dbe39e94cf7b540900860e8e135
Reviewed-on: https://boringssl-review.googlesource.com/5980
Reviewed-by: Adam Langley <agl@google.com>

[modify] http://crrev.com/4c60d356a926b68ac20e8090755799e1525874f0/crypto/bn/bn_asn1.c
[modify] http://crrev.com/4c60d356a926b68ac20e8090755799e1525874f0/crypto/bn/bn_test.cc
[modify] http://crrev.com/4c60d356a926b68ac20e8090755799e1525874f0/crypto/evp/p_rsa_asn1.c
[modify] http://crrev.com/4c60d356a926b68ac20e8090755799e1525874f0/crypto/rsa/rsa_asn1.c
[modify] http://crrev.com/4c60d356a926b68ac20e8090755799e1525874f0/include/openssl/bn.h
[modify] http://crrev.com/4c60d356a926b68ac20e8090755799e1525874f0/include/openssl/rsa.h

Yeah, fun indeed :) Thanks for quick response.
New ones are ok and 6 months planned to resolve.
Will look into workaround.
Confirmed - workaround works for certificates in focus. Please continue to merge in M46
Er, how did you test this? Rolling the change into Chromium is still making its way through the system. I'm not going to request a merge until probably tomorrow when the change has hit the canary channel and no problems have come up.
We checked out latest bssl and modified tool.cc to check those certificates against bssl.
Usage: tool/bssl [client|genrsa|md5sum|pkcs12|rand|s_client|s_server|server|sha1sum|sha224sum|sha256sum|sha384sum|sha512sum|speed]
Path /../37101010021.cer
Cert OK
PKEY OK

Project Member Comment 8 by bugdroid1@chromium.org, Sep 24 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5fe77ecea369e669d76b067c42334f9382b441b

commit b5fe77ecea369e669d76b067c42334f9382b441b
Author: davidben <davidben@chromium.org>
Date: Thu Sep 24 19:44:17 2015

Roll src/third_party/boringssl/src 231cb8214..4c60d356a

https://boringssl.googlesource.com/boringssl/+log/231cb8214511ea5784dd94a59f3e5f5fb7d39f8e..4c60d356a926b68ac20e8090755799e1525874f0

This rolls just before the signing digest changes to make
sure the new Estonia workaround gets into the next canary.

BUG= 534766 

Review URL: https://codereview.chromium.org/1367613005

Cr-Commit-Position: refs/heads/master@{#350619}

[modify] http://crrev.com/b5fe77ecea369e669d76b067c42334f9382b441b/DEPS

Labels: Merge-Request-46
Sorry TPMs, there's another one of these. :-( Apparently Estonian IDs managed to screw something else up too, so we have to work around this bug as well. And, of course, it was only reported the day after I'd already merged the other workaround.

I'd like to merge https://boringssl.googlesource.com/boringssl.git/+/4c60d356a926b68ac20e8090755799e1525874f0%5E%21/ to M46. The only non-comment, non-test change is removal of a block of code in crypto/bn/bn_asn1.c.

This is a funny DEPS change, so it would amount to moving boringssl_revision to 8f7f3837b88197e571159b84efdaa8ad23712a99 which is a cherry-pick of the change onto where Chromium branched BoringSSL:
https://boringssl.googlesource.com/boringssl/+/2490
Comment 10 by tin...@google.com, Sep 25 2015
Labels: -Merge-Request-46 Merge-Review-46 Hotlist-Merge-Review
[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.
Comment 11 by tin...@google.com, Sep 25 2015
Labels: -Merge-Review-46 Merge-Approved-46
Merge approved for M46 branch (branch: 2490).
Project Member Comment 12 by bugdroid1@chromium.org, Sep 25 2015
Labels: -Merge-Approved-46 merge-merged-2490
The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=78887

------------------------------------------------------------------
r78887 | davidben@google.com | 2015-09-25T18:07:29.665615Z

-----------------------------------------------------------------
Status: Fixed
Project Member Comment 14 by bugdroid1@chromium.org, May 10
The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl/+/1d59f6e3e2f3acef7498635adad4de0540433f72

commit 1d59f6e3e2f3acef7498635adad4de0540433f72
Author: David Benjamin <davidben@google.com>
Date: Wed May 10 15:49:33 2017

Add a flag to toggle the buggy RSA parser.

It's about time we got rid of this. As a first step, introduce a flag,
so that some consumers may stage this change in appropriately.

BUG= chromium:534766 , chromium:532048 

Change-Id: Id53f0bacf5bdbf85dd71d1262d9f3a9ce3c4111f
Reviewed-on: https://boringssl-review.googlesource.com/16104
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

[modify] https://crrev.com/1d59f6e3e2f3acef7498635adad4de0540433f72/include/openssl/evp.h
[modify] https://crrev.com/1d59f6e3e2f3acef7498635adad4de0540433f72/crypto/evp/p_rsa_asn1.c

Sign in to add a comment