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.

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

add OCSP status worker #336

Merged
merged 11 commits into from
May 8, 2018
Merged

add OCSP status worker #336

merged 11 commits into from
May 8, 2018

Conversation

adamdecaf
Copy link
Contributor

This builds on #286 and takes the work done there to finish the worker.

Issue: #199

--- Analyzers ---
* CAA records: not found
* CRL: Not Revoked
* Mozilla evaluation: non compliant
  - for modern level: remove ciphersuites AES128-GCM-SHA256, AES128-SHA256, DHE-RSA-AES128-GCM-SHA256, DHE-RSA-AES128-SHA256, AES128-SHA, ECDHE-RSA-AES128-SHA, DHE-RSA-AES128-SHA, AES256-GCM-SHA384, AES256-SHA256, DHE-RSA-AES256-GCM-SHA384, DHE-RSA-AES256-SHA256, AES256-SHA, ECDHE-RSA-AES256-SHA, DHE-RSA-AES256-SHA, DES-CBC3-SHA, EDH-RSA-DES-CBC3-SHA
  - for modern level: consider adding ciphers ECDHE-ECDSA-AES256-GCM-SHA384, ECDHE-ECDSA-CHACHA20-POLY1305, ECDHE-RSA-CHACHA20-POLY1305, ECDHE-ECDSA-AES128-GCM-SHA256, ECDHE-ECDSA-AES256-SHA384, ECDHE-ECDSA-AES128-SHA256
  - for modern level: remove protocols SSLv3, TLSv1, TLSv1.1
  - for modern level: enable Perfect Forward Secrecy with a curve of at least 256bits, don't use DHE
  - for modern level: use a certificate of type ecdsa, not RSA
  - oldest clients: 
* Grade: A (90/100)
* OCSP Status: 
 - Not revoked
* SSLLabs Client Support: showing oldest known clients
  - Firefox 10.0.12 ESR, Chrome 27, Edge 12, IE 6, Safari 5, Opera 12.15, Android 2.3.7, OpenSSL 0.9.8y, Java 6u45
* Symantec distrust: not impacted

}
if resp.Status == ocsp.Revoked {
status.RevokedAt = resp.RevokedAt
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The revoked_at field is still stored here, but it looks to be detected by .IsZero(), which doesn't really make sense as a revocation time anyway.

The status field is always checked in the printer anyway.

tls_observatory=# select * from analysis where worker_name='ocspStatus' and scan_id=5; 
 id | scan_id | worker_name | success |                       output                        
----+---------+-------------+---------+-----------------------------------------------------
 25 |       5 | ocspStatus  | t       | {"status": 0, "revoked_at": "0001-01-01T00:00:00Z"}

@adamdecaf
Copy link
Contributor Author

It looks like there's some (stalled) work on a badssl.com example for revoked/expired OCSP. I don't see an example on their main site.

chromium/badssl.com#54

@ghost
Copy link

ghost commented Apr 15, 2018

@adamdecaf Thanks for finishing this! I was trying to find time to complete this, but I've been really busy lately.

Copy link
Contributor

@jvehent jvehent left a comment

Choose a reason for hiding this comment

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

Works locally and code looks good, just one nit on the printer output.

results = append(results, fmt.Sprintf(" - Revoked at %s\n", result.RevokedAt.Format(time.RFC3339)))
default:
results = append(results, fmt.Sprintf(" - Unknown status code %d\n", result.Status))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets put everything on a single line like we did for the CAA and CRL workers. The output should be fmt.Sprintf("* OCSP: <status>"). Also, don't include a newline, the printer does that automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jvehent Done!

@jvehent jvehent merged commit 8f2d8d7 into mozilla:master May 8, 2018
@adamdecaf adamdecaf deleted the ocspStatus branch May 8, 2018 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants