You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/04/25 03:48:33 UTC
[GitHub] [hadoop-ozone] avijayanhwx opened a new pull request #871: HDDS-3487. Ozone start fails with NullPointerException in TLS enabled…
avijayanhwx opened a new pull request #871:
URL: https://github.com/apache/hadoop-ozone/pull/871
… cluster.
## What changes were proposed in this pull request?
**SCM start failed with exception
java.lang.NullPointerException
at org.apache.hadoop.hdds.conf.ConfigurationSource.getPassword(ConfigurationSource.java:112)
at org.apache.hadoop.hdds.server.http.BaseHttpServer.getPassword(BaseHttpServer.java:348)**
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3487
## How was this patch tested?
Manually tested.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on pull request #871: HDDS-3487. Ozone start fails with NullPointerException in TLS enabled cluster
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on pull request #871:
URL: https://github.com/apache/hadoop-ozone/pull/871#issuecomment-620250046
Thank you for the reviews @elek , @mukul1987.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on pull request #871: HDDS-3487. Ozone start fails with NullPointerException in TLS enabled cluster
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #871:
URL: https://github.com/apache/hadoop-ozone/pull/871#issuecomment-619955523
> What do you think about adding more verbose exception message as I proposed?
Or throw an exception.
Or remove it from the interface as Mukul suggested.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx edited a comment on pull request #871: HDDS-3487. Ozone start fails with NullPointerException in TLS enabled cluster
Posted by GitBox <gi...@apache.org>.
avijayanhwx edited a comment on pull request #871:
URL: https://github.com/apache/hadoop-ozone/pull/871#issuecomment-619702307
> I tested it with a TLS enabled cluster and it worked well:
>
> ssl-conf:
>
> ```
> <configuration>
> <property><name>ssl.server.keystore.location</name><value>/etc/keystore/keystore</value></property>
> <property><name>ssl.server.exclude.cipher.list</name><value>^.*MD5.*$,^TLS_DH_.*$,^.*RC4.*$,^.*CCM.*$,^TLS_DHE.*$,^.*SHA$,^TLS_RSA_WITH.*$</value></property>
> <property><name>ssl.server.keystore.password</name><value>Welcome1</value></property>
> <property><name>ssl.server.keystore.keypassword</name><value>Welcome1</value></property>
> <property><name>ssl.server.truststore.location</name><value>/etc/keystore/keystore</value></property>
> <property><name>ssl.server.truststore.password</name><value>Welcome1</value></property>
> </configuration>
> ```
>
> OM/SCM can be started without any problem. Based on the code I think It's a problem with the missing support of `CredentialProviders`. If you use localjceks, I would suggest to add this information to the NPE:
>
> ConfigurationSource.java:
>
> ```
> default char[] getPassword(String key) throws IOException {
> String value = get(key);
> if (value == null) {
> throw new NullPointerException(
> "Password entry is missing for key " + key
> + ".Note: generic ConfigurationSource interface doesn't support"
> + " Hadoop CredentialProvider implementations");
> }
> return value.toCharArray();
> }
> ```
@elek In the cluster that I tried out, the 'ssl.server.truststore.password' config key is not defined. Instead the getPassword relies on the Hadoop credential provider to get the truststore password.
Before this change, we relied on the Configuration#getPassword method while starting up the HTTP Server whose implementation looks like this.
`public char[] getPassword(String name) throws IOException {
char[] pass = null;
pass = getPasswordFromCredentialProviders(name);
if (pass == null) {
pass = getPasswordFromConfig(name);
}
return pass;
}`
After this change, the code logic becomes
` default char[] getPassword(String key) throws IOException {
return get(key).toCharArray();
}`
Hence, we have skipped the logic of checking in Credential provider.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] mukul1987 commented on a change in pull request #871: HDDS-3487. Ozone start fails with NullPointerException in TLS enabled cluster
Posted by GitBox <gi...@apache.org>.
mukul1987 commented on a change in pull request #871:
URL: https://github.com/apache/hadoop-ozone/pull/871#discussion_r415091934
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/LegacyHadoopConfigurationSource.java
##########
@@ -39,6 +40,11 @@ public String get(String key) {
return configuration.getRaw(key);
}
+ @Override
+ public char[] getPassword(String key) throws IOException {
Review comment:
Should we also remove the deault method on ConfigurationSource for getPassword, and have the get() method in OzoneConfiguration and the new method in LegacyHadoopConfig ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on pull request #871: HDDS-3487. Ozone start fails with NullPointerException in TLS enabled cluster
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on pull request #871:
URL: https://github.com/apache/hadoop-ozone/pull/871#issuecomment-619702307
> I tested it with a TLS enabled cluster and it worked well:
>
> ssl-conf:
>
> ```
> <configuration>
> <property><name>ssl.server.keystore.location</name><value>/etc/keystore/keystore</value></property>
> <property><name>ssl.server.exclude.cipher.list</name><value>^.*MD5.*$,^TLS_DH_.*$,^.*RC4.*$,^.*CCM.*$,^TLS_DHE.*$,^.*SHA$,^TLS_RSA_WITH.*$</value></property>
> <property><name>ssl.server.keystore.password</name><value>Welcome1</value></property>
> <property><name>ssl.server.keystore.keypassword</name><value>Welcome1</value></property>
> <property><name>ssl.server.truststore.location</name><value>/etc/keystore/keystore</value></property>
> <property><name>ssl.server.truststore.password</name><value>Welcome1</value></property>
> </configuration>
> ```
>
> OM/SCM can be started without any problem. Based on the code I think It's a problem with the missing support of `CredentialProviders`. If you use localjceks, I would suggest to add this information to the NPE:
>
> ConfigurationSource.java:
>
> ```
> default char[] getPassword(String key) throws IOException {
> String value = get(key);
> if (value == null) {
> throw new NullPointerException(
> "Password entry is missing for key " + key
> + ".Note: generic ConfigurationSource interface doesn't support"
> + " Hadoop CredentialProvider implementations");
> }
> return value.toCharArray();
> }
> ```
@elek In the cluster that I tried out, the 'ssl.server.truststore.password' config key is not defined. Instead the getPassword relies on the Hadoop credential provider to get the truststore password.
Before this change, we relied on the Configuration#getPassword method while starting up the HTTP Server whose implementation looks like this.
` public char[] getPassword(String name) throws IOException {
char[] pass = null;
pass = getPasswordFromCredentialProviders(name);
if (pass == null) {
pass = getPasswordFromConfig(name);
}
return pass;
}`
After this change, the code logic becomes
` default char[] getPassword(String key) throws IOException {
return get(key).toCharArray();
}`
Hence, we have skipped the logic of checking in Credential provider.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on pull request #871: HDDS-3487. Ozone start fails with NullPointerException in TLS enabled cluster
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #871:
URL: https://github.com/apache/hadoop-ozone/pull/871#issuecomment-619952134
@avijayanhwx Thanks to explain it. This is the same what I found, it's not related to the TLS config it's related to the credential provider. What do you think about adding more verbose exception message as I proposed?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx edited a comment on pull request #871: HDDS-3487. Ozone start fails with NullPointerException in TLS enabled cluster
Posted by GitBox <gi...@apache.org>.
avijayanhwx edited a comment on pull request #871:
URL: https://github.com/apache/hadoop-ozone/pull/871#issuecomment-619702307
> I tested it with a TLS enabled cluster and it worked well:
>
> ssl-conf:
>
> ```
> <configuration>
> <property><name>ssl.server.keystore.location</name><value>/etc/keystore/keystore</value></property>
> <property><name>ssl.server.exclude.cipher.list</name><value>^.*MD5.*$,^TLS_DH_.*$,^.*RC4.*$,^.*CCM.*$,^TLS_DHE.*$,^.*SHA$,^TLS_RSA_WITH.*$</value></property>
> <property><name>ssl.server.keystore.password</name><value>Welcome1</value></property>
> <property><name>ssl.server.keystore.keypassword</name><value>Welcome1</value></property>
> <property><name>ssl.server.truststore.location</name><value>/etc/keystore/keystore</value></property>
> <property><name>ssl.server.truststore.password</name><value>Welcome1</value></property>
> </configuration>
> ```
>
> OM/SCM can be started without any problem. Based on the code I think It's a problem with the missing support of `CredentialProviders`. If you use localjceks, I would suggest to add this information to the NPE:
>
> ConfigurationSource.java:
>
> ```
> default char[] getPassword(String key) throws IOException {
> String value = get(key);
> if (value == null) {
> throw new NullPointerException(
> "Password entry is missing for key " + key
> + ".Note: generic ConfigurationSource interface doesn't support"
> + " Hadoop CredentialProvider implementations");
> }
> return value.toCharArray();
> }
> ```
@elek In the cluster that I tried out, the 'ssl.server.truststore.password' config key is not defined. Instead the getPassword relies on the Hadoop credential provider to get the truststore password.
Before this change, we relied on the Configuration#getPassword method while starting up the HTTP Server whose implementation looks like this.
```java
public char[] getPassword(String name) throws IOException {
char[] pass = null;
pass = getPasswordFromCredentialProviders(name);
if (pass == null) {
pass = getPasswordFromConfig(name);
}
return pass;
}
```
After this change, the code logic becomes
```java
default char[] getPassword(String key) throws IOException {
return get(key).toCharArray();
}
```
Hence, we have skipped the logic of checking in Credential provider.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek edited a comment on pull request #871: HDDS-3487. Ozone start fails with NullPointerException in TLS enabled cluster
Posted by GitBox <gi...@apache.org>.
elek edited a comment on pull request #871:
URL: https://github.com/apache/hadoop-ozone/pull/871#issuecomment-619955523
> What do you think about adding more verbose exception message as I proposed?
Or throw an `UnssupportedOperationException` exception.
Or remove it from the interface as Mukul suggested.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on a change in pull request #871: HDDS-3487. Ozone start fails with NullPointerException in TLS enabled cluster
Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #871:
URL: https://github.com/apache/hadoop-ozone/pull/871#discussion_r415769570
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/LegacyHadoopConfigurationSource.java
##########
@@ -39,6 +40,11 @@ public String get(String key) {
return configuration.getRaw(key);
}
+ @Override
+ public char[] getPassword(String key) throws IOException {
Review comment:
Long-term we might need a `getPassword()` for `ConfigurationSource` and we can have different implementations (with Hadoop Configuration we can use `CredentialProviders`, with different `ConfigurationSource` we can have different approaches).
Currently we have a very naive approach for `ConfigurationSource` we read the value as is, but using `getPassword` instead of `get` shows that we need an encryption.
But as of know getPassword() is not called only when we use Hadoop Configuration. Agree, we can remove it temporary, until we clean-up the HTTP utilities and switch to use full `Configuration`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org