bug#66245: [PATCH] ; Silence macOS 14 warning

bug#66245: [PATCH] ; Silence macOS 14 warning

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

Tags: patch

Hi,

After updating to macOS 14 (and rebuilding Emacs), I see the following
warning whenever I start Emacs:

    WARNING: Secure coding is not enabled for restorable state! Enable secure coding by implementing NSApplicationDelegate.applicationSupportsSecureRestorableState: and returning YES.

This patch does exactly what the warning suggests, and it silences the
warning.

TBH I'm not entirely sure I understand the implications of implementing
`applicationSupportsSecureRestorableState`.  IIUC it makes Emacs opt-in
to the "secure state restoration" mechanism in contrast to a former
(supposedly less secure) mechanism, but AFAICT Emacs doesn't opt-in to
state restoration in the NS port in the first place...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Silence-macOS-14-warning.patch --]
[-- Type: text/patch, Size: 738 bytes --]

From 10a9205535c20ff951c61fb84a32b9ab6511fc67 Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Wed, 27 Sep 2023 20:46:26 +0200
Subject: [PATCH] ; Silence macOS 14 warning

* src/nsterm.m: Implement 'applicationSupportsSecureRestorableState'
to silence warning on macOS 14.
---
 src/nsterm.m | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/nsterm.m b/src/nsterm.m
index 4e0dfa58c63..e46b71a499b 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -6110,6 +6110,11 @@ - (void)antialiasThresholdDidChange:(NSNotification *)notification
 
 */
 
