[rfc][icedtea-web] Trusted-only manifest attribute

[Jiri Vanek]

On 03/20/2014 04:33 PM, Andrew Azores wrote:
> Hi,
>
> This implements the Trusted-only manifest attribute [0]. It's quite a simple check compared to some of the other new ones. A test case is included that verifies that specifying the attribute in the manifest, for a signed applet, without specifying the applet security in the applet HTML tag, is not allowed. More test cases will come later on.
>
> [0] http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/manifest.html#trusted_only

Thanx for prompt work!

Few nits:

+ ...  file.getManifestsAttributes().getAttribute(JNLPFile.ManifestsAttributes.TRUSTED_ONLY);

You do not need to bother with this. There is method for you in file.getManifestsAttributes() which returns  ManifestBoolean (TRUE, FALSE, UNDEFINED) for you.

It will affect also

+        if (!trustedOnly.equalsIgnoreCase("true")) {
+            OutputController.getLogger().log(OutputController.Level.WARNING_ALL, "Trusted Only manifest attribute is not recognized. Continuing anyway.");
+            return;
+        }

Instead of this your code will recieve IllegalArgumentException, And I 'm in favour to let it be thrown.

Instead of MESSAGE_ALL please go on with MESSAGE_DEBUG for those two messages

+        if (!(isFullySigned && SecurityDesc.ALL_PERMISSIONS.equals(desc))) {
+            OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
+                    "Trusted Only manifest attribute is \"true\". Applet is fully signed? " + isFullySigned
+                            + ". Applet is requesting permission level: " + securityType + ". This is fatal.");
+            throw new LaunchException(Translator.R("STrustedOnlyAttributeFailure", Boolean.toString(isFullySigned), securityType));
+        }


I would like to have the
OutputController.getLogger().log(OutputController.Level.ERROR_ALL, as MESSAFE_DEBUG and *before* the if itself.

Also the essafe is bit wierd :)

"Trusted Only manifest attribute is " + trustedOnly +  ". Applet's signing is " + signing + ". Applet is requesting permission level: " + securityType + ".");

Or similar.. moreover everything iexcept the "Applet is fully signed?" :)

Also the LaunchException itself -  Ithink the Boolean.toString(isFullySigned)  nor securityType is necessary. All is known in this state.
So just throw exception with explanation without variables (thay may be in only one state or not? isFullySigned==true and desc!=SecurityDesc.ALL_PERMISSIONS) )


Also maybe different value for null and for "rest" in case of securityType string.


The reproducer is missing javaws parts and is missing correct case. Minimalistical  reproducer should be
- applet signed  trusted-only=false
- applet signed  trusted-only=true
- applet signed  trusted-only=illegal
- applet mixed signatures  trusted-only=false
- applet mixed signatures  trusted-only=false
- applet mixed signatures  trusted-only=illegal
- applet not signed  trusted-only=false
- applet not signed  trusted-only=true
- applet not signed  trusted-only=illegal
- javaws  signed  trusted-only=false
- javaws  signed  trusted-only=true
- javaws  signed  trusted-only=illegal
- javaws  not signed  trusted-only=false
- javaws  not signed  trusted-only=true
- javaws  not signed  trusted-only=illegal

However it is to much work. In long-term it would be nice to have them. For now, just extends your:
applet not signed  trusted-only=true
by:
javaws not signed  trusted-only=true


Also youu do not need custom reproducer for this!  Unsigned applet with manifest is classical simple reproducer :)

thanx!
   J.