[rfc][icedtea-web] New PartiallySigned Dialog

[Andrew Azores]

On 03/06/2014 11:19 AM, Jiri Vanek wrote:
> On 03/04/2014 11:07 PM, Andrew Azores wrote:
>> Hi,
>>
>> This patch introduces a new PartiallySigned dialog to replace the 
>> NotAllSigned prompt. This new dialog uses the same abstracted parent 
>> class that was pulled out of the Unsigned dialog, so it uses the same 
>> remembered action storage and has the same general look and feel. 
>> This dialog also has a Sandbox button, just like CertWarningPane 
>> recently gained for fully signed applets, which allows partially 
>> signed ones to also be run with only sandbox permissions. Some more 
>> security info is also present on the dialog, eg the applet's 
>> publisher and codebase. Not yet included is a new Help text for this 
>> dialog, but this can be written up separately IMO. Right now it will 
>> just display the same Help as the Unsigned dialog.
>>
>> ChangeLog:
>> Added new PartiallySigned Dialog to replace NotAllSignedWarningPane.
>> Also includes a Sandbox button.
>> * netx/net/sourceforge/jnlp/resources/Messages.properties:
>> (APPEXTSecunsignedAppletActionSandbox, LPartiallySignedApplet,
>> LPartiallySignedAppletUserDenied) new messages
>> * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java:
>> Logic added for displaying new PartiallySigned dialog.
>> (showNotAllSignedDialog) removed. (getSigningState) new method.
>> (promptUserOnPartialSigning, userPromptedForPartialSigning) new 
>> methods for
>> SecurityDelegate.
>> * netx/net/sourceforge/jnlp/security/AppTrustWarningDialog.java:
>> (partiallySigned) new method
>> * netx/net/sourceforge/jnlp/security/AppTrustWarningPanel.java:
>> (chosenActionSetter) refactored to allow Sandbox action. 
>> (setupInfoPanel) applet
>> title made overrideable by subclasses
>> * netx/net/sourceforge/jnlp/security/SecurityDialog.java: 
>> (NOTALLSIGNED_WARNING)
>> renamed PARTIALLYSIGNED_WARNING, display new dialog rather than old
>> * netx/net/sourceforge/jnlp/security/SecurityDialogs.java: 
>> (NOTALLSIGNED_WARNING)
>> renamed PARTIALLYSIGNED_WARNING. (showNotAllSignedWarningDialog) 
>> removed.
>> (showPartiallySignedWarningDialog) new method
>> * 
>> netx/net/sourceforge/jnlp/security/appletextendedsecurity/ExecuteAppletAction.java:
>> Added Sandbox action
>> * 
>> netx/net/sourceforge/jnlp/security/appletextendedsecurity/UnsignedAppletTrustConfirmation.java:
>> (checkPartiallySignedWithUserIfRequired) new method
>> * 
>> tests/reproducers/custom/SignedAppletCodebaseLoading/testcases/SignedAppletCodebaseLoadingTests.java:
>> test now passes since dialog will not appear if applet security is 
>> set to Low.
>> KnownToFail removed.
>> * 
>> tests/reproducers/custom/SignedAppletExternalMainClass/testcases/SignedAppletExternalMainClassTest.java:
>> same
>> * 
>> netx/net/sourceforge/jnlp/security/PartiallySignedAppTrustWarningPanel.java:
>> new class
>> * netx/net/sourceforge/jnlp/security/NotAllSignedWarningPane.java: 
>> deleted
>> in favour of PartiallySignedAppTrustWarningPanel
>>
>> Thanks,
>>
> Ouch - "legacy" dialogs packages? :) Please dont forget to adapt 
> changelog. For patch I think there will be more rounds. Also pls move 
> your new dialogue to proper pacage.package

Updated.

>
> General thoughts:
>
> SANDBOX_ALWAYS is not (going to be) supported? If so, the checbox and 
> radiobuttons should be disabled (I have not noted it but I could 
> overlook). But I' +1 to support SANDBOX_ALWAYS

