[icedtea-web] RFC: add configuration support for user prompts (and other access control options)
Omair Majid
omajid at redhat.com
Wed Nov 10 14:51:11 PST 2010
Hi Deepak,
Thanks for reviewing the patch. I have attached an updated version.
On 11/10/2010 04:02 PM, Deepak Bhole wrote:
> * Omair Majid<omajid at redhat.com> [2010-11-05 15:14]:
>> Hi,
>>
>> The attached patch adds support in netx for part of the 'security
>> access and control' set of configuration options. The patch allows
>> disabling user prompts for security warnings and denying
>> permissions, as well as not installing custom authenticator and not
>> granting permissions to hide window warnings.
>>
>> ChangeLog
>> 2010-11-05 Omair Majid<omajid at redhat.com>
>>
>> * netx/net/sourceforge/jnlp/SecurityDesc.java: Remove window banner
>> permissions from sandboxPermissions and j2eePermissions.
>> (getSandBoxPermissions): Dynamically add window banner permissions
>> if allowed by configuration.
>> * netx/net/sourceforge/jnlp/runtime/DeploymentConfiguration.java:
>> Add KEY_SECURITY_PROMPT_USER,
>> KEY_SECURITY_ALLOW_HIDE_WINDOW_WARNING,
>> KEY_SECURITY_PROMPT_USER_FOR_JNLP, and
>> KEY_SECURITY_INSTALL_AUTHENTICATOR.
>> (loadDefaultProperties): Use the new constants.
>> * netx/net/sourceforge/jnlp/security/SecurityWarning.java
>> (showAccessWarningDialog): Check if the user should be prompted
>> before prompting the user.
>> (showNotAllSignedWarningDialog): Likewise.
>> (showCertWarningDialog): Likewise.
>> (showAppletWarning): Likewise.
>> (shouldPromptUser): New method. Check if configuration allows
>> showing user prompts.
>> * netx/net/sourceforge/jnlp/services/ServiceUtil.java
>> (checkAccess(AccessType,Object...)): Clarify javadocs.
>> (checkAccess(ApplicationInstance,AccessType,Object...)): Clarify
>> javadocs. Only prompt the user if showing JNLP prompts is ok.
>> (shouldPromptUser): New method. Returns true if configuration allows
>> for showing JNLP api prompts.
>> * plugin/icedteanp/java/sun/applet/PluginMain.java
>> (init): Only install custom authenticator if allowed by
>> configuration.
>>
>> Any thoughts or comments?
>>
>> Thanks,
>> Omair
>
>> diff -r 8e66d9386273 netx/net/sourceforge/jnlp/SecurityDesc.java
>> --- a/netx/net/sourceforge/jnlp/SecurityDesc.java Thu Nov 04 16:44:27 2010 -0700
>> +++ b/netx/net/sourceforge/jnlp/SecurityDesc.java Fri Nov 05 12:23:29 2010 -0400
>> @@ -23,6 +23,9 @@
>> import java.security.*;
>> import java.awt.AWTPermission;
>>
>> +import net.sourceforge.jnlp.runtime.DeploymentConfiguration;
>> +import net.sourceforge.jnlp.runtime.JNLPRuntime;
>> +
>> /**
>> * The security element.
>> *
>> @@ -67,7 +70,6 @@
>> // queues, or even prevent access to security dialog queues.
>> //
>> // new AWTPermission("accessEventQueue"),
>> - new AWTPermission("showWindowWithoutWarningBanner"),
>> new RuntimePermission("exitVM"),
>> new RuntimePermission("loadLibrary"),
>> new RuntimePermission("queuePrintJob"),
>> @@ -105,7 +107,6 @@
>> new PropertyPermission("javaws.*", "read,write"),
>> new RuntimePermission("exitVM"),
>> new RuntimePermission("stopThread"),
>> - new AWTPermission("showWindowWithoutWarningBanner"),
>> // disabled because we can't at this time prevent an
>> // application from accessing other applications' event
>> // queues, or even prevent access to security dialog queues.
>> @@ -187,6 +188,11 @@
>> for (int i=0; i< sandboxPermissions.length; i++)
>> permissions.add(sandboxPermissions[i]);
>>
>> + String key = DeploymentConfiguration.KEY_SECURITY_ALLOW_HIDE_WINDOW_WARNING;
>> + if (Boolean.valueOf(JNLPRuntime.getConfiguration().getProperty(key)) == Boolean.TRUE) {
>> + permissions.add(new AWTPermission("showWindowWithoutWarningBanner"));
>> + }
>> +
>> if (file.isApplication())
>> for (int i=0; i< jnlpRIAPermissions.length; i++)
>> permissions.add(jnlpRIAPermissions[i]);
>> diff -r 8e66d9386273 netx/net/sourceforge/jnlp/runtime/DeploymentConfiguration.java
>> --- a/netx/net/sourceforge/jnlp/runtime/DeploymentConfiguration.java Thu Nov 04 16:44:27 2010 -0700
>> +++ b/netx/net/sourceforge/jnlp/runtime/DeploymentConfiguration.java Fri Nov 05 12:23:29 2010 -0400
>> @@ -17,6 +17,7 @@
>>
>> package net.sourceforge.jnlp.runtime;
>>
>> +import java.awt.AWTPermission;
>> import java.io.BufferedOutputStream;
>> import java.io.BufferedReader;
>> import java.io.File;
>> @@ -142,6 +143,15 @@
>> */
>> public static final String KEY_USER_NETX_RUNNING_FILE = "deployment.user.runningfile";
>>
>> + /** Boolean. Only show security prompts to user if true */
>> + public static final String KEY_SECURITY_PROMPT_USER = "deployment.security.askgrantdialog.show";
>> + /** Boolean. Only give AWTPermission("showWindowWithoutWarningBanner") if true */
>> + public static final String KEY_SECURITY_ALLOW_HIDE_WINDOW_WARNING = "deployment.security.sandbox.awtwarningwindow";
>> + /** Boolean. Only prompt user for granting any JNLP permissions if true */
>> + public static final String KEY_SECURITY_PROMPT_USER_FOR_JNLP = "deployment.security.sandbox.jnlp.enhanced";
>> + /** Boolean. Only install the custom authenticator if true */
>> + public static final String KEY_SECURITY_INSTALL_AUTHENTICATOR = "deployment.security.authenticator";
>> +
>
> Minor nitpick.. can you please add spaces between each of the
> common/decl. combo above? Would make it a bit easier to read in viewers
> that don't highlight..
>
Sure. Does the code in the updated patch look ok?
>> public enum ConfigType {
>> System, User
>> }
>> @@ -327,15 +337,15 @@
>> { "deployment.system.security.trusted.jssecerts", SYSTEM_SECURITY + File.separator + "trusted.jssecerts" },
>> { "deployment.system.security.trusted.clientautcerts", SYSTEM_SECURITY + File.separator + "trusted.clientcerts" },
>> /* security access and control */
>> - { "deployment.security.askgrantdialog.show", String.valueOf(true) },
>> + { KEY_SECURITY_PROMPT_USER, String.valueOf(true) },
>> { "deployment.security.askgrantdialog.notinca", String.valueOf(true) },
>> { "deployment.security.notinca.warning", String.valueOf(true) },
>> { "deployment.security.expired.warning", String.valueOf(true) },
>> { "deployment.security.jsse.hostmismatch.warning", String.valueOf(true) },
>> { "deployment.security.trusted.policy", null },
>> - { "deployment.security.sandbox.awtwarningwindow", String.valueOf(true) },
>> - { "deployment.security.sandbox.jnlp.enhanced", String.valueOf(true) },
>> - { "deployment.security.authenticator", String.valueOf(true) },
>> + { KEY_SECURITY_ALLOW_HIDE_WINDOW_WARNING, String.valueOf(true) },
>> + { KEY_SECURITY_PROMPT_USER_FOR_JNLP, String.valueOf(true) },
>> + { KEY_SECURITY_INSTALL_AUTHENTICATOR, String.valueOf(true) },
>> /* networking */
>> { "deployment.proxy.type", String.valueOf(PROXY_TYPE_BROWSER) },
>> { "deployment.proxy.same", String.valueOf(false) },
>> diff -r 8e66d9386273 netx/net/sourceforge/jnlp/security/SecurityWarning.java
>> --- a/netx/net/sourceforge/jnlp/security/SecurityWarning.java Thu Nov 04 16:44:27 2010 -0700
>> +++ b/netx/net/sourceforge/jnlp/security/SecurityWarning.java Fri Nov 05 12:23:29 2010 -0400
>> @@ -49,6 +49,7 @@
>> import javax.swing.SwingUtilities;
>>
>> import net.sourceforge.jnlp.JNLPFile;
>> +import net.sourceforge.jnlp.runtime.DeploymentConfiguration;
>> import net.sourceforge.jnlp.runtime.JNLPRuntime;
>>
>> /**
>> @@ -111,6 +112,11 @@
>> */
>> public static boolean showAccessWarningDialog(final AccessType accessType,
>> final JNLPFile file, final Object[] extras) {
>> +
>> + if (!shouldPromptUser()) {
>> + return false;
>> + }
>> +
>> final SecurityDialogMessage message = new SecurityDialogMessage();
>>
>> message.dialogType = DialogType.ACCESS_WARNING;
>> @@ -140,6 +146,10 @@
>> */
>> public static boolean showNotAllSignedWarningDialog(JNLPFile file) {
>>
>> + if (!shouldPromptUser()) {
>> + return false;
>> + }
>> +
>> final SecurityDialogMessage message = new SecurityDialogMessage();
>> message.dialogType = DialogType.NOTALLSIGNED_WARNING;
>> message.accessType = AccessType.NOTALLSIGNED;
>> @@ -174,6 +184,10 @@
>> public static boolean showCertWarningDialog(AccessType accessType,
>> JNLPFile file, CertVerifier jarSigner) {
>>
>> + if (!shouldPromptUser()) {
>> + return false;
>> + }
>> +
>> final SecurityDialogMessage message = new SecurityDialogMessage();
>> message.dialogType = DialogType.CERT_WARNING;
>> message.accessType = accessType;
>> @@ -200,6 +214,10 @@
>> */
>> public static int showAppletWarning() {
>>
>> + if (!shouldPromptUser()) {
>> + return 2;
>> + }
>> +
>
> That seems like a magic number.. what does 2 represent?
>
Yes, it does seems like a magic number. showAppletWarning returns an
integer; the comment just above the return block describes it:
// result 0 = Yes, 1 = No, 2 = Cancel
And the method itself returns 2 if selected value is not set. That said,
this method is currently unused. I dont know of any place in Netx or the
plugin that calls this method. In fact, I would like to remove it at
some point (see the FIXME in the javadoc for this method).
>> SecurityDialogMessage message = new SecurityDialogMessage();
>> message.dialogType = DialogType.APPLET_WARNING;
>>
>> @@ -295,4 +313,18 @@
>> return message.userResponse;
>> }
>>
>> + /**
>> + * Returns whether the current runtime configuration allows prompting user
>> + * for security warnings.
>> + *
>> + * @return true if security warnings should be shown to the user.
>> + */
>> + private static boolean shouldPromptUser() {
>> + boolean allowed = Boolean.valueOf(JNLPRuntime.getConfiguration()
>> + .getProperty(DeploymentConfiguration.KEY_SECURITY_PROMPT_USER));
>> +
>> + return allowed;
>> +
>> + }
>> +
>
> You can just return Boolean.valueOf(...) instead of creating an
> additional var and returning that.
>
Done. I had some code to print debugging messages between "allowed =
..." and "return allowed;" lines which I eventually removed. The result
is the strange-looking code above. Fixed in the updated patch.
>> }
>> diff -r 8e66d9386273 netx/net/sourceforge/jnlp/services/ServiceUtil.java
>> --- a/netx/net/sourceforge/jnlp/services/ServiceUtil.java Thu Nov 04 16:44:27 2010 -0700
>> +++ b/netx/net/sourceforge/jnlp/services/ServiceUtil.java Fri Nov 05 12:23:29 2010 -0400
>> @@ -39,6 +39,7 @@
>>
>> import net.sourceforge.jnlp.JNLPFile;
>> import net.sourceforge.jnlp.runtime.ApplicationInstance;
>> +import net.sourceforge.jnlp.runtime.DeploymentConfiguration;
>> import net.sourceforge.jnlp.runtime.JNLPRuntime;
>> import net.sourceforge.jnlp.security.SecurityWarning;
>> import net.sourceforge.jnlp.security.SecurityWarning.AccessType;
>> @@ -208,9 +209,10 @@
>> };
>>
>> /**
>> - * Returns whether the app requesting a service is signed. If the app is
>> - * unsigned, the user is prompted with a dialog asking if the action
>> - * should be allowed.
>> + * Returns whether the app requesting a JNLP service has the right permissions.
>> + * If it doesn't, user is prompted for permissions. This method should only be
>> + * used for JNLP API related permissions.
>> + *
>> * @param type the type of access being requested
>> * @param extras extra Strings (usually) that are passed to the dialog for
>> * message formatting.
>> @@ -221,8 +223,9 @@
>> }
>>
>> /**
>> - * Returns whether the app requesting a service has the right permissions.
>> - * If it doesn't, user is prompted for permissions.
>> + * Returns whether the app requesting a JNLP service has the right permissions.
>> + * If it doesn't, user is prompted for permissions. This method should only be
>> + * used for JNLP API related permissions.
>> *
>> * @param app the application which is requesting the check. If null, the current
>> * application is used.
>> @@ -265,6 +268,11 @@
>> }
>>
>> if (!codeTrusted) {
>> +
>> + if (!shouldPromptUser()) {
>> + return false;
>> + }
>> +
>> final AccessType tmpType = type;
>> final Object[] tmpExtras = extras;
>> final ApplicationInstance tmpApp = app;
>> @@ -285,4 +293,17 @@
>>
>> return true; //allow
>> }
>> +
>> + /**
>> + * Returns whether the current runtime configuration allows prompting the
>> + * user for JNLP permissions.
>> + *
>> + * @return true if the user should be prompted for JNLP API related permissions.
>> + */
>> + private static boolean shouldPromptUser() {
>> + boolean allowed = Boolean.valueOf(JNLPRuntime.getConfiguration()
>> + .getProperty(DeploymentConfiguration.KEY_SECURITY_PROMPT_USER_FOR_JNLP));
>> + return allowed;
>> + }
>> +
>> }
>
> Same as above.. no need for the 'allowed' var.
>
Fixed.
>> diff -r 8e66d9386273 plugin/icedteanp/java/sun/applet/PluginMain.java
>> --- a/plugin/icedteanp/java/sun/applet/PluginMain.java Thu Nov 04 16:44:27 2010 -0700
>> +++ b/plugin/icedteanp/java/sun/applet/PluginMain.java Fri Nov 05 12:23:29 2010 -0400
>> @@ -230,7 +230,11 @@
>> }
>>
>> // plug in a custom authenticator and proxy selector
>> - Authenticator.setDefault(new CustomAuthenticator());
>> + boolean installAuthenticator = Boolean.valueOf(JNLPRuntime.getConfiguration()
>> + .getProperty(DeploymentConfiguration.KEY_SECURITY_INSTALL_AUTHENTICATOR));
>> + if (installAuthenticator) {
>> + Authenticator.setDefault(new CustomAuthenticator());
>> + }
>> ProxySelector.setDefault(new PluginProxySelector());
>>
>> CookieManager ckManager = new PluginCookieManager();
>
> Rest looks fine to me!
>
Thanks for the review! Ok to commit the updated patch?
Cheers,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: icedtea-web-integrate-configuration-prompts-02.patch
Type: text/x-patch
Size: 10734 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20101110/1eae89e1/icedtea-web-integrate-configuration-prompts-02.patch
More information about the distro-pkg-dev
mailing list