From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 7003745943754375168 X-Received: by 2002:a2e:5c9:: with SMTP id 192mr9793938ljf.337.1630919239926; Mon, 06 Sep 2021 02:07:19 -0700 (PDT) X-BeenThere: isar-users@googlegroups.com Received: by 2002:ac2:4146:: with SMTP id c6ls763991lfi.2.gmail; Mon, 06 Sep 2021 02:07:18 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwMCpCwoeoqZpJKYKcE/dH3hBhqnrjaRus7C0EuPdW1ZwPrcQM/zCf0aI9cMuDQbnIsokfP X-Received: by 2002:a05:6512:32b7:: with SMTP id q23mr8777378lfe.125.1630919238820; Mon, 06 Sep 2021 02:07:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1630919238; cv=none; d=google.com; s=arc-20160816; b=YFPERHk+Iz4XRYWNQkSB83f6P3tKACYnsPzaUX6gr2wFOShvSaoS2xFcpHlEDcZO6o LAPEZMLIkO9wkAoMO7u25tMl4rpuWgesdni8DWXIXBnL+pUDldXdlAVnTFA1OVLzJSs8 Nc+1VZVMhnZUnCygU6X+EUGXkUPXZMQ5CVwe1+BVnhYeznyPo9tlLRBjl9vkVbD3FcQk m8e82aVZXmAhvVVJLwwEUEcpe7nwlL5ny1N40El7IAIurn8fTLWk90jpBkGC8TpaeToz /1I+WHzyyeWMEeXlWDhN4Z5VqvtV3jqtsHAuE4Jm/n9ORU45MBBhqn/7Lv3XSjRDJc5+ scwg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=StCNh0GnNxCWVkJ4YrESXnU1fmi+30vrKMOqcSIX02o=; b=ubo1NOgXAnDGBxmmM+M3sJsG9tmN/DTcumwJq/r2WC72ocp1ov9g13UOYAA5RZbGqS dQSn7eb3K0U64s+I9AcvPC3DrPqT7AHHpgXrsmupmi7PCsZNAvoHCi17FP2D/D6AzzeW pFmVm8kruOB7+bdYyQuG5TLBF8jTzdDorpqA7Xazrg/EwMyleBQACLdjbT9o0i99xASI Y0aLeVCyOXHFo1Xh2pl3iq8x6gUiRliz5vaNw1Ja6+23uQpkGtYS79HsVd4+Pj4F7NMz 6e5HJ/8CVxLkrdBGyZ22wUtH4p+/1EFcbwMGvq8yx3rLXE64a6ksuuH3o6tuz/BC0bOb kV0g== ARC-Authentication-Results: i=1; gmr-mx.google.com; spf=pass (google.com: domain of amikan@ilbers.de designates 85.214.156.166 as permitted sender) smtp.mailfrom=amikan@ilbers.de Return-Path: Received: from shymkent.ilbers.de (shymkent.ilbers.de. [85.214.156.166]) by gmr-mx.google.com with ESMTPS id b25si410722ljk.6.2021.09.06.02.07.18 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 06 Sep 2021 02:07:18 -0700 (PDT) Received-SPF: pass (google.com: domain of amikan@ilbers.de designates 85.214.156.166 as permitted sender) client-ip=85.214.156.166; Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of amikan@ilbers.de designates 85.214.156.166 as permitted sender) smtp.mailfrom=amikan@ilbers.de Received: from [192.168.67.164] (mm-70-70-214-37.mgts.dynamic.pppoe.byfly.by [37.214.70.70] (may be forged)) (authenticated bits=0) by shymkent.ilbers.de (8.15.2/8.15.2/Debian-8) with ESMTPSA id 18697FtS012418 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 6 Sep 2021 11:07:15 +0200 Subject: Re: [PATCH 2/2] isar-bootstrap: Do not let gpg-agent to stay running To: Henning Schild Cc: isar-users@googlegroups.com, Venkata.Pyla@toshiba-tsip.com References: <20210903163105.54003-1-amikan@ilbers.de> <20210903163105.54003-3-amikan@ilbers.de> <20210903193010.65525070@md1za8fc.ad001.siemens.net> From: Anton Mikanovich Message-ID: <9e132cae-982f-dca5-4ee8-acd7c291abb7@ilbers.de> Date: Mon, 6 Sep 2021 12:07:09 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <20210903193010.65525070@md1za8fc.ad001.siemens.net> Content-Type: multipart/alternative; boundary="------------135F31B61232AB6A4C8408C7" Content-Language: en-US X-TUID: dfnq0tLt8l1w This is a multi-part message in MIME format. --------------135F31B61232AB6A4C8408C7 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 03.09.2021 20:30, Henning Schild wrote: > Venkata found that /tmp/gpghomeXXXXXXX to be part of the rootfs. It not > being deleted had the same reason the agent was never killed. Even if incorrect check will not fail and gpg-agent will be killed, the second part of the original line >chroot "${ROOTFSDIR}" gpgconf --kill gpg-agent && /bin/rm -rf "${MY_GPGHOME}" will be executed outside the chroot, so /tmp/gpghome* will not be removed because of wrong path. Executing rm outside the chroot with corrected path (`rm -rf "${ROOTFSDIR}${MY_GPGHOME}"`) is more efficient and will fix the issue. >> + if [ "${@get_distro_needs_gpg_support(d)}" = "gnupg" -a -d >> "${ROOTFSDIR}${MY_GPGHOME}" ]; then > Why that new condition on get_distr_needs_gpg_support? I think this can > go away. In fact i think the whole if can go since the rm has a -f. > > If the condition needs to stay ... i think > > -n "${@get_distro_needs_gpg_support(d)}" > > would be better than "= gnupg" This one is needed because original `if [ -d "${ROOTFSDIR}${MY_GPGHOME}" ]` will be true in case there is no need in gpg-agent (MY_GPGHOME empty, but ROOTFSDIR exists). And that's why we also can't leave just `rm -rf "${ROOTFSDIR}${MY_GPGHOME}"` there. The check `= gnupg` was already used above, but yes I can rebuild previous check also. > And in fact we can probably keep using daemon mode and pull that kill > out of the if as well > > chroot "${ROOTFSDIR}" gpgconf --kill gpg-agent || true > rm -rf "${ROOTFSDIR}${MY_GPGHOME}" > > Could be a little more efficient, but probably not worth too much > investigation. Just choose what seems more readable and maintainable. In the original code the number of chroot executions was 2+[keys_number], the number of gpg-agent executions was 1. Now it is just [keys_number] for both. I don't think there will be critical performance drop. The first priority of this rebuild was it's stability. gpg-agent should not stays running even in case apt-key fails. It is also possible to move the loop code inside gpg-agent run command, but that will be much less readable. -- Anton Mikanovich Promwad Ltd. External service provider of ilbers GmbH Maria-Merian-Str. 8 85521 Ottobrunn, Germany +49 (89) 122 67 24-0 Commercial register Munich, HRB 214197 General Manager: Baurzhan Ismagulov --------------135F31B61232AB6A4C8408C7 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit
03.09.2021 20:30, Henning Schild wrote:
Venkata found that /tmp/gpghomeXXXXXXX to be part of the rootfs. It not
being deleted had the same reason the agent was never killed.
Even if incorrect check will not fail and gpg-agent will be killed, the second part of the original line
>chroot "${ROOTFSDIR}" gpgconf --kill gpg-agent && /bin/rm -rf "${MY_GPGHOME}"
will be executed outside the chroot, so /tmp/gpghome* will not be removed because of wrong path.
Executing rm outside the chroot with corrected path (`rm -rf "${ROOTFSDIR}${MY_GPGHOME}"`) is more efficient and will fix the issue.

