RFC10015 support in TlsCiphers

[garydgregory]

RFC10015 support in TlsCiphers

• TlsCiphers.isWeak(String) return true for deprecated and discouraged
  cipher suites.
• No change to TlsCiphers.isH2Blacklisted(String)
• Disable Checkstyle MissingDeprecated for TestTlsCiphers, it's not
  smart enough to avoid a false positive.

[ok2c]

@garydgregory Could you please re-apply your changes on top of master?

Disable Checkstyle MissingDeprecated for TestTlsCiphers, it's not smart enough to avoid a false positive.

I do not understand why this is necessary. It should not be. There should be no @Deprecated annotations in test classes.

[garydgregory]

The build fails without this because Checkstyle sees the word "deprecated" in the comment and wants a deprecated annotation I guess. It's lame but that's what happens.

[ok2c]

The build fails without this because Checkstyle sees the word "deprecated" in the comment and wants a deprecated annotation I guess. It's lame but that's what happens.

@garydgregory This is bizarre. This is what the check does. It should be looking for @deprecated tag, not just a random occurrence of deprecated text as far as I can tell. Once you have your change-set ready I will test it locally with and without the MissingDeprecated check override.

    @Override
    public void visitJavadocToken(DetailNode ast) {
        final DetailAST parentAst = getParent(getBlockCommentAst());

        final boolean containsAnnotation =
            AnnotationUtil.containsAnnotation(parentAst, DEPRECATED)
            || AnnotationUtil.containsAnnotation(parentAst, FQ_DEPRECATED);

        final boolean containsJavadocTag = containsDeprecatedTag(ast);

        if (containsAnnotation ^ containsJavadocTag) {
            log(parentAst.getLineNo(), MSG_KEY_ANNOTATION_MISSING_DEPRECATED);
        }
    }

[garydgregory]

@ok2c
I resolved the conflicts in this PR, and without the Checkstyle changes, I get:

[INFO] --- checkstyle:3.3.1:checkstyle (validate-main) @ httpcore5 ---
[INFO] Rendering content with org.apache.maven.skins:maven-default-skin:jar:1.3 skin.
[INFO] Starting audit...
[ERROR] /Users/garygregory/git/g/httpcomponents-core/httpcore5/src/test/java/org/apache/hc/core5/http/ssl/TestTlsCiphers.java:109: Javadoc comment at column 109 has parse error. Details: no viable alternative at input '<a href="https://www.rfc-editor.org/rfc/rfc10015.html>"RFC10015<' while parsing HTML_ELEMENT [MissingDeprecated]
[ERROR] /Users/garygregory/git/g/httpcomponents-core/httpcore5/src/test/java/org/apache/hc/core5/http/ssl/TestTlsCiphers.java:183: Javadoc comment at column 125 has parse error. Details: no viable alternative at input '<a href="https://www.rfc-editor.org/rfc/rfc10015.html>"RFC10015<' while parsing HTML_ELEMENT [MissingDeprecated]
[ERROR] /Users/garygregory/git/g/httpcomponents-core/httpcore5/src/test/java/org/apache/hc/core5/http/ssl/TestTlsCiphers.java:234: Javadoc comment at column 110 has parse error. Details: no viable alternative at input '<a href="https://www.rfc-editor.org/rfc/rfc10015.html>"RFC10015<' while parsing HTML_ELEMENT [MissingDeprecated]
[ERROR] /Users/garygregory/git/g/httpcomponents-core/httpcore5/src/test/java/org/apache/hc/core5/http/ssl/TestTlsCiphers.java:315: Javadoc comment at column 110 has parse error. Details: no viable alternative at input '<a href="https://www.rfc-editor.org/rfc/rfc10015.html>"RFC10015<' while parsing HTML_ELEMENT [MissingDeprecated]
Audit done.
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Apache HttpComponents Core 5.5-beta3-SNAPSHOT:
[INFO] 
[INFO] Apache HttpComponents Core ......................... SUCCESS [  2.801 s]
[INFO] Apache HttpComponents Core HTTP/1.1 ................ FAILURE [  2.964 s]
[INFO] Apache HttpComponents Core HTTP/2 .................. SKIPPED
[INFO] Apache HttpComponents Core Reactive Extensions ..... SKIPPED
[INFO] Apache HttpComponents Core Integration Tests ....... SKIPPED
[INFO] Jackson 3 bindings for Apache HttpComponents Core .. SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------

[ok2c]

@garydgregory GitHub still says "This branch cannot be rebased due to conflicts". There are four commits. I presume there should be just one on top of the latest master.

[garydgregory]

@ok2c
I added some Javadoc and left the test disabled. I'm not sure this PR should untangle weak vs. prohibited. I am concerned that adding RFC10015 "deprecated" (but not "discouraged") to our H2 blacklist (prohibited list) will blow up too many existing clients. A future enhancement could allow clients to enabled failing when an RFC10015 "deprecated" or "discouraged" cipher is used. This could be useful for pentesting.

[ok2c]

I added some Javadoc and left the test disabled. I'm not sure this PR should untangle weak vs. prohibited. I am concerned that adding RFC10015 "deprecated" (but not "discouraged") to our H2 blacklist (prohibited list) will blow up too many existing clients.

@garydgregory My proposal was different. We should not mess with the H2 blacklist given it is a part of the protocol specification, but we could make sure all its ciphers are also in the "weak" list.

Having said that I personally do not see a contradiction with some ciphers being in H2 blacklist and not being in "weak" list, so I do not see any value in adding a disabled test unless you personally would invest time into it and enable the test.

[garydgregory]

@ok2c
I will have another look... Please see commit 43bdeb0.