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

8139348: Deprecate 3DES and RC4 in Kerberos #2701

Closed
wants to merge 3 commits into from

Conversation

wangweij
Copy link
Contributor

@wangweij wangweij commented Feb 24, 2021

Deprecate des3-hmac-sha1 (etype 16) and rc4-hmac (etype 23). User can add "allow_weak_crypto = true" in krb5.conf to re-enable them (plus the DES-based etypes deprecated long ago).


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

Reviewers

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2701/head:pull/2701
$ git checkout pull/2701

8139348: Deprecate 3DES and RC4 in Kerberos
@bridgekeeper
Copy link

bridgekeeper bot commented Feb 24, 2021

👋 Welcome back weijun! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@wangweij
Copy link
Contributor Author

/csr

@openjdk openjdk bot added rfr Pull request is ready for review csr Pull request needs approved CSR before integration labels Feb 24, 2021
@openjdk
Copy link

openjdk bot commented Feb 24, 2021

@wangweij this pull request will not be integrated until the CSR request JDK-8262273 for issue JDK-8139348 has been approved.

@openjdk
Copy link

openjdk bot commented Feb 24, 2021

@wangweij The following label will be automatically applied to this pull request:

  • security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the security security-dev@openjdk.org label Feb 24, 2021
@mlbridge
Copy link

mlbridge bot commented Feb 24, 2021

Webrevs

@seanjmullan
Copy link
Member

Is there a test that checks that the weak algorithms are actually disabled? I wasn't sure if I saw anything or maybe that is another test that you didn't have to modify?

@@ -23,7 +23,7 @@

/*
* @test
* @bug 6960894 8194486
* @bug 6960894 8194486 8139348
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if there are any rules or best practices about this, but I usually don't put a bugid on a test if it isn't specifically testing what this bug is about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll remove it.

@wangweij
Copy link
Contributor Author

wangweij commented Feb 24, 2021

Is there a test that checks that the weak algorithms are actually disabled? I wasn't sure if I saw anything or maybe that is another test that you didn't have to modify?

Yes there's one and I'll update it. I can also add all weak etypes into onlythree.conf and they should be ignored.

@wangweij
Copy link
Contributor Author

Updated tests. There is a weakcrypto.conf file which has been useless for a long time since WeakCrypto.java generates krb5.conf on the fly.

@openjdk openjdk bot removed the csr Pull request needs approved CSR before integration label Feb 24, 2021
@wangweij
Copy link
Contributor Author

Please also review the release note at https://bugs.openjdk.java.net/browse/JDK-8262335.

@openjdk
Copy link

openjdk bot commented Feb 25, 2021

@wangweij This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8139348: Deprecate 3DES and RC4 in Kerberos

Reviewed-by: mullan

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 25 new commits pushed to the master branch:

  • c54724d: 8257234: Add gz option to SA jmap to write a gzipped heap dump
  • aa35b42: 8261131: jcmd jmap dump should not accept gz option with no value
  • ebdc80e: 8252883: AccessDeniedException caused by delayed file deletion on Windows
  • f79c626: 8262296: Fix remaining doclint warnings in jdk.httpserver
  • ea48a0b: 8262163: Extend settings printout in jcmd VM.metaspace
  • a83e802: 8262299: C2 compilation fails with "modified node was not processed by IGVN.transform_old()"
  • 0f8be6e: 8261868: Reduce inclusion of metaspace.hpp
  • 3a0d6a6: 8262099: jcmd VM.metaspace should report unlimited size if MaxMetaspaceSize isn't specified
  • a50725d: 8254001: [Metrics] Enhance parsing of cgroup interface files for version detection
  • 6549212: 8262315: missing ';' in generated entities
  • ... and 15 more: https://git.openjdk.java.net/jdk/compare/d2b9c227e55defc37c0b0f6362637823ced6a220...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Feb 25, 2021
@wangweij
Copy link
Contributor Author

/integrate

@openjdk openjdk bot closed this Feb 25, 2021
@openjdk openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 25, 2021
@openjdk
Copy link

openjdk bot commented Feb 25, 2021

@wangweij Since your change was applied there have been 29 commits pushed to the master branch:

  • 5a9b701: 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides
  • 7d4f60b: 8260403: javap should be more robust in the face of invalid class files
  • 674be87: 8261203: Incorrectly escaped javadoc html with type annotations
  • 2eca17d: 8261457: test/langtools/tools/javac/T8187978 can fail if ArrayList class is modified
  • c54724d: 8257234: Add gz option to SA jmap to write a gzipped heap dump
  • aa35b42: 8261131: jcmd jmap dump should not accept gz option with no value
  • ebdc80e: 8252883: AccessDeniedException caused by delayed file deletion on Windows
  • f79c626: 8262296: Fix remaining doclint warnings in jdk.httpserver
  • ea48a0b: 8262163: Extend settings printout in jcmd VM.metaspace
  • a83e802: 8262299: C2 compilation fails with "modified node was not processed by IGVN.transform_old()"
  • ... and 19 more: https://git.openjdk.java.net/jdk/compare/d2b9c227e55defc37c0b0f6362637823ced6a220...master

Your commit was automatically rebased without conflicts.

Pushed as commit ded96dd.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@wangweij wangweij deleted the 8139348 branch February 25, 2021 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated Pull request has been integrated security security-dev@openjdk.org
2 participants