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

Refresh the OkHttp cipher suite process #7393

Merged
merged 8 commits into from
Sep 18, 2022
Merged

Refresh the OkHttp cipher suite process #7393

merged 8 commits into from
Sep 18, 2022

Conversation

yschimke
Copy link
Collaborator

A quick attempt at #4655

Starting from raw source files as much as possible.

SSL_ and TLS seems like an issue. Plus TLS1.3 ciphers are not enabled...

@yschimke yschimke changed the title Refresh the OkHttp cipher updates Refresh the OkHttp cipher suite process Jul 24, 2022
@yschimke yschimke requested a review from swankjesse July 24, 2022 17:58
@yschimke
Copy link
Collaborator Author

TODO

Remove these entries and source from a clean record and checkin.

    addRecord(RECORDS, "0x1301  TLS_AES_128_GCM_SHA256", "", "")
    addRecord(RECORDS, "0x1302  TLS_AES_256_GCM_SHA384", "", "")
    addRecord(RECORDS, "0x1303  TLS_CHACHA20_POLY1305_SHA256", "", "")
    addRecord(RECORDS, "0x1304  TLS_AES_128_CCM_SHA256", "", "")
    addRecord(RECORDS, "0x1305  TLS_AES_128_CCM_8_SHA256", "", "")
    addRecord(RECORDS, "0xc02b TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "1", "0")
    addRecord(RECORDS, "0xc02f TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "8", "3")

@yschimke
Copy link
Collaborator Author

yschimke commented Aug 7, 2022

Results are here https://docs.google.com/spreadsheets/d/15IMJSVHjFQxuy1_trOGRz4FS9MCFquKzrfu_4pI84kU/edit#gid=0

@yschimke yschimke marked this pull request as ready for review August 7, 2022 12:33
@yschimke
Copy link
Collaborator Author

yschimke commented Aug 7, 2022

@swankjesse anywhere good to land?

@yschimke
Copy link
Collaborator Author

yschimke commented Aug 7, 2022

Likely no cipher changes for now.

@yschimke yschimke mentioned this pull request Aug 7, 2022
Closed
@yschimke yschimke mentioned this pull request Aug 8, 2022
Closed
8 tasks
@yschimke
Copy link
Collaborator Author

Some interesting additional sources

https://ciphersuite.info/api/

@yschimke
Copy link
Collaborator Author

@swankjesse It's noticeable given #6390 and #7452

That the intent of this process was not being honoured.

@yschimke
Copy link
Collaborator Author

Landing as sample code only

@yschimke yschimke merged commit c56e864 into square:master Sep 18, 2022
@yschimke yschimke deleted the survey branch May 27, 2023 11:24
@yschimke
Copy link
Collaborator Author

Where are you referring to?

swankjesse
swankjesse reviewed Apr 1, 2024
View reviewed changes
samples/tlssurvey/build.gradle.kts
}

dependencies {
implementation("com.squareup.okhttp3:okhttp:5.0.0-alpha.10")
Copy link
Member

Choose a reason for hiding this comment

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

Including a dependency on the published OkHttp interferes pretty badly with IntelliJ. I’m gonna send a PR to remove this.

Copy link
Member

Choose a reason for hiding this comment

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

#8323

swankjesse
swankjesse reviewed Apr 1, 2024
View reviewed changes
Copy link
Member

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

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

LGTM!

samples/tlssurvey/src/main/resources/okhttp_3_11.txt
@@ -0,0 +1,13 @@
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
Copy link
Member

Choose a reason for hiding this comment

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

Checking these in in a machine-readable format is rad

samples/tlssurvey/src/main/kotlin/okhttp3/survey/types/Client.kt
*/
package okhttp3.survey.types

data class Client(
Copy link
Member

Choose a reason for hiding this comment

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

NICE

samples/tlssurvey/src/main/kotlin/okhttp3/survey/ssllabs/SslLabsScrape.kt
import retrofit2.Retrofit
import retrofit2.converter.moshi.MoshiConverterFactory

class SslLabsScraper(
Copy link
Member

Choose a reason for hiding this comment

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

I slightly object to the word scraper here, as it suggests we’re doing something untoward?

The SSL labs people are awesome and offer their data under a generous creative commons license!
https://www.ssllabs.com/downloads/Qualys_SSL_Labs_Terms_of_Use.pdf

Lemme send a PR to rename this!

Copy link
Member

Choose a reason for hiding this comment

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

#8324

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swankjesse swankjesse swankjesse left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@yschimke @swankjesse