It's supported here. It's disabled for CertWarningPane ie fully signed 
just because in that case we aren't using the nice extended security 
action storage - it just uses the certificate keystore, so there wasn't 
a clean and easy way to incorporate remembering the sandbox decision, as 
far as I could tell. But since this one is based off of the Extended 
Applet Security stuff, remembering this action is no problem.

>  - if this will be supported, then there is going to be fun with the 
> sandbox+custom policies saving :)

What do you mean? It will allow partially signed applets to always be 
run with a user-defined set of extra permissions above the sandbox level 
but below all-permission. This was the exact overarching objective of 
all of these patches.

>
> The removal of NotAllSignedWarningPane is not compelte. Please check 
> also used Trasnlator.R keys. If they are dangling, please rmeove them 
> from all Messages*.properties

They were being reused in the PartiallySigned dialog, so no removals. I 
renamed them though.

>
> You have removed:
> -    @KnownToFail
> -        assertTrue("NotAllSigned dialog will appear if this test 
> runs. Remove this exception and KnownToFail "
> -                + "when a proper replacement is in place", false);
>
> How come? I think partialy signed dialogue should appear always, no 
> matter of EAS level.
> Or not?

I can see the motivation behind having it always appear, since partially 
signed applets seem to be the most inherently untrustworthy class, but 
from a user perspective it also seems like it makes little sense for 
only these applets to have a dialog appear. Maybe a new, even lower 
level could be made to display no dialogs ever, and partially signed 
will appear at any level above this? I'm not sure this is really worthwhile.

>
> +    protected String getAppletTitle() {
> +        return R("SAppletTitle", file.getTitle());
> +    }
>
> (And few more methods about title) Wasn't this handled in previous push?

Not sure what you mean.

>
> Can we have some testing main method as I added recently for unsigned 
> warnng dialogue?

It will be messy, since it will require a mocked SecurityDialog, which 
only has package-private constructors, and these classes are no longer 
in the same package.

>
>
> hmmm . . . I ahve just spotted usage of
> type = AccessType.SIGNING_ERROR;

Where?

> In this case this dialogue have sense also for JNLP files.
> Then ~/icedtea-web-image/bin/javaws -allowredirect 
> http://java.net/projects/electric/downloads/download/WebStartFiles/electricAnt.jnlp 
> would be an example... But it is something for think about for future 
> work.
>

Yes, this dialog is built to depend only on JNLPFile, not PluginBridge, 
and JNLPClassLoader's checks that cause it to appear are also not 
restricted only for browser applets.

>
>
> +    private String getSigningInfo() {
> +        CertInformation info = 
> jcv.getCertInformation(jcv.getCertPath(null));
> +
> +        AccessType type;
> +        if (info != null && info.isRootInCacerts() && 
> !info.hasSigningIssues()) {
> +            type = AccessType.VERIFIED;
> +        } else if (info != null && info.isRootInCacerts()) {
> +            type = AccessType.SIGNING_ERROR;
> +        } else {
> +            type = AccessType.UNVERIFIED;
> +        }
> +
> +        String mainText = "";
> +        switch (type) {
> +            case VERIFIED:
> +                mainText = R("SSigVerified");
> +                break;
> +            case UNVERIFIED:
> +                mainText = R("SSigUnverified");
> +                break;
> +            case SIGNING_ERROR:
> +                mainText = R("SSignatureError");
> +                break;
> +        }
> +
> +        return mainText;
> +    }
>
> ouh thats looong way to return one of three strings :)
>
>
> Good work otherwise,
>   Thanx
>    J.

Yea, what the heck was I doing there... jeez. Thanks for pointing this out.

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: partially-signed-dialog-new-packages.patch
Type: text/x-patch
Size: 46414 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140310/9ebae4f6/partially-signed-dialog-new-packages-0001.patch>