New issue
Advanced search Search tips

Issue 792300 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 1
Type: Bug-Regression



Sign in to add a comment

HTTPSCRLSetTest.CRLSetRevokedBySubject fails under Fuchsia

Project Member Reported by w...@chromium.org, Dec 6 2017

Issue description

This new test fails under Fuchsia, possibly because the test runs under Fuchsia while the testserver runs under the host OS. Failure is:

[ RUN      ] HTTPSCRLSetTest.CRLSetRevokedBySubject
../../net/url_request/url_request_unittest.cc:11522: Failure
      Expected: CERT_STATUS_REVOKED
      Which is: 64
To be equal to: cert_status & CERT_STATUS_ALL_ERRORS
      Which is: 0
[  FAILED  ] HTTPSCRLSetTest.CRLSetRevokedBySubject (4156 ms)
 
Cc: w...@chromium.org
Owner: sergeyu@chromium.org
I believe it should work with https://chromium-review.googlesource.com/c/chromium/src/+/810096 .

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1bb6e4589298158b281d1dd07792307df2d3c63

commit a1bb6e4589298158b281d1dd07792307df2d3c63
Author: Wez <wez@chromium.org>
Date: Wed Dec 06 02:27:27 2017

Filter out HTTPSCRLSetTest.CRLSetRevokedBySubject under Fuchsia.

This test is newly-introduced but doesn't work on Fuchsia.

Bug:  792300 
Change-Id: Ice4288e9c5e199ff49e323c0d79470883da0e659
Reviewed-on: https://chromium-review.googlesource.com/809838
Commit-Queue: Wez <wez@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521960}
[modify] https://crrev.com/a1bb6e4589298158b281d1dd07792307df2d3c63/testing/buildbot/filters/fuchsia.net_unittests.filter

Comment 3 by w...@chromium.org, Dec 6 2017

Status: Started (was: Assigned)

Comment 4 by w...@chromium.org, Dec 6 2017

Can you update the test filters in the CL as well?

Comment 5 by agl@google.com, Dec 6 2017

It might be that you have a cert_verify_proc_fuchsia.cc that doesn't have the needed call to CheckSubject.

Comment 6 by w...@chromium.org, Dec 6 2017

Owner: w...@chromium.org
Re #5: Under Fuchsia we're currently using CertVerifyProcBuiltin(), which appears to defer to the revocation_checker.cc implementation:
https://cs.chromium.org/chromium/src/net/cert/internal/revocation_checker.cc?sq=package:chromium&dr=CSs&l=245

That does indeed appear to be missing a call to CheckSubject() - do we probably want to add a bit to RevocationPolicy to control that?
Right, https://chromium-review.googlesource.com/c/chromium/src/+/797678 didn't update the CertVerifyProcBuiltin version of CRLSet enforcement.

And since new tests were added to url_request_unittest.cc (rather than cert_verify_proc_unittest.cc), the test only runs for Fuchsia.

To fix will need to add a call to CheckSubject() in revocation_checker.cc as pointed out in comment #6. Separately I would move (or duplicate) the CRLSet testing into cert_verify_proc_unittest.cc.

> do we probably want to add a bit to RevocationPolicy to control that?

No.

@wez: If you are working on this great! If not, feel free to re-assign to me.
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d1680b09c0c05b5fcc2f5e778f5e252ea6d048a

commit 3d1680b09c0c05b5fcc2f5e778f5e252ea6d048a
Author: Wez <wez@chromium.org>
Date: Thu Dec 07 20:59:57 2017

Add Subject-based check to CheckChainRevocationUsingCRLSet.

CertVerifyProc implementations now include Subject-based revocation
checks against the CRLSet. These checks had been added to the platform-
specific implementations, but not to the built-in revocation checker.

Bug:  792300 
Change-Id: Iba719d7c56835e8c80e723d916ee6e709f505115
Reviewed-on: https://chromium-review.googlesource.com/811988
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522541}
[modify] https://crrev.com/3d1680b09c0c05b5fcc2f5e778f5e252ea6d048a/net/cert/internal/revocation_checker.cc
[modify] https://crrev.com/3d1680b09c0c05b5fcc2f5e778f5e252ea6d048a/testing/buildbot/filters/fuchsia.net_unittests.filter

Comment 9 by w...@chromium.org, Dec 7 2017

Owner: eroman@chromium.org
Status: Assigned (was: Started)
eroman: Assigning this to you, as discussed, for the CertVerifyProc test improvements work.
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee7c8db6b554120fb28385f8759695048f1d8612

commit ee7c8db6b554120fb28385f8759695048f1d8612
Author: Sergey Ulanov <sergeyu@chromium.org>
Date: Fri Dec 08 00:18:39 2017

Implement OCSP server support in RemoteTestServer.

Previously OCSP was broken with RemoteTestServer because OCSP server
port wasn't proxied.
 1. Updated testserver.py to accept --ocsp-proxy-port-number argument
    which makes it possible to generate correct cerificate for the
    proxied connection.
 2. Updated chrome_test_server_spawner.py to forward oscp server port.
 3. Updated RemtoeTestServer to proxy OCSP connection.
 4. Enabled OCSP tests on Fuchsia where they were previously failing.

Bug:  776575 ,  792300 
Change-Id: Iedb1f5b26fc29ad0c73e78f450cd49335a424c18
Reviewed-on: https://chromium-review.googlesource.com/810096
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522634}
[modify] https://crrev.com/ee7c8db6b554120fb28385f8759695048f1d8612/build/util/lib/common/chrome_test_server_spawner.py
[modify] https://crrev.com/ee7c8db6b554120fb28385f8759695048f1d8612/net/test/spawned_test_server/base_test_server.cc
[modify] https://crrev.com/ee7c8db6b554120fb28385f8759695048f1d8612/net/test/spawned_test_server/base_test_server.h
[modify] https://crrev.com/ee7c8db6b554120fb28385f8759695048f1d8612/net/test/spawned_test_server/remote_test_server.cc
[modify] https://crrev.com/ee7c8db6b554120fb28385f8759695048f1d8612/net/test/spawned_test_server/remote_test_server.h
[modify] https://crrev.com/ee7c8db6b554120fb28385f8759695048f1d8612/net/test/spawned_test_server/remote_test_server_proxy.cc
[modify] https://crrev.com/ee7c8db6b554120fb28385f8759695048f1d8612/net/test/spawned_test_server/remote_test_server_proxy.h
[modify] https://crrev.com/ee7c8db6b554120fb28385f8759695048f1d8612/net/test/spawned_test_server/remote_test_server_proxy_unittests.cc
[modify] https://crrev.com/ee7c8db6b554120fb28385f8759695048f1d8612/net/tools/testserver/testserver.py
[modify] https://crrev.com/ee7c8db6b554120fb28385f8759695048f1d8612/net/url_request/url_request_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 10 2018

Owner: w...@chromium.org
Status: Fixed (was: Assigned)
Addressed by wez@ in comment #8.

Regression test added at CertVerifyProc layer in comment #9.

Sign in to add a comment