Re: RFR: 8273236: keytool does not accurately warn about algorithms that are disabled but have additional constraints [v2]
> `keytool` currently uses a simpler scheme in `DisabledAlgorithmConstraints` > class when performing algorithm constraints checks. This change is to enhance > `keytool` to make use of the new methods > `DisabledAlgorithmConstraints.permits` with `CertPathConstraintsParameters` > and `checkKey` parameters. For the keyusage in the EE certificate of a > certificate chains, set the variant accordingly when calling > `CertPathConstraintsParameters` constructor. Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision: Update with review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/7039/files - new: https://git.openjdk.java.net/jdk/pull/7039/files/10e73e50..a05728a7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7039=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7039=00-01 Stats: 60 lines in 3 files changed: 20 ins; 11 del; 29 mod Patch: https://git.openjdk.java.net/jdk/pull/7039.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7039/head:pull/7039 PR: https://git.openjdk.java.net/jdk/pull/7039
Re: RFR: 8273236: keytool does not accurately warn about algorithms that are disabled but have additional constraints [v2]
On Thu, 13 Jan 2022 16:31:35 GMT, Sean Mullan wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update with review comments
>
> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 187:
>
>> 185: private List weakWarnings = new ArrayList<>();
>> 186:
>> 187: Set trustedCerts = new HashSet<>();
>
> Make private.
Fixed.
> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 1129:
>
>> 1127: }
>> 1128:
>> 1129: buildTrustedCerts();
>
> Can we reuse the keystore loaded by `buildTrustedCerts()` instead of
> reloading it again on line 1138?
No change. This is because `caks` global variable can only be initialized with
cacerts keystore when the `trustcacerts` option is specified; otherwise if has
to be kept null. `buildTrustedCerts`() is always executed.
> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 1558:
>
>> 1556: checkWeakConstraint(rb.getString("the.issuer"),
>> 1557: keyStore.getCertificateChain(alias));
>> 1558: cpcp = new CertPathConstraintsParameters(cert, null, null,
>> null);
>
> You could also specify the variant parameter if the certificate contains an
> EKU extension. The mapping from EKU to variant would be:
>
> - anyExtendedKeyUsage -> VAR_GENERIC
> - serverAuth -> VAR_TLS_SERVER
> - clientAuth -> VAR_TLS_CLIENT
> - codeSigning -> VAR_CODE_SIGNING
> - emailProtection -> none defined, so just use null or VAR_GENERIC
> - timeStamping -> VAR_TSA_SERVER
> - OCSPSigning -> none defined, so just use null or VAR_GENERIC
Fixed.
> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 1706:
>
>> 1704: }
>> 1705: dumpCert(cert, out);
>> 1706: CertPathConstraintsParameters cpcp =
>
> Here you could also specify the variant if the cert contains an EKU
> extension. Same comment applies to other places where you are checking the
> algorithm constraints of a certificate.
Fixed.
> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 2198:
>
>> 2196: ("Certificate.chain.length.") + chain.length);
>> 2197:
>> 2198: X509Certificate[] xcerts = convertCerts(chain);
>
> I think you can just cast to an `X509Certificate[]` instead of reparsing all
> the certificates, i.e.:
>
> `X509Certificate[] xcerts = (X509Certificate[]) chain;`
I got `java.lang.ClassCastException` when casting `Certificate[]` array form
here. However, I should cast Certificate object directly instead of reparsing
the certificate, and made the change to do so.
> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 2259:
>
>> 2257: }
>> 2258: cpcp = new
>> CertPathConstraintsParameters((X509Certificate)cert,
>> 2259: null,null, null);
>
> Nit - add space between `null,null`.
Fixed.
> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 5039:
>
>> 5037:
>> trustedCerts.add((X509Certificate)caks.getCertificate(a));
>> 5038: } catch (Exception e2) {
>> 5039: // ignore, when a SecretkeyEntry does not
>> include a cert
>
> Not sure I understand this comment, as these should not be SecretKeyEntries.
> You should still ignore it but this should not throw an Exception unless the
> KeyStore was not loaded/initialized properly (which normally won't occur).
Updated with the suggested comments.
> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 5048:
>
>> 5046: }
>> 5047:
>> 5048: private TrustAnchor findTrustAnchor(List chain) {
>
> I would consider having an initial check that returns `null` if
> `chain.isEmpty()`. Not sure if that is a valid scenario, but it would avoid
> an `IndexOOBException` just in case.
Added the initial check.
> src/java.base/share/classes/sun/security/tools/keytool/Resources.java line
> 486:
>
>> 484: {"verified.by.s.in.s.weak", "Verified by %1$s in %2$s with a
>> %3$s"},
>> 485: {"whose.sigalg.disabled", "%1$s uses the %2$s signature
>> algorithm which is considered a security risk and is disabled."},
>> 486: {"whose.sigalg.usagesignedjar", "%1$s uses the %2$s signature
>> algorithm which is considered a security risk and cannot be used to sign
>> JARs after 2019-01-01."},
>
> Instead of hard-coding "2019-01-01", we should extract this date from the
> `denyAfter` attribute of the `jdk.certpath.disabledAlgorithms` security
> property and pass it in as a parameter.
Fixed.
-
PR: https://git.openjdk.java.net/jdk/pull/7039
Integrated: 8280401: [sspi] gss_accept_sec_context leaves output_token uninitialized
On Thu, 20 Jan 2022 18:19:19 GMT, Weijun Wang wrote: > Set `output_token` to empty. It is always accessed (even for a > `GSS_S_FAILURE`) at > https://github.com/openjdk/jdk/blob/cfa3f7493149170f2b23a516bc95110dab43fd06/src/java.security.jgss/share/native/libj2gss/GSSLibStub.c#L1160. This pull request has now been integrated. Changeset: 6352c020 Author:Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/6352c020c25f2701afb4fabee0cc7fcef2d407fb Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod 8280401: [sspi] gss_accept_sec_context leaves output_token uninitialized Reviewed-by: valeriep - PR: https://git.openjdk.java.net/jdk/pull/7163
Re: RFR: 8280401: [sspi] gss_accept_sec_context leaves output_token uninitialized
On Thu, 20 Jan 2022 18:19:19 GMT, Weijun Wang wrote: > Set `output_token` to empty. It is always accessed (even for a > `GSS_S_FAILURE`) at > https://github.com/openjdk/jdk/blob/cfa3f7493149170f2b23a516bc95110dab43fd06/src/java.security.jgss/share/native/libj2gss/GSSLibStub.c#L1160. Changes look good. Remember to add the noreg-xxx label. Thanks~ - Marked as reviewed by valeriep (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7163
Re: RFR: 8272317: jstatd has dependency on Security Manager which needs to be removed [v2]
On Thu, 20 Jan 2022 16:54:21 GMT, Mandy Chung wrote: > If `sun.jvmstat.monitor.remote.RemoteVm` is the only proxy interface, > `com.sun.proxy.jdk.proxy*` should adequately cover the proxy classes created > for `RemoteVm`. Thanks. With that endorsement I think there are no unresolved issues with this change. - PR: https://git.openjdk.java.net/jdk/pull/6919
Re: RFR: 8255739: x509Certificate returns � for invalid subjectAlternativeNames
On 1/18/2022 4:10 PM, Sean Mullan wrote: On Thu, 6 Jan 2022 20:28:22 GMT, Sean Mullan wrote: Could you please review the JDK-8255739 bug fix? I think sun.security.x509.SubjectAlternativeNameExtension() should throw an exception for incorrect SubjectAlternativeNames instead of returning the substituted characters, which is explained in the description of BugDB. I modified DerValue.readStringInternal() not to read incorrect SubjectAlternativeNames and throw an IOException. sun.security.x509.X509CertInfo.parse() catch the IOExcepton and ignore it if SAN is a non-ciritical extension like the behavior of the IOException in readStringInternal(). So I added a test with -Djava.security.debug=x509 to confirm that. I understand the reasons for making the code more robust and detecting invalid DER encodings, but this may have a non-trivial compatibility risk. In general, I think detecting invalid encodings is generally the right thing to do, but compatibility needs to be considered. Sometimes other implementations have encoding bugs that we need to workaround, etc. This change affects not only certificates but other security components that use DER in the JDK. Certificates already treat non-critical extensions that are badly encoded as not a failure, so there is some compatibility built-in already. But I think it makes sense to look at other code that calls into the DerValue methods and evaluate the potential compatibility risk. At a minimum, a CSR must be filed. As a compromise, it may make sense to (at least initially) reduce the compatibility risk by allowing the caller (ex: `sun.security.x509.DNSName`) to decide if it wants a stricter parsing of the DER-encoded string. I would like @wangweij or @valeriepeng to also review this. With respect to the test, it seems like overkill to launch a java process inside the test to run each test. Instead, I would just have separate methods for each test and run them in the same process as the main test. @seanjmullan @wangweij I have commented on what you pointed out, so could you please reply? I need another couple of days to look at this issue again before I can reply. - PR:https://git.openjdk.java.net/jdk/pull/6928 Hi - Bouncycastle started enforcing properly encoded INTEGERs a while back and that caused one of my programs to start failing due to a TPM X509 endorsement certificate just having the TPM serial number stuffed into the certificate serial number without normalizing it to the appropriate INTEGER encoding. BC provided a work around (setting "org.bouncycastle.asn1.allow_unsafe_integer") which gave me time to re-code around the problem. If you're going to break things, it may be useful to provide a work around similar to what they did. In any event, DerValue.java uses "new String(byteArrayValue, charsetType)" to produce the various String values including in getIA5String(). I.e., public String(byte[] bytes, Charset charset) Constructs a new |String| by decoding the specified array of bytes using the specified charset. The length of the new |String| is a function of the charset, and hence may not be equal to the length of the byte array. _*This method always replaces malformed-input and unmappable-character sequences with this charset's default replacement string.*_ The |CharsetDecoder| class should be used when more control over the decoding process is required. Perhaps it might make sense to update the various places where this is used in DerValue to CharsetDecoder and to use charsetDecoder.onMalformedInput() and charsetDecoder.onUnmappableCharacter() to set the appropriate action related to parsing the byte array into a String of a given charset? That action could be set based on the presence of the bypass property. I don't think the change as proposed is backward-compatible safe enough, nor does it really address the general issue of poorly encoded DER String values in a certificate. Mike
RFR: 8277976: Break up SEQUENCE in X509Certiticate#getSubjectAlternativeNames() in otherName
The enhancement adds two extra items in the `getSubjectAlternativeNames()` output for an OtherName. It also fix several errors: 1. In `OtherName.java`, `nameValue` should be the value inside `CONTEXT [0]` without the tag and length bytes. 2. The argument in constructor `extClass.getConstructor(Object.class)` is suspicious. Maybe it meant `byte[]`. - Commit messages: - 8277976: Break up SEQUENCE in X509Certiticate#getSubjectAlternativeNames() in otherName Changes: https://git.openjdk.java.net/jdk/pull/7167/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7167=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277976 Stats: 136 lines in 4 files changed: 117 ins; 0 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/7167.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7167/head:pull/7167 PR: https://git.openjdk.java.net/jdk/pull/7167
RFR: 8280401: [sspi] gss_accept_sec_context leaves output_token uninitialized
Set `output_token` to empty. It is always accessed (even for a `GSS_S_FAILURE`) at https://github.com/openjdk/jdk/blob/cfa3f7493149170f2b23a516bc95110dab43fd06/src/java.security.jgss/share/native/libj2gss/GSSLibStub.c#L1160. - Commit messages: - 8280401: [sspi] gss_accept_sec_context leaves output_token uninitialized Changes: https://git.openjdk.java.net/jdk/pull/7163/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7163=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280401 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7163.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7163/head:pull/7163 PR: https://git.openjdk.java.net/jdk/pull/7163
Re: RFR: 8272317: jstatd has dependency on Security Manager which needs to be removed [v2]
On Mon, 10 Jan 2022 11:17:12 GMT, Kevin Walls wrote: >> Remove the use of Security Manager from jstatd. >> Add use of an ObjectInputFilter to restrict RMI. >> >> Also we can undo the property-setting Launcher.gmk change from: 8279007: >> jstatd fails to start because SecurityManager is disabled >> ..as that is no longer needed. >> >> Docs/man page update to follow (JDK-8278619). > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > Wildcard in object filter to permit proxies, in case other activity in this > JVM changes the nameing/numbering of proxy classes. If `sun.jvmstat.monitor.remote.RemoteVm` is the only proxy interface, `com.sun.proxy.jdk.proxy*` should adequately cover the proxy classes created for `RemoteVm`. - PR: https://git.openjdk.java.net/jdk/pull/6919
Re: RFR: 8280363: Minor correction of ALPN specification in SSLParameters [v2]
On Thu, 20 Jan 2022 15:31:55 GMT, Daniel Fuchs wrote: >> Hm, much better. Thanks! > >> The phrase "descending order" seems more appropriate for numerical values. I >> think the previous wording was more clear, with a small change: "The array >> is ordered based on protocol preference, with the first entry being the most >> preferred." > > Thanks Sean - I had the same comment - but for some reason it seemed to have > been lost when I clicked on Approve. Not sure what happened to it :-( Thank you! - PR: https://git.openjdk.java.net/jdk/pull/7152
Integrated: 8280363: Minor correction of ALPN specification in SSLParameters
On Thu, 20 Jan 2022 07:12:42 GMT, Xue-Lei Andrew Fan wrote: > In the getApplicationProtocols() method in javax.net.ssl.SSLParameters, the > return statement says that "The array is ordered based on protocol > preference, with protocols[0] being the most preferred.". However, there is > no "protocols" variable in this method. > > The update is a minor correction so that the specification is not rely on the > "protocols" variable. This pull request has now been integrated. Changeset: 0ea2b390 Author:Xue-Lei Andrew Fan URL: https://git.openjdk.java.net/jdk/commit/0ea2b39035f1b535a53770379c94ae43f0ddb8b6 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8280363: Minor correction of ALPN specification in SSLParameters Reviewed-by: dfuchs, mullan - PR: https://git.openjdk.java.net/jdk/pull/7152
Re: RFR: 8280363: Minor correction of ALPN specification in SSLParameters [v2]
On Thu, 20 Jan 2022 14:42:56 GMT, Xue-Lei Andrew Fan wrote:
>> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 619:
>>
>>> 617: *
>>> 618: * @return a non-null, possibly zero-length array of application
>>> protocol
>>> 619: * {@code String}s. The array is placed in descending
>>> order of
>>
>> The phrase "descending order" seems more appropriate for numerical values. I
>> think the previous wording was more clear, with a small change: "The array
>> is ordered based on protocol preference, with the first entry being the most
>> preferred."
>
> Hm, much better. Thanks!
> The phrase "descending order" seems more appropriate for numerical values. I
> think the previous wording was more clear, with a small change: "The array is
> ordered based on protocol preference, with the first entry being the most
> preferred."
Thanks Sean - I had the same comment - but for some reason it seemed to have
been lost when I clicked on Approve. Not sure what happened to it :-(
-
PR: https://git.openjdk.java.net/jdk/pull/7152
Re: RFR: 8280363: Minor correction of ALPN specification in SSLParameters [v2]
On Thu, 20 Jan 2022 14:46:28 GMT, Xue-Lei Andrew Fan wrote: >> In the getApplicationProtocols() method in javax.net.ssl.SSLParameters, the >> return statement says that "The array is ordered based on protocol >> preference, with protocols[0] being the most preferred.". However, there is >> no "protocols" variable in this method. >> >> The update is a minor correction so that the specification is not rely on >> the "protocols" variable. > > Xue-Lei Andrew Fan has updated the pull request incrementally with one > additional commit since the last revision: > > Update per feedback Marked as reviewed by mullan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7152
Re: RFR: 8280363: Minor correction of ALPN specification in SSLParameters [v2]
On Thu, 20 Jan 2022 14:16:43 GMT, Sean Mullan wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update per feedback
>
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 619:
>
>> 617: *
>> 618: * @return a non-null, possibly zero-length array of application
>> protocol
>> 619: * {@code String}s. The array is placed in descending
>> order of
>
> The phrase "descending order" seems more appropriate for numerical values. I
> think the previous wording was more clear, with a small change: "The array is
> ordered based on protocol preference, with the first entry being the most
> preferred."
Hm, much better. Thanks!
-
PR: https://git.openjdk.java.net/jdk/pull/7152
Re: RFR: 8280363: Minor correction of ALPN specification in SSLParameters [v2]
> In the getApplicationProtocols() method in javax.net.ssl.SSLParameters, the > return statement says that "The array is ordered based on protocol > preference, with protocols[0] being the most preferred.". However, there is > no "protocols" variable in this method. > > The update is a minor correction so that the specification is not rely on the > "protocols" variable. Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: Update per feedback - Changes: - all: https://git.openjdk.java.net/jdk/pull/7152/files - new: https://git.openjdk.java.net/jdk/pull/7152/files/c16bf6c6..3992327e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7152=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7152=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7152.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7152/head:pull/7152 PR: https://git.openjdk.java.net/jdk/pull/7152
Re: RFR: 8280363: Minor correction of ALPN specification in SSLParameters
On Thu, 20 Jan 2022 07:12:42 GMT, Xue-Lei Andrew Fan wrote:
> In the getApplicationProtocols() method in javax.net.ssl.SSLParameters, the
> return statement says that "The array is ordered based on protocol
> preference, with protocols[0] being the most preferred.". However, there is
> no "protocols" variable in this method.
>
> The update is a minor correction so that the specification is not rely on the
> "protocols" variable.
src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 619:
> 617: *
> 618: * @return a non-null, possibly zero-length array of application
> protocol
> 619: * {@code String}s. The array is placed in descending order
> of
The phrase "descending order" seems more appropriate for numerical values. I
think the previous wording was more clear, with a small change: "The array is
ordered based on protocol preference, with the first entry being the most
preferred."
-
PR: https://git.openjdk.java.net/jdk/pull/7152
Re: RFR: 8280363: Minor correction of ALPN specification in SSLParameters
On Thu, 20 Jan 2022 07:12:42 GMT, Xue-Lei Andrew Fan wrote: > In the getApplicationProtocols() method in javax.net.ssl.SSLParameters, the > return statement says that "The array is ordered based on protocol > preference, with protocols[0] being the most preferred.". However, there is > no "protocols" variable in this method. > > The update is a minor correction so that the specification is not rely on the > "protocols" variable. Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7152
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v3]
On Thu, 20 Jan 2022 10:58:27 GMT, Michael McMahon wrote:
>> Hi,
>>
>> This change adds Channel Binding Token (CBT) support to HTTPS
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO,
>> Kerberos) authentication scheme. When enabled, the implementation
>> preemptively includes a CBT with authentication requests over Kerberos. The
>> feature is enabled as follows:
>>
>> A system property "jdk.spnego.cbt" is defined which can have the values
>> "never" (default), which means the feature is disabled, "always", which
>> means the CBT is included for all https Negotiate authentications, or it can
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of
>> domains/hosts where the feature is enabled, and disabled everywhere else. In
>> the given example, the CBT would be included in authentication requests for
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its
>> sub-domains.
>>
>> A test will be added separately to the implementation.
>>
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>>
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one
> additional commit since the last revision:
>
> removed sasl module dependency and added SaslException cause
src/java.base/share/classes/java/net/doc-files/net-properties.html line 220:
> 218: This controls the generation and sending of TLS channel binding tokens
> (CBT) when Kerberos
> 219: or the Negotiate authentication scheme using Kerberos are
> employed over HTTPS with
> 220: {@code HttpURLConnection}. There are three possible settings:
Should it be `{@code HttpsURLConnection}`?
(BTW - can we use {@code } here ? Would be worth checking the generated doc)
src/java.base/share/classes/sun/net/www/http/HttpClient.java line 189:
> 187: } else {
> 188: logError("Unexpected value for \"jdk.https.negotiate.cbt\"
> system property");
> 189: return s;
Should this return either "always" or "never" instead? It seems that junk
values will be treated as "always". It would be better to make it clear here.
src/java.base/share/classes/sun/security/util/ChannelBindingException.java line
31:
> 29: * Thrown by TlsChannelBinding if an error occurs
> 30: */
> 31: public class ChannelBindingException extends Exception {
Should this extend `GeneralSecurityException` instead? Or should we just remove
this class and throw plain `GeneralSecurityException` in `TlsChannelBinding` ?
src/java.naming/share/classes/com/sun/jndi/ldap/sasl/LdapSasl.java line 143:
> 141: tlsCB = TlsChannelBinding.create(cert);
> 142: } catch (ChannelBindingException e) {
> 143: throw new SaslException(e.getMessage());
Why is there a difference compared to line 133?
-
PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v3]
On Thu, 20 Jan 2022 10:58:27 GMT, Michael McMahon wrote:
>> Hi,
>>
>> This change adds Channel Binding Token (CBT) support to HTTPS
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO,
>> Kerberos) authentication scheme. When enabled, the implementation
>> preemptively includes a CBT with authentication requests over Kerberos. The
>> feature is enabled as follows:
>>
>> A system property "jdk.spnego.cbt" is defined which can have the values
>> "never" (default), which means the feature is disabled, "always", which
>> means the CBT is included for all https Negotiate authentications, or it can
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of
>> domains/hosts where the feature is enabled, and disabled everywhere else. In
>> the given example, the CBT would be included in authentication requests for
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its
>> sub-domains.
>>
>> A test will be added separately to the implementation.
>>
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>>
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one
> additional commit since the last revision:
>
> removed sasl module dependency and added SaslException cause
src/java.naming/share/classes/com/sun/jndi/ldap/sasl/LdapSasl.java line 133:
> 131:
> (String)env.get(TlsChannelBinding.CHANNEL_BINDING_TYPE));
> 132: } catch (ChannelBindingException e) {
> 133: throw new SaslException(e.getMessage(), e);
Why not ust pass the exception if the API allows? This looks like message
duplication.
-
PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v3]
> Hi, > > This change adds Channel Binding Token (CBT) support to HTTPS > (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, Kerberos) > authentication scheme. When enabled, the implementation preemptively includes > a CBT with authentication requests over Kerberos. The feature is enabled as > follows: > > A system property "jdk.spnego.cbt" is defined which can have the values > "never" (default), which means the feature is disabled, "always", which means > the CBT is included for all https Negotiate authentications, or it can take > the form "domain:a,b.c,*.d.com" which is a comma separated list of > domains/hosts where the feature is enabled, and disabled everywhere else. In > the given example, the CBT would be included in authentication requests for > hosts "a", "b.c" and all hosts under the domain "d.com" and all of its > sub-domains. > > A test will be added separately to the implementation. > > Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842 > > Thanks, > Michael Michael McMahon has updated the pull request incrementally with one additional commit since the last revision: removed sasl module dependency and added SaslException cause - Changes: - all: https://git.openjdk.java.net/jdk/pull/7065/files - new: https://git.openjdk.java.net/jdk/pull/7065/files/f2ee58ec..fd56b5e3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7065=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7065=01-02 Stats: 2 lines in 2 files changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7065.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7065/head:pull/7065 PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v2]
On Wed, 19 Jan 2022 22:25:43 GMT, Weijun Wang wrote:
>> Michael McMahon has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> changes after first review round
>
> src/java.naming/share/classes/com/sun/jndi/ldap/sasl/LdapSasl.java line 133:
>
>> 131:
>> (String)env.get(TlsChannelBinding.CHANNEL_BINDING_TYPE));
>> 132: } catch (ChannelBindingException e) {
>> 133: throw new SaslException(e.getMessage());
>
> How about setting `e` as cause of new exception? In `TlsChannelBinding.java`
> the when the original exception was thrown (the 2nd throws) there was a cause.
Agreed.
> src/java.security.jgss/share/classes/module-info.java line 36:
>
>> 34: module java.security.jgss {
>> 35: requires java.naming;
>> 36: requires java.security.sasl;
>
> Can this be removed now?
Yes, well spotted!
-
PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8272317: jstatd has dependency on Security Manager which needs to be removed [v2]
On Wed, 19 Jan 2022 19:56:53 GMT, Mandy Chung wrote: > Are all the proxy interfaces public? sun.jvmstat.monitor.remote.RemoteVm is "public interface RemoteVm extends Remote" and methods in there only return basic types. This is in the jdk.jstatd module, where I see the module info contains "exports sun.jvmstat.monitor.remote to java.rmi;" The only other names permitted are proxy/reflect/rmi related. - PR: https://git.openjdk.java.net/jdk/pull/6919
