8194534: Manifest better support

Reviewed-by: mchung, igerasim

src/java.base/share/classes/java/net/URLClassLoader.java

@@ -535,13 +535,13 @@ public class URLClassLoader extends SecureClassLoader implements Closeable {
      * @spec JPMS
      */
     protected Package definePackage(String name, Manifest man, URL url) {
-        String path = name.replace('.', '/').concat("/");
         String specTitle = null, specVersion = null, specVendor = null;
         String implTitle = null, implVersion = null, implVendor = null;
         String sealed = null;
         URL sealBase = null;
 
-        Attributes attr = man.getAttributes(path);
+        Attributes attr = SharedSecrets.javaUtilJarAccess()
+                .getTrustedAttributes(man, name.replace('.', '/').concat("/"));
         if (attr != null) {
             specTitle   = attr.getValue(Name.SPECIFICATION_TITLE);
             specVersion = attr.getValue(Name.SPECIFICATION_VERSION);
@@ -585,10 +585,12 @@ public class URLClassLoader extends SecureClassLoader implements Closeable {
     /*
      * Returns true if the specified package name is sealed according to the
      * given manifest.
+     *
+     * @throws SecurityException if the package name is untrusted in the manifest
      */
     private boolean isSealed(String name, Manifest man) {
-        String path = name.replace('.', '/').concat("/");
+        Attributes attr = SharedSecrets.javaUtilJarAccess()
-        Attributes attr = man.getAttributes(path);
+                .getTrustedAttributes(man, name.replace('.', '/').concat("/"));
         String sealed = null;
         if (attr != null) {
             sealed = attr.getValue(Name.SEALED);

src/java.base/share/classes/java/util/jar/JarFile.java

@@ -417,10 +417,10 @@ class JarFile extends ZipFile {
             if (manEntry != null) {
                 if (verify) {
                     byte[] b = getBytes(manEntry);
-                    man = new Manifest(new ByteArrayInputStream(b), getName());
                     if (!jvInitialized) {
                         jv = new JarVerifier(b);
                     }
+                    man = new Manifest(jv, new ByteArrayInputStream(b), getName());
                 } else {
                     man = new Manifest(super.getInputStream(manEntry), getName());
                 }

src/java.base/share/classes/java/util/jar/JarVerifier.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -866,4 +866,24 @@ class JarVerifier {
     static CodeSource getUnsignedCS(URL url) {
         return new VerifierCodeSource(null, url, (java.security.cert.Certificate[]) null);
     }
+
+    /**
+     * Returns whether the name is trusted. Used by
+     * {@link Manifest#getTrustedAttributes(String)}.
+     */
+    boolean isTrustedManifestEntry(String name) {
+        // How many signers? MANIFEST.MF is always verified
+        CodeSigner[] forMan = verifiedSigners.get(JarFile.MANIFEST_NAME);
+        if (forMan == null) {
+            return true;
+        }
+        // Check sigFileSigners first, because we are mainly dealing with
+        // non-file entries which will stay in sigFileSigners forever.
+        CodeSigner[] forName = sigFileSigners.get(name);
+        if (forName == null) {
+            forName = verifiedSigners.get(name);
+        }
+        // Returns trusted if all signers sign the entry
+        return forName != null && forName.length == forMan.length;
+    }
 }

src/java.base/share/classes/java/util/jar/JavaUtilJarAccessImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -60,4 +60,9 @@ class JavaUtilJarAccessImpl implements JavaUtilJarAccess {
     public List<Object> getManifestDigests(JarFile jar) {
         return jar.getManifestDigests();
     }
+
+    public Attributes getTrustedAttributes(Manifest man, String name) {
+        return man.getTrustedAttributes(name);
+    }
+
 }

src/java.base/share/classes/java/util/jar/Manifest.java

@@ -50,10 +50,13 @@ import sun.security.util.SecurityProperties;
 public class Manifest implements Cloneable {
 
     // manifest main attributes
-    private Attributes attr = new Attributes();
+    private final Attributes attr = new Attributes();
 
     // manifest entries
-    private Map<String, Attributes> entries = new HashMap<>();
+    private final Map<String, Attributes> entries = new HashMap<>();
+
+    // associated JarVerifier, not null when called by JarFile::getManifest.
+    private final JarVerifier jv;
 
     // name of the corresponding jar archive if available.
     private final String jarFilename;
@@ -63,6 +66,7 @@ public class Manifest implements Cloneable {
      */
     public Manifest() {
         jarFilename = null;
+        jv = null;
     }
 
     /**
@@ -72,8 +76,7 @@ public class Manifest implements Cloneable {
      * @throws IOException if an I/O error has occurred
      */
     public Manifest(InputStream is) throws IOException {
-        this();
+        this(null, is, null);
-        read(is);
     }
 
     /**
@@ -84,8 +87,17 @@ public class Manifest implements Cloneable {
      * @throws IOException if an I/O error has occured
      */
     Manifest(InputStream is, String jarFilename) throws IOException {
+        this(null, is, jarFilename);
+    }
+
+    /**
+     * Constructs a new Manifest from the specified input stream
+     * and associates it with a JarVerifier.
+     */
+    Manifest(JarVerifier jv, InputStream is, String jarFilename) throws IOException {
         read(is);
         this.jarFilename = jarFilename;
+        this.jv = jv;
     }
 
     /**
@@ -94,9 +106,10 @@ public class Manifest implements Cloneable {
      * @param man the Manifest to copy
      */
     public Manifest(Manifest man) {
-        this();
         attr.putAll(man.getMainAttributes());
         entries.putAll(man.getEntries());
+        jarFilename = null;
+        jv = man.jv;
     }
 
     /**
@@ -146,6 +159,23 @@ public class Manifest implements Cloneable {
         return getEntries().get(name);
     }
 
+    /**
+     * Returns the Attributes for the specified entry name, if trusted.
+     *
+     * @param name entry name
+     * @return returns the same result as {@link #getAttributes(String)}
+     * @throws SecurityException if the associated jar is signed but this entry
+     *      has been modified after signing (i.e. the section in the manifest
+     *      does not exist in SF files of all signers).
+     */
+    Attributes getTrustedAttributes(String name) {
+        Attributes result = getAttributes(name);
+        if (result != null && jv != null && ! jv.isTrustedManifestEntry(name)) {
+            throw new SecurityException("Untrusted manifest entry: " + name);
+        }
+        return result;
+    }
+
     /**
      * Clears the main Attributes as well as the entries in this Manifest.
      */

src/java.base/share/classes/jdk/internal/loader/BuiltinClassLoader.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -60,6 +60,7 @@ import java.util.jar.Attributes;
 import java.util.jar.Manifest;
 import java.util.stream.Stream;
 
+import jdk.internal.misc.SharedSecrets;
 import jdk.internal.misc.VM;
 import jdk.internal.module.ModulePatcher.PatchedModuleReader;
 import jdk.internal.module.Resources;
@@ -862,7 +863,8 @@ public class BuiltinClassLoader
      * Manifest are used to get the package version and sealing information.
      *
      * @throws IllegalArgumentException if the package name duplicates an
-     * existing package either in this class loader or one of its ancestors
+     *      existing package either in this class loader or one of its ancestors
+     * @throws SecurityException if the package name is untrusted in the manifest
      */
     private Package definePackage(String pn, Manifest man, URL url) {
         String specTitle = null;
@@ -875,7 +877,8 @@ public class BuiltinClassLoader
         URL sealBase = null;
 
         if (man != null) {
-            Attributes attr = man.getAttributes(pn.replace('.', '/').concat("/"));
+            Attributes attr = SharedSecrets.javaUtilJarAccess()
+                    .getTrustedAttributes(man, pn.replace('.', '/').concat("/"));
             if (attr != null) {
                 specTitle = attr.getValue(Attributes.Name.SPECIFICATION_TITLE);
                 specVersion = attr.getValue(Attributes.Name.SPECIFICATION_VERSION);
@@ -921,10 +924,12 @@ public class BuiltinClassLoader
     /**
      * Returns {@code true} if the specified package name is sealed according to
      * the given manifest.
+     *
+     * @throws SecurityException if the package name is untrusted in the manifest
      */
     private boolean isSealed(String pn, Manifest man) {
-        String path = pn.replace('.', '/').concat("/");
+        Attributes attr = SharedSecrets.javaUtilJarAccess()
-        Attributes attr = man.getAttributes(path);
+                .getTrustedAttributes(man, pn.replace('.', '/').concat("/"));
         String sealed = null;
         if (attr != null)
             sealed = attr.getValue(Attributes.Name.SEALED);

src/java.base/share/classes/jdk/internal/misc/JavaUtilJarAccess.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -30,8 +30,10 @@ import java.net.URL;
 import java.security.CodeSource;
 import java.util.Enumeration;
 import java.util.List;
+import java.util.jar.Attributes;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
+import java.util.jar.Manifest;
 
 public interface JavaUtilJarAccess {
     public boolean jarFileHasClassPathAttribute(JarFile jar) throws IOException;
@@ -41,4 +43,5 @@ public interface JavaUtilJarAccess {
     public Enumeration<JarEntry> entries2(JarFile jar);
     public void setEagerValidation(JarFile jar, boolean eager);
     public List<Object> getManifestDigests(JarFile jar);
+    public Attributes getTrustedAttributes(Manifest man, String name);
 }

test/lib/jdk/test/lib/util/JarUtils.java

@@ -23,6 +23,7 @@
 
 package jdk.test.lib.util;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -271,6 +272,11 @@ public final class JarUtils {
         changes = new HashMap<>(changes);
 
         System.out.printf("Creating %s from %s...\n", dest, src);
+
+        if (dest.equals(src)) {
+            throw new IOException("src and dest cannot be the same");
+        }
+
         try (JarOutputStream jos = new JarOutputStream(
                 new FileOutputStream(dest))) {
 
@@ -298,6 +304,22 @@ public final class JarUtils {
         System.out.println();
     }
 
+    /**
+     * Update the Manifest inside a jar.
+     *
+     * @param src the original jar file name
+     * @param dest the new jar file name
+     * @param man the Manifest
+     *
+     * @throws IOException
+     */
+    public static void updateManifest(String src, String dest, Manifest man)
+            throws IOException {
+        ByteArrayOutputStream bout = new ByteArrayOutputStream();
+        man.write(bout);
+        updateJar(src, dest, Map.of(JarFile.MANIFEST_NAME, bout.toByteArray()));
+    }
+
     private static void updateEntry(JarOutputStream jos, String name, Object content)
            throws IOException {
         if (content instanceof Boolean) {