Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test2043: use revoked.badssl.com instead of revoked.grc.com #5233

Closed
wants to merge 1 commit into from
Closed

test2043: use revoked.badssl.com instead of revoked.grc.com #5233

wants to merge 1 commit into from

Conversation

mback2k
Copy link
Member

@mback2k mback2k commented Apr 13, 2020

The certificate of revoked.grc.com has expired on 2020-04-13.

@mback2k
test2043: use revoked.badssl.com instead of revoked.grc.com
9041b01 
The certificate of revoked.grc.com has expired on 2020-04-13.
@mback2k mback2k added the tests label Apr 13, 2020
@mback2k mback2k requested a review from bagder April 13, 2020 18:38
@mback2k mback2k self-assigned this Apr 13, 2020
@mback2k mback2k requested a review from jay April 13, 2020 18:39
jay
jay approved these changes Apr 13, 2020
View reviewed changes
Copy link
Member

@jay jay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The certificate of revoked.grc.com has expired on 2020-04-13.

I'm sure they'll fix that but the change is fine with me.

@mback2k
Copy link
Member Author

mback2k commented Apr 13, 2020

Let’s wait till tomorrow and if it’s not fixed by then, I will merge this PR.

bagder
bagder reviewed Apr 13, 2020
View reviewed changes
Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we had a local test for this instead of using a live out-of-our-control remote server...

@mback2k
Copy link
Member Author

mback2k commented Apr 13, 2020

We would need to generate a CA including CRL, should be possible statically or dynamically, but would require some extra work on systems there the OS certificate store is used.

@bagder
Copy link
Member

bagder commented Apr 13, 2020

Still, we've at least traditionally have had users run the test suite on machines without internet access, we might be causing undesired traffic to this site and we may at times get undesired test failures.

@jay
Copy link
Member

jay commented Apr 13, 2020 •
edited
Loading

The test as it is now only applies to WinSSL. If the --cacert option is used with a local CA with no revocation information (CRL, AIA, ???) then it should fail unless --ssl-no-revoke is used.

@mback2k
Copy link
Member Author

mback2k commented Apr 13, 2020

@bagder: I am with you. I was completely surprised that this test case relies on some online service. This also leaks information about test suits users to that online service.

@jay
Copy link
Member

jay commented Apr 13, 2020

I did it. At the time there was no way to test --ssl-no-revoke with WinSSL since there was no support for user-specified CAs, so I settled on an internet server.

@mback2k
Copy link
Member Author

mback2k commented Apr 14, 2020

@jay @bagder thanks for your input, but for now it would be okay to merge this to get the test going again, right?

@bagder
Copy link
Member

bagder commented Apr 14, 2020

Yes I'm fine with that.

@mback2k mback2k closed this in a6f7b2f Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bagder bagder bagder left review comments

@jay jay jay approved these changes

Assignees

@mback2k mback2k

Labels
tests
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mback2k @bagder @jay