[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-build-dev
Subject:    Re: macOS build success but codesign fail on macOS 10.13.5 or older
From:       Erik Joelsson <erik.joelsson () oracle ! com>
Date:       2020-02-28 14:26:58
Message-ID: 9aaa4389-490e-56ce-a181-27189aaece61 () oracle ! com
[Download RAW message or body]

On 2020-02-28 01:04, Magnus Ihse Bursie wrote:
> On 2020-02-28 09:59, Magnus Ihse Bursie wrote:
>> On 2020-02-27 16:07, Erik Joelsson wrote:
>>> On 2020-02-27 06:16, Magnus Ihse Bursie wrote:
>>>> I don't think it should be a fatal error. If you have a codesign 
>>>> binary on your path that does not support --option runtime, you 
>>>> should still be able to build, but not sign. Change it to a 
>>>> warning, and let the user continue without CODESIGN.
>>>>
>>> My interpretation of this patch is that the new check is only 
>>> performed if a valid --with-macosx-codesign-identity was provided, 
>>> so the user has clearly requested signing to be performed. In that 
>>> case I agree that it should error out.
>>
>> I'm sorry Erik, but that is not open to "interpretation". Look at the 
>> code:
>>
>>     UTIL_PATH_PROGS(CODESIGN, codesign)
>>
>>     if test "x$CODESIGN" != "x"; then
>>       # Check for user provided code signing identity.
>>       # If no identity was provided, fall back to "openjdk_codesign".
>>       AC_ARG_WITH([macosx-codesign-identity], 
>> [AS_HELP_STRING([--with-macosx-codesign-identity],
>>         [specify the code signing identity])],
>> [MACOSX_CODESIGN_IDENTITY=$with_macosx_codesign_identity],
>>         [MACOSX_CODESIGN_IDENTITY=openjdk_codesign]
>>       )
>>
>>       AC_SUBST(MACOSX_CODESIGN_IDENTITY)
>>
>>       # Verify that the codesign certificate is present
>>       AC_MSG_CHECKING([if codesign certificate is present])
>>       $RM codesign-testfile
>>       $TOUCH codesign-testfile
>>       $CODESIGN -s "$MACOSX_CODESIGN_IDENTITY" codesign-testfile 
>> 2>&AS_MESSAGE_LOG_FD >&AS_MESSAGE_LOG_FD || CODESIGN=
>>       $RM codesign-testfile
>>       if test "x$CODESIGN" = x; then
>>         AC_MSG_RESULT([no])
>>       else
>>         AC_MSG_RESULT([yes])
>>        # Verify that the codesign has --option runtime
>>        AC_MSG_CHECKING([if codesign has --option runtime])
>>        $RM codesign-testfile
>>        $TOUCH codesign-testfile
>>        $CODESIGN --option runtime -s "$MACOSX_CODESIGN_IDENTITY" 
>> codesign-testfile 2>&AS_MESSAGE_LOG_FD >&AS_MESSAGE_LOG_FD || CODESIGN=
>>        $RM codesign-testfile
>>        if test "x$CODESIGN" = x; then
>>          AC_MSG_ERROR([codesign does not have --option runtime. macOS 
>> 10.13.6 and above is required.])
>>        else
>>          AC_MSG_RESULT([yes])
>>        fi
>>       fi
>>     fi
>>
>> This means that if you have a binary named "codesign" on your path, 
>> and it does not accept the '--option runtime' argument, configure 
>> will fail.
> Sorry, my bad: configure will fail if you have codesign, and 
> openjdk_codesign is a valid codesign identity, but --option runtime is 
> not supported. This does indeed limit the impact of this patch. 
> Nevertheless, I still think this is bad design. If the code would e.g. 
> check that --with-macosx-codesign-identity was explicitly given on the 
> command line, then it would be OK to fail.
>
I agree that an explicit check that --with-macosx-codesign-identity was 
set would make the code clearer. The point is that if you have a valid 
identity defined, but you are building on an older MacOS version so the 
--option runtime is not supported, the build will unconditionally fail 
at the first signing attempt anyway. This is the proper fail fast mechanism.

On a side note, there is also no point in supporting signing on older 
macs without the --option runtime because such signatures are basically 
useless today.

/Erik

> /Magnus
>
>>
>> This is not acceptable.
>>
>> However, I understand that there is a need to be able to *enforce* 
>> signing. I'm actually currently working with a patch that will add 
>> --enable-jdk-feature-codesign, and if that is enabled, configure will 
>> fail unless a working codesign binary and certificate is present. It 
>> will be easy to adapt this change as well. But in the meantime, the 
>> AC_MSG_ERROR must be changed to a warning.
>>
>> /Magnus
>
[prev in list] [next in list] [prev in thread] [next in thread]