[GitHub] tomcat pull request #123: Checking the signing service response

[adriannistor <gi...@git.apache.org>]

GitHub user adriannistor opened a pull request:

    https://github.com/apache/tomcat/pull/123

    Checking the signing service response

    The code at lines [398](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L398) --- [403](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L403) in file [SignCode.java](https://github.com/apache/tomcat/blob/trunk/java/org/apache/tomcat/buildutil/SignCode.java) assumes, without
    checking, that the signing service will return the same number of
    files and in the same order as in the `List<File>` files parameter ([line 393](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L393)).
    
    If the signing service returns fewer file, then
    
    `zis.getNextEntry()` ([line 400](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L400))
    
    will return `null`
    
    and 
    
    `zis.read(buf)` ([line 402](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L402))
    
    will fail (because there is no entry to read from).
    
    Similarly, if the order is different, 
    
    `fos.write(buf, 0 , numRead);`   ([line 403](https://github.com/apache/tomcat/blob/1fde5d696a5d23ed589d05fb6cf6307d5adb563c/java/org/apache/tomcat/buildutil/SignCode.java#L403))
    
    will overwrite the wrong files.
    
    The above two assumptions about the signing service are fine, but it
    is best to have a sanity check for them.
    
    This pull request adds a sanity check for the number of files.
    
    The full check (which also checks for the name equality) is:
    
    ```
    ZipEntry zisEntry;
    if (((zisEntry = zis.getNextEntry()) == null) || !zisEntry.getName().equals(files.get(i).getName())) {
        throw new BuildException("Signing failed. Malformed service reply.");
    }
    ```
    
    I am not familiar enough with Tomcat code to be sure the
    `zisEntry.getName()` and `files.get(i).getName()` calls are correct,
    so this pull request does not have name checks (but you can just
    copy-paste the above code for the name checks).

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/adriannistor/tomcat checkSigningServiceResult

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/tomcat/pull/123.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #123
    
----
commit addad0cec94d2535a435de50a6d1bb066fa90b33
Author: Adrian <ad...@...>
Date:   2018-10-02T21:07:09Z

    Sanity check: the signing service returned the same number of files as the List<File> files parameter.  See pull request for a check on the file names too.

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #123: Checking the signing service response

[markt-asf <gi...@git.apache.org>]

Github user markt-asf commented on the issue:

    https://github.com/apache/tomcat/pull/123
  
    Thanks for reviewing this code and providing a patch. Unfortunately, we plan to shortly move away from our custom signing service and switch to the solution provided by DigiCert that integrates a command line tool with the signing service that we can then call from our build script.
    
    Therefore, we will not be applying this PR.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat pull request #123: Checking the signing service response

[markt-asf <gi...@git.apache.org>]

Github user markt-asf closed the pull request at:

    https://github.com/apache/tomcat/pull/123


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org