Re: org-crypt leaking data when encryption password is not entered twice (was: Please document the caching and its user options)

2024-07-02 Thread Daniel Clemente
> > In addition, „leaving some encrypted sections unencrypted for a short
> > amount of time, and closing and reopening the buffer during that time“
> > isn't a bug, it's a possible user behaviour that we can't control. But
> > org-crypt can mention that that behaviour is unsafe when using on-disk
> > cache. Or detect it (if it's fast) and warn the user.
>
> I did not mean that opening/closing buffer is a bug.
> And I do not see why this behavior is unsafe, sorry.
>

Closing a buffer is unsafe when this happens at the same time:
1. you have org-element-cache-persistent set to t (the default)
2. you're using org-crypt and have temporarily decrypted a region but
for some reason not reencrypted yet (maybe you plan to reencrypt it
some hours later today)
3. the cache directory is in a place which is not as private as the
org file. E.g. a network disk, an unencrypted hard drive etc.

It's unsafe because closing the buffer triggers saving the org-element
cache to disk.

> >
> > Disabling backups makes sense too, if we decide that unencrypted
> > private data shouldn't end up in backups.
> > I don't have an absolute opinion. Some people may prefer having
> > backups of all data (including private unencrypted data).
>
> Actually, thinking about it more, I realize that backups may never
> contain unencrypted data as long as we never write this unencrypted data
> when saving normally. That's because backup is always taken from disk
> and never from the buffer contents.
>
> So, the real problem to solve is how to _reliably_ prevent the
> unencrypted data to be saved onto the disk.
>

If that works, that's good.


> > If it's possible to detect whether encryption failed in this buffer,
> > there could be a warning saying „Last encryption failed. Really
> > save?“.
>
> Yes. In fact, `org-entrypt-entries' throws an error when encryption
> fails. However, this error is displayed as a simple message, which is
> immediately hidden by "Wrote ..." message emitted a bit later.
>
> This is because `basic-save-buffer' has
>
> ;; Don't let errors prevent saving the buffer.
> (with-demoted-errors "Before-save hook error: %S"
>   (run-hooks 'before-save-hook))
>
> If we use `write-contents-functions' instead of `before-save-hook',
> there should be no such problem.
>

It seems so.
But can you also prevent auto-save from saving it?
I randomly found in tar-mode.el line 860 that auto-save doesn't call
the write-contents-functions hook.


> What I am saying is that "we might have bugs, so be careful" is not
> something we need to write in the documentation. The only exception is
> when there is a known, long-living bug, that we cannot fix quickly and
> must warn users about.
>

The needed message isn't „we might have bugs, so be careful“
… but „if for some reason you keep org-crypt sections unencrypted for
long periods of time, be careful“.
That situation may come from user behaviour (e.g. not having enabled
org-crypt-use-before-save-magic), not from bugs.



> > It would be different If we had a third component… E.g. imagine we had
> > a component/overlay/text property/… in Emacs that could tell whether a
> > buffer's region contains very private information or not; then all
> > other components could just obey that setting (that section won't be
> > backed up, it won't end up in disk cache, … It can even be displayed
> > in a different face). Then org-crypt just needs to set that flag when
> > encryption fails. Does something like that exist? Anyway this is a bit
> > utopic or overengineered. Simpler ways of improving things are with
> > documentation (e.g. „Don't do this, it's unsafe“), with messages
> > („You're doing this, which may be unsafe“), or with questions („Really
> > do this unsafe thing?“)
>
> Sounds interesting, but I am afraid that this idea is too abstract. It
> is not clear what Emacs is supposed to do with such regions. Maybe Eli
> has something better to say.
>

By the way, there's already reveal-mode which marks some private
sections and replaces them with asterisks. For instance if you edit
~/.authinfo and write this
machine some_pc  login my_user port su password my_password
…then you'll see
machine some_pc  login my_user port su password 
unless you position the cursor there.
It seems it's a display thing only, using overlays. I thought that
maybe unencrypted org-crypt sections could be marked or displayed like
that (with * when unfocused), and then org-element could detect them
and avoid caching those parts. But I'm not sure if org-persist sees
overlays.
Anyway this is beyond my Emacs knowledge and I'm speculating.
 also don't know whether „unfocused unencrypted org-crypt sections“
replaced by asterisks would look nice.



Re: org-crypt leaking data when encryption password is not entered twice (was: Please document the caching and its user options)

2024-06-27 Thread Ihor Radchenko
Daniel Clemente  writes:

>> One simple idea is to disable backups if encryption fails.
>> Or use `write-contents-functions' instead of `before-save-hook' - that
>> way, Emacs will not ignore errors thrown by org-crypt and will not
>> actually save anything if encryption fails.
>>
>
> Disabling backups makes sense too, if we decide that unencrypted
> private data shouldn't end up in backups.
> I don't have an absolute opinion. Some people may prefer having
> backups of all data (including private unencrypted data).

Actually, thinking about it more, I realize that backups may never
contain unencrypted data as long as we never write this unencrypted data
when saving normally. That's because backup is always taken from disk
and never from the buffer contents.

So, the real problem to solve is how to _reliably_ prevent the
unencrypted data to be saved onto the disk.

> If it's possible to detect whether encryption failed in this buffer,
> there could be a warning saying „Last encryption failed. Really
> save?“.

Yes. In fact, `org-entrypt-entries' throws an error when encryption
fails. However, this error is displayed as a simple message, which is
immediately hidden by "Wrote ..." message emitted a bit later.

This is because `basic-save-buffer' has

;; Don't let errors prevent saving the buffer.
(with-demoted-errors "Before-save hook error: %S"
  (run-hooks 'before-save-hook))

If we use `write-contents-functions' instead of `before-save-hook',
there should be no such problem.

> Or just a message in the style of „Encryption failed. Saving the file
> may store unencrypted data in disk, and in backups and cache if
> enabled“.
>
> Totally preventing the user from saving a file seems harsh but it also
> seems safer. Since users have different safety preferences, Emacs can
> let the user decide what the do, through a question or optional
> setting.

I agree that "saving prevention" must be a user option.

>> These things should be considered bugs. And we should fix them. Cache and
>> other libraries should not be responsible for special treatment of
>> optional org-crypt library.
>>
>
> You can't fix all bugs all the time, so you can't base security on „we
> strongly believe there are no more bugs“.

I did not suggest that.
What I am saying is that "we might have bugs, so be careful" is not
something we need to write in the documentation. The only exception is
when there is a known, long-living bug, that we cannot fix quickly and
must warn users about.

> ... If doing an extra
> verification (to avoid storing private data on disk in unencrypted
> form) is fast, it's better with the verification.

>> Cache and other libraries should not be responsible for special
>> treatment of optional org-crypt library.
>
> That's arbitrary. Both persistent cache and org-crypt are optional,
> but any of them can check whether the other is enabled and try to do
> what the user wants.
> I know they both have separate responsibilities, but if there are only
> these 2 parts, one of them must be the one caring about „unencrypted
> data leaking into disk caches“.

Sure. But I meant that we should still write this code in org-crypt
library, not inside org-persist. This is more of a technical detail and
code style.

> In addition, „leaving some encrypted sections unencrypted for a short
> amount of time, and closing and reopening the buffer during that time“
> isn't a bug, it's a possible user behaviour that we can't control. But
> org-crypt can mention that that behaviour is unsafe when using on-disk
> cache. Or detect it (if it's fast) and warn the user.

I did not mean that opening/closing buffer is a bug.
And I do not see why this behavior is unsafe, sorry.

> It would be different If we had a third component… E.g. imagine we had
> a component/overlay/text property/… in Emacs that could tell whether a
> buffer's region contains very private information or not; then all
> other components could just obey that setting (that section won't be
> backed up, it won't end up in disk cache, … It can even be displayed
> in a different face). Then org-crypt just needs to set that flag when
> encryption fails. Does something like that exist? Anyway this is a bit
> utopic or overengineered. Simpler ways of improving things are with
> documentation (e.g. „Don't do this, it's unsafe“), with messages
> („You're doing this, which may be unsafe“), or with questions („Really
> do this unsafe thing?“)

Sounds interesting, but I am afraid that this idea is too abstract. It
is not clear what Emacs is supposed to do with such regions. Maybe Eli
has something better to say.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: org-crypt leaking data when encryption password is not entered twice (was: Please document the caching and its user options)

2024-06-27 Thread Daniel Clemente
>
> As for not typing the same password twice and not using
> org-crypt-use-before-save-magic, we should somehow fix this.
> (I am starting a new thread branch.)
>

„Not using org-crypt-use-before-save-magic“ is currently a user
decision, not a bug.
For instance, I don't use it because it adds around 5 seconds to each
saving of a large file. If it were instantaneous I would enable it.
With it disabled, this explains why I often find unencrypted sections
at the end of the day… I have to rely on myself to reencrypt them
again.



> One simple idea is to disable backups if encryption fails.
> Or use `write-contents-functions' instead of `before-save-hook' - that
> way, Emacs will not ignore errors thrown by org-crypt and will not
> actually save anything if encryption fails.
>

Disabling backups makes sense too, if we decide that unencrypted
private data shouldn't end up in backups.
I don't have an absolute opinion. Some people may prefer having
backups of all data (including private unencrypted data).

If it's possible to detect whether encryption failed in this buffer,
there could be a warning saying „Last encryption failed. Really
save?“.
Or just a message in the style of „Encryption failed. Saving the file
may store unencrypted data in disk, and in backups and cache if
enabled“.

Totally preventing the user from saving a file seems harsh but it also
seems safer. Since users have different safety preferences, Emacs can
let the user decide what the do, through a question or optional
setting.


> > At the end of the day when I do "git diff" + "git commit" sometimes I
> > realize there's unencrypted data and then I have to reencrypt it. In
> > the meantime I might have killed and reopened the buffer, thus
> > updating the file cache.
> > That may be a problem by org-encrypt and something to document in
> > org-crypt itself. The point is that users of org-encrypt should take
> > extra precautions when enabling org-element-cache-persistent. Like:
> > not closing buffers while the sections are unencrypted.
>
> These things should be considered bugs. And we should fix them. Cache and
> other libraries should not be responsible for special treatment of
> optional org-crypt library.
>

You can't fix all bugs all the time, so you can't base security on „we
strongly believe there are no more bugs“. If doing an extra
verification (to avoid storing private data on disk in unencrypted
form) is fast, it's better with the verification.

In addition, „leaving some encrypted sections unencrypted for a short
amount of time, and closing and reopening the buffer during that time“
isn't a bug, it's a possible user behaviour that we can't control. But
org-crypt can mention that that behaviour is unsafe when using on-disk
cache. Or detect it (if it's fast) and warn the user.

> Cache and
> other libraries should not be responsible for special treatment of
> optional org-crypt library.

That's arbitrary. Both persistent cache and org-crypt are optional,
but any of them can check whether the other is enabled and try to do
what the user wants.
I know they both have separate responsibilities, but if there are only
these 2 parts, one of them must be the one caring about „unencrypted
data leaking into disk caches“.

It would be different If we had a third component… E.g. imagine we had
a component/overlay/text property/… in Emacs that could tell whether a
buffer's region contains very private information or not; then all
other components could just obey that setting (that section won't be
backed up, it won't end up in disk cache, … It can even be displayed
in a different face). Then org-crypt just needs to set that flag when
encryption fails. Does something like that exist? Anyway this is a bit
utopic or overengineered. Simpler ways of improving things are with
documentation (e.g. „Don't do this, it's unsafe“), with messages
(„You're doing this, which may be unsafe“), or with questions („Really
do this unsafe thing?“)