+        if [ "${@get_distro_needs_gpg_support(d)}" = "gnupg" -a -d
"${ROOTFSDIR}${MY_GPGHOME}" ]; then

Why that new condition on get_distr_needs_gpg_support? I think this can
go away. In fact i think the whole if can go since the rm has a -f.

If the condition needs to stay ... i think
  
  -n "${@get_distro_needs_gpg_support(d)}"

would be better than "= gnupg"
This one is needed because original `if [ -d "${ROOTFSDIR}${MY_GPGHOME}" ]` will be true in case there is no need in gpg-agent (MY_GPGHOME empty, but ROOTFSDIR exists). And that's why we also can't leave just `rm -rf "${ROOTFSDIR}${MY_GPGHOME}"` there.
The check `= gnupg` was already used above, but yes I can rebuild previous check also.

And in fact we can probably keep using daemon mode and pull that kill
out of the if as well

chroot "${ROOTFSDIR}" gpgconf --kill gpg-agent || true
rm -rf "${ROOTFSDIR}${MY_GPGHOME}"

Could be a little more efficient, but probably not worth too much
investigation. Just choose what seems more readable and maintainable.
In the original code the number of chroot executions was 2+[keys_number], the number of gpg-agent executions was 1. Now it is just [keys_number] for both. I don't think there will be critical performance drop.
The first priority of this rebuild was it's stability. gpg-agent should not stays running even in case apt-key fails.
It is also possible to move the loop code inside gpg-agent run command, but that will be much less readable.
-- 
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov
--------------135F31B61232AB6A4C8408C7--