+- (BOOL) applicationSupportsSecureRestorableState:(NSApplication *)app
+{
+  return YES;
+}
+
 - (void) terminate: (id)sender
 {
   struct input_event ie;
-- 
2.42.0

bug#66245: [PATCH] ; Silence macOS 14 warning

[Alan Third]

On Wed, Sep 27, 2023 at 09:00:45PM +0200, Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
> Tags: patch
> 
> Hi,
> 
> After updating to macOS 14 (and rebuilding Emacs), I see the following
> warning whenever I start Emacs:
> 
>     WARNING: Secure coding is not enabled for restorable state! Enable secure coding by implementing NSApplicationDelegate.applicationSupportsSecureRestorableState: and returning YES.
> 
> This patch does exactly what the warning suggests, and it silences the
> warning.
> 
> TBH I'm not entirely sure I understand the implications of implementing
> `applicationSupportsSecureRestorableState`.  IIUC it makes Emacs opt-in
> to the "secure state restoration" mechanism in contrast to a former
> (supposedly less secure) mechanism, but AFAICT Emacs doesn't opt-in to
> state restoration in the NS port in the first place...

A description of what this fixes is here:

    https://sector7.computest.nl/post/2022-08-process-injection-breaking-all-macos-security-layers-with-a-single-vulnerability/

I'm not sure if making this change will affect us, as I don't think we
support saved states, although I could be wrong.

Is it possible for you to try a before and after test of how Emacs
handles saving the state over a reboot? That is, have a running Emacs
with open files and reboot, tick the "reopen windows when logging back
in" option, and see if it behaves differently with this patch applied
and not applied?

If it doesn't then I think this is probably safe and won't affect us,
so we should apply it. Otherwise we'll need to examine what's changed
and see if we can work around it.

Thanks!
-- 
Alan Third

bug#66245: [PATCH] ; Silence macOS 14 warning

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Alan,

> I'm not sure if making this change will affect us, as I don't think we
> support saved states, although I could be wrong.

Yes, that's what I thought too.

> Is it possible for you to try a before and after test of how Emacs
> handles saving the state over a reboot? That is, have a running Emacs
> with open files and reboot, tick the "reopen windows when logging back
> in" option, and see if it behaves differently with this patch applied
> and not applied?

I tried that now, and I couldn't see any difference.  With and without
my patch, Emacs starts after reboot and shows the usual *scratch*
buffer, with no sign of the buffers/files that I had open before
rebooting.  (That could have been nice though!)

bug#66245: [PATCH] ; Silence macOS 14 warning

[Alan Third]

On Thu, Sep 28, 2023 at 03:46:36PM +0200, Eshel Yaron wrote:
> > Is it possible for you to try a before and after test of how Emacs
> > handles saving the state over a reboot? That is, have a running Emacs
> > with open files and reboot, tick the "reopen windows when logging back
> > in" option, and see if it behaves differently with this patch applied
> > and not applied?
> 
> I tried that now, and I couldn't see any difference.  With and without
> my patch, Emacs starts after reboot and shows the usual *scratch*
> buffer, with no sign of the buffers/files that I had open before
> rebooting.  (That could have been nice though!)

Thank you for testing that.

I think this should go into emacs-29, but it's unclear to me what the
(security) implications are. This change is required to fix
CVE-2021-30873, which is rated "high", however it's over a year old at
this point, and given that Apple are requiring us to explicitly set
this in our code rather than forcing it on us, does that mean they
don't consider it that big of a deal?

Eli, Stefan, any thoughts? Does this look bad enough to force a new
Emacs 29 release?

The link with the in-depth explanation again:

    https://sector7.computest.nl/post/2022-08-process-injection-breaking-all-macos-security-layers-with-a-single-vulnerability/

CVE info:

    https://nvd.nist.gov/vuln/detail/CVE-2021-30873
-- 
Alan Third

bug#66245: [PATCH] ; Silence macOS 14 warning

[Stefan Kangas]

Alan Third <alan@idiocy.org> writes:

> Eli, Stefan, any thoughts? Does this look bad enough to force a new
> Emacs 29 release?
>
> The link with the in-depth explanation again:
>
>     https://sector7.computest.nl/post/2022-08-process-injection-breaking-all-macos-security-layers-with-a-single-vulnerability/

Let's see if I understand this right.

Without this code, are we enabling malicious processes to escape the
macOS sandbox, and gain the same privileges as the Emacs process?

It is presumably easy for some malware to just test all processes on the
machine until one is found to be vulnerable, right?  So they don't have
to specifically target Emacs?

The full exploit chain there is not very easy to understand, but it
seems like several techniques are used for some of the more nasty stuff,
and some of the steps have been fixed already.  There can be other ways
to do the same thing of course.  So I'm not sure what to say about the
urgency of fixing this; it could be urgent, or it could wait until 29.2.
What is your view?

Another thing.  The link says:

    Nevertheless, if you write an Objective-C application, please make
    sure you add -applicationSupportsSecureRestorableState: to return
    TRUE and to adapt secure coding for all classes used for your saved
    states!

Do we use "secure coding for all classes used for saved states", or does
that also need to be fixed?

BTW, any idea why we're only hearing about it now?

bug#66245: [PATCH] ; Silence macOS 14 warning

[Alan Third]

On Thu, Sep 28, 2023 at 03:16:21PM -0700, Stefan Kangas wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > Eli, Stefan, any thoughts? Does this look bad enough to force a new
> > Emacs 29 release?
> >
> > The link with the in-depth explanation again:
> >
> >     https://sector7.computest.nl/post/2022-08-process-injection-breaking-all-macos-security-layers-with-a-single-vulnerability/
> 
> Let's see if I understand this right.
> 
> Without this code, are we enabling malicious processes to escape the
> macOS sandbox, and gain the same privileges as the Emacs process?

As I understand it, yes.

I'm not sure that Emacs has any particularly noteworthy privileges,
though. The example they give is an application that has installer
type privileges, which I doubt Emacs would ever have or need.

> It is presumably easy for some malware to just test all processes on the
> machine until one is found to be vulnerable, right?  So they don't have
> to specifically target Emacs?

Possibly. I'm not entirely clear. I think the process is to create a
fake "state" file and put it in the right place on the users machine
and the next time they reboot it will use that file.

> The full exploit chain there is not very easy to understand, but it
> seems like several techniques are used for some of the more nasty stuff,
> and some of the steps have been fixed already.  There can be other ways
> to do the same thing of course.  So I'm not sure what to say about the
> urgency of fixing this; it could be urgent, or it could wait until 29.2.
> What is your view?

I'm not sure either. Is there a rough timeline for the release of
29.2? I feel like this is perhaps not very urgent, but if we're
talking, say, three or four months or more we maybe don't want to wait
that long.

> Another thing.  The link says:
> 
>     Nevertheless, if you write an Objective-C application, please make
>     sure you add -applicationSupportsSecureRestorableState: to return
>     TRUE and to adapt secure coding for all classes used for your saved
>     states!
> 
> Do we use "secure coding for all classes used for saved states", or does
> that also need to be fixed?

I believe that's what Eshel's patch does.

> BTW, any idea why we're only hearing about it now?

I guess Eshel's the first person to try building with the relevant
version of xcode who's noticed and reported the message. However that
version of xcode must have come out over a year ago (going by the date
on that article) so I don't know why nobody's noticed it before now.

My Mac is years behind, and I rarely build Emacs on it, so I don't get
these messages at all.
-- 
Alan Third

bug#66245: [PATCH] ; Silence macOS 14 warning

[Yuan Fu]

> 
>> BTW, any idea why we're only hearing about it now?
> 
> I guess Eshel's the first person to try building with the relevant
> version of xcode who's noticed and reported the message. However that
> version of xcode must have come out over a year ago (going by the date
> on that article) so I don't know why nobody's noticed it before now.
> 
> My Mac is years behind, and I rarely build Emacs on it, so I don't get
> these messages at all.

MacOS 14 is just released, I think. I’ve been building Emacs on macOS 13 and haven’t seen this warning.

Yuan

bug#66245: [PATCH] ; Silence macOS 14 warning

[Stefan Kangas]

Alan Third <alan@idiocy.org> writes:

> I'm not sure that Emacs has any particularly noteworthy privileges,
> though. The example they give is an application that has installer
> type privileges, which I doubt Emacs would ever have or need.

One thing we do commonly have, I think, is access to the Documents
directory.

OTOH, on GNU/Linux we typically don't really have any special protection
for user files.

>> The full exploit chain there is not very easy to understand, but it
>> seems like several techniques are used for some of the more nasty stuff,
>> and some of the steps have been fixed already.  There can be other ways
>> to do the same thing of course.  So I'm not sure what to say about the
>> urgency of fixing this; it could be urgent, or it could wait until 29.2.
>> What is your view?
>
> I'm not sure either. Is there a rough timeline for the release of
> 29.2? I feel like this is perhaps not very urgent, but if we're
> talking, say, three or four months or more we maybe don't want to wait
> that long.

I don't think we have a rough timeline for 29.2 as of now.  I'm leaning
towards just including this in the next release as usual, since the bug
only affects the macOS port, and anyways, and IIUC, depends on other
things being vulnerable to be exploited.

But I'm very open to being convinced otherwise, if anyone sees any
problems with that.

Eli, do you have any comments here?

bug#66245: [PATCH] ; Silence macOS 14 warning

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Fri, 29 Sep 2023 02:34:52 -0700
> Cc: Eshel Yaron <me@eshelyaron.com>, 66245@debbugs.gnu.org, eliz@gnu.org
> 
> Eli, do you have any comments here?

I had hard time catching up on all the emails for the last 2 days, but
I finally posted a response a few minutes ago.

bug#66245: [PATCH] ; Silence macOS 14 warning

[Gerd Möllmann]

Stefan Kangas <stefankangas@gmail.com> writes:

> Alan Third <alan@idiocy.org> writes:
>
>> Eli, Stefan, any thoughts? Does this look bad enough to force a new
>> Emacs 29 release?
>>
>> The link with the in-depth explanation again:
>>
>>     https://sector7.computest.nl/post/2022-08-process-injection-breaking-all-macos-security-layers-with-a-single-vulnerability/
>
> Let's see if I understand this right.
>
> Without this code, are we enabling malicious processes to escape the
> macOS sandbox, and gain the same privileges as the Emacs process?

Well, not that drastically...  From the release notes of macOS 12 Appkit
(we're now at 14).

https://developer.apple.com/documentation/macos-release-notes/appkit-release-notes-for-macos-12?changes=lat__5_3

Restorable State

    To enable secure coding for a restorable state, implement
    applicationSupportsSecureRestorableState(_:). When opted in:

        The system requires classes passed to restorationClass to
        explicitly conform to NSWindowRestoration.

        ...

I understand that as meaning that this switches on additional checks in
Appkit.  That should be okay for Emacs because it doesn't use this
feature of Appkit, at least AFAIK.

> It is presumably easy for some malware to just test all processes on the
> machine until one is found to be vulnerable, right?  So they don't have
> to specifically target Emacs?
>
> The full exploit chain there is not very easy to understand, but it
> seems like several techniques are used for some of the more nasty stuff,
> and some of the steps have been fixed already.  There can be other ways
> to do the same thing of course.  So I'm not sure what to say about the
> urgency of fixing this; it could be urgent, or it could wait until 29.2.
> What is your view?
>
> Another thing.  The link says:
>
>     Nevertheless, if you write an Objective-C application, please make
>     sure you add -applicationSupportsSecureRestorableState: to return
>     TRUE and to adapt secure coding for all classes used for your saved
>     states!
>
> Do we use "secure coding for all classes used for saved states", or does
> that also need to be fixed?
>
> BTW, any idea why we're only hearing about it now?

I guess Apple is more and more turning on "Secure Coding" stuff in their
libs.

bug#66245: [PATCH] ; Silence macOS 14 warning

[Stefan Kangas]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

>> Without this code, are we enabling malicious processes to escape the
>> macOS sandbox, and gain the same privileges as the Emacs process?
>
> Well, not that drastically...  From the release notes of macOS 12 Appkit
> (we're now at 14).
>
> https://developer.apple.com/documentation/macos-release-notes/appkit-release-notes-for-macos-12?changes=lat__5_3
>
> Restorable State
>
>     To enable secure coding for a restorable state, implement
>     applicationSupportsSecureRestorableState(_:). When opted in:
>
>         The system requires classes passed to restorationClass to
>         explicitly conform to NSWindowRestoration.
>
>         ...
>
> I understand that as meaning that this switches on additional checks in
> Appkit.  That should be okay for Emacs because it doesn't use this
> feature of Appkit, at least AFAIK.

Thanks.  IIUC, that seems to speak in favor of not making an emergency
release of Emacs 29.2 at this point.

bug#66245: [PATCH] ; Silence macOS 14 warning

[Gerd Möllmann]

Stefan Kangas <stefankangas@gmail.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>>> Without this code, are we enabling malicious processes to escape the
>>> macOS sandbox, and gain the same privileges as the Emacs process?
>>
>> Well, not that drastically...  From the release notes of macOS 12 Appkit
>> (we're now at 14).
>>
>> https://developer.apple.com/documentation/macos-release-notes/appkit-release-notes-for-macos-12?changes=lat__5_3
>>
>> Restorable State
>>
>>     To enable secure coding for a restorable state, implement
>>     applicationSupportsSecureRestorableState(_:). When opted in:
>>
>>         The system requires classes passed to restorationClass to
>>         explicitly conform to NSWindowRestoration.
>>
>>         ...
>>
>> I understand that as meaning that this switches on additional checks in
>> Appkit.  That should be okay for Emacs because it doesn't use this
>> feature of Appkit, at least AFAIK.
>
> Thanks.  IIUC, that seems to speak in favor of not making an emergency
> release of Emacs 29.2 at this point.

I agree.  The new method would just enable "secure coding" for
restorable state on macOS < 14 (it's the default in 14), but since we're
not using this stuff to begin with, it's kind of pointless.

bug#66245: [PATCH] ; Silence macOS 14 warning

[Alan Third]

On Fri, Sep 29, 2023 at 12:11:50PM +0200, Gerd Möllmann wrote:
> >> I understand that as meaning that this switches on additional checks in
> >> Appkit.  That should be okay for Emacs because it doesn't use this
> >> feature of Appkit, at least AFAIK.
> >
> > Thanks.  IIUC, that seems to speak in favor of not making an emergency
> > release of Emacs 29.2 at this point.
> 
> I agree.  The new method would just enable "secure coding" for
> restorable state on macOS < 14 (it's the default in 14), but since we're
> not using this stuff to begin with, it's kind of pointless.

Not quite true according to the long description linked elsewhere.
Saved state is turned on automatically and it seems there's no way to
opt out. As I understand it that means that even though we don't make
use of it the vulnerable code still executes.

I do agree though that it's probably not worth making an emergency
release.
-- 
Alan Third

bug#66245: [PATCH] ; Silence macOS 14 warning

[Eli Zaretskii]

> Cc: 66245@debbugs.gnu.org
> Date: Thu, 28 Sep 2023 22:47:34 +0100
> From: Alan Third <alan@idiocy.org>
> 
> Eli, Stefan, any thoughts? Does this look bad enough to force a new
> Emacs 29 release?

I don't understand this issue clearly enough to have an opinion, so I
trust you guys to decide whether this is important.

In any case, if we are worth our salt, Emacs 29.2 should be just a
month or two away, so I wouldn't make any emergency releases before
that, just for this issue.  After all, it isn't like Emacs doesn't
work, it just annoys you with a message at startup, right?

bug#66245: [PATCH] ; Silence macOS 14 warning

[Stefan Kangas]

Version: 29.2

Eshel Yaron via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs@gnu.org> writes:

> Tags: patch
>
> Hi,
>
> After updating to macOS 14 (and rebuilding Emacs), I see the following
> warning whenever I start Emacs:
>
>     WARNING: Secure coding is not enabled for restorable state! Enable secure coding by implementing NSApplicationDelegate.applicationSupportsSecureRestorableState: and returning YES.
>
> This patch does exactly what the warning suggests, and it silences the
> warning.

Thanks, this patch is now installed on emacs-29 ([1: a4185f87bd0]).

I'm also closing the bug, but this obviously doesn't preclude further
discussion.

[1: a4185f87bd0]: 2023-09-29 13:32:55 +0200
  ; Silence macOS 14 warning
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=a4185f87bd0e5c129ce93a56b5a3330e2d6b1776