The Trunk: System-ul.1148.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: System-ul.1148.mcz

commits-2
Levente Uzonyi uploaded a new version of System to project The Trunk:
http://source.squeak.org/trunk/System-ul.1148.mcz

==================== Summary ====================

Name: System-ul.1148
Author: ul
Time: 24 March 2020, 9:15:28.593463 pm
UUID: 79bbf013-47f5-4f3c-9e42-8f169fe22a6c
Ancestors: System-ul.1147

- create a deep copy of the keys read from the file in SecurityManager >> #loadSecurityKeys, so that they can be modified later

=============== Diff against System-ul.1147 ===============

Item was changed:
  ----- Method: SecurityManager>>loadSecurityKeys (in category 'fileIn/out') -----
  loadSecurityKeys
  "SecurityManager default loadSecurityKeys"
  "Load the keys file for the current user"
  | fd loc file keys |
  self isInRestrictedMode ifTrue:[^self]. "no point in even trying"
  loc := self secureUserDirectory. "where to get it from"
  loc last = FileDirectory pathNameDelimiter ifFalse:[
  loc := loc copyWith: FileDirectory pathNameDelimiter.
  ].
  fd := FileDirectory on: loc.
  file := [fd readOnlyFileNamed: keysFileName]
  on: FileDoesNotExistException do:[:ex| nil].
  file ifNil:[^self]. "no keys file"
+ keys := (Object readFrom: file) veryDeepCopy. "Use #veryDeepCopy to make the non-writable literal objects created by the Compiler writable."
- keys := Object readFrom: file.
  privateKeyPair := keys first.
  trustedKeys := keys last.
  file close.!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-ul.1148.mcz

Eliot Miranda-2
Hi Levente,


> On Mar 24, 2020, at 1:15 PM, [hidden email] wrote:
>
> Levente Uzonyi uploaded a new version of System to project The Trunk:
> http://source.squeak.org/trunk/System-ul.1148.mcz
>
> ==================== Summary ====================
>
> Name: System-ul.1148
> Author: ul
> Time: 24 March 2020, 9:15:28.593463 pm
> UUID: 79bbf013-47f5-4f3c-9e42-8f169fe22a6c
> Ancestors: System-ul.1147
>
> - create a deep copy of the keys read from the file in SecurityManager >> #loadSecurityKeys, so that they can be modified later
>
> =============== Diff against System-ul.1147 ===============
>
> Item was changed:
>  ----- Method: SecurityManager>>loadSecurityKeys (in category 'fileIn/out') -----
>  loadSecurityKeys
>      "SecurityManager default loadSecurityKeys"
>      "Load the keys file for the current user"
>      | fd loc file keys |
>      self isInRestrictedMode ifTrue:[^self]. "no point in even trying"
>      loc := self secureUserDirectory. "where to get it from"
>      loc last = FileDirectory pathNameDelimiter ifFalse:[
>          loc := loc copyWith: FileDirectory pathNameDelimiter.
>      ].
>      fd := FileDirectory on: loc.
>      file := [fd readOnlyFileNamed: keysFileName]
>              on: FileDoesNotExistException do:[:ex| nil].
>      file ifNil:[^self]. "no keys file"
> +    keys := (Object readFrom: file) veryDeepCopy. "Use #veryDeepCopy to make the non-writable literal objects created by the Compiler writable."
> -    keys := Object readFrom: file.
>      privateKeyPair := keys first.
>      trustedKeys := keys last.
>      file close.!

Why not change Object class>>readFrom: to answer mutable objects?

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-ul.1148.mcz

K K Subbu
On 25/03/20 2:11 PM, Eliot Miranda wrote:
> Why not change Object class>>readFrom: to answer mutable objects?

+1. Since immutability has been introduced only now, it makes sense for
readFrom to return mutable objects and then use a message to turn it
readonly. E.g.

     (Object readfrom: file) beReadOnly

BTW, the object being read is a secret key. Wouldn't making it mutable
or allowing copies increase leak risk? Like mutability, we could mark
certain objects as secrets so that they are wiped clean during garbage
collection. Or, the garbage collector could wipe out the first word of
an object while finalization.

Squeak often loads such small objects from files. Is there a way to load
an entire object from a file in a single message (a counterpart to
#saveOnFileNamed:). This could even be made a primitive for efficiency.
The primitive can fail if the file exceeds a set limit (say 10% of free
memory).

Regards .. Subbu

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-ul.1148.mcz

Eliot Miranda-2
In reply to this post by Eliot Miranda-2
Hi Levente,

    take a look at Compiler-eem.421 and Kernel-eem.1320 in Inbox and see if you like the approach.

Name: Compiler-eem.421
Author: eem
Time: 25 March 2020, 9:00:37.328826 pm
UUID: 7bacb14a-f088-4fca-aa94-35dae28aa8e8
Ancestors: Compiler-nice.420

A suggested implementation of writable results from doits.  This scheme is careful to only make read-only collection literals that the compiler makes read-only, not e.g. numeric literals (which should always be read-only) or objects the evaluation makes read-only.

Name: Kernel-eem.1320
Author: eem
Time: 25 March 2020, 9:01:48.950717 pm
UUID: 5e03652f-dca4-40fb-991e-ec71a89bb282
Ancestors: Kernel-eem.1319

Use Compiler-eem.421 to make Object class>>readFrom: answer writable collection literals.

On Wed, Mar 25, 2020 at 1:41 AM Eliot Miranda <[hidden email]> wrote:
Hi Levente,


> On Mar 24, 2020, at 1:15 PM, [hidden email] wrote:
>
> Levente Uzonyi uploaded a new version of System to project The Trunk:
> http://source.squeak.org/trunk/System-ul.1148.mcz
>
> ==================== Summary ====================
>
> Name: System-ul.1148
> Author: ul
> Time: 24 March 2020, 9:15:28.593463 pm
> UUID: 79bbf013-47f5-4f3c-9e42-8f169fe22a6c
> Ancestors: System-ul.1147
>
> - create a deep copy of the keys read from the file in SecurityManager >> #loadSecurityKeys, so that they can be modified later
>
> =============== Diff against System-ul.1147 ===============
>
> Item was changed:
>  ----- Method: SecurityManager>>loadSecurityKeys (in category 'fileIn/out') -----
>  loadSecurityKeys
>      "SecurityManager default loadSecurityKeys"
>      "Load the keys file for the current user"
>      | fd loc file keys |
>      self isInRestrictedMode ifTrue:[^self]. "no point in even trying"
>      loc := self secureUserDirectory. "where to get it from"
>      loc last = FileDirectory pathNameDelimiter ifFalse:[
>          loc := loc copyWith: FileDirectory pathNameDelimiter.
>      ].
>      fd := FileDirectory on: loc.
>      file := [fd readOnlyFileNamed: keysFileName]
>              on: FileDoesNotExistException do:[:ex| nil].
>      file ifNil:[^self]. "no keys file"
> +    keys := (Object readFrom: file) veryDeepCopy. "Use #veryDeepCopy to make the non-writable literal objects created by the Compiler writable."
> -    keys := Object readFrom: file.
>      privateKeyPair := keys first.
>      trustedKeys := keys last.
>      file close.!

Why not change Object class>>readFrom: to answer mutable objects?


--
_,,,^..^,,,_
best, Eliot


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: System-ul.1148.mcz

Levente Uzonyi
Hi Eliot,

The original problem is that the keys stored in SecurityManager are Arrays
of LargeIntegers. The error comes from SecurityManager trying to set all
bytes of the LargePositiveInteger objects to 0 in order to erase the key
from memory.
Since your changes leave numeric literals non-writable, they won't help
with the original problem.


Levente

On Wed, 25 Mar 2020, Eliot Miranda wrote:

> Hi Levente,
>
>     take a look at Compiler-eem.421 and Kernel-eem.1320 in Inbox and see if you like the approach.
>
> Name: Compiler-eem.421
> Author: eem
> Time: 25 March 2020, 9:00:37.328826 pm
> UUID: 7bacb14a-f088-4fca-aa94-35dae28aa8e8
> Ancestors: Compiler-nice.420
>
> A suggested implementation of writable results from doits.  This scheme is careful to only make read-only collection literals that the compiler makes read-only, not e.g. numeric literals (which should always be read-only) or objects the evaluation makes read-only.
>
> Name: Kernel-eem.1320
> Author: eem
> Time: 25 March 2020, 9:01:48.950717 pm
> UUID: 5e03652f-dca4-40fb-991e-ec71a89bb282
> Ancestors: Kernel-eem.1319
>
> Use Compiler-eem.421 to make Object class>>readFrom: answer writable collection literals.
>
> On Wed, Mar 25, 2020 at 1:41 AM Eliot Miranda <[hidden email]> wrote:
>       Hi Levente,
>
>
>       > On Mar 24, 2020, at 1:15 PM, [hidden email] wrote:
>       >
>       > Levente Uzonyi uploaded a new version of System to project The Trunk:
>       > http://source.squeak.org/trunk/System-ul.1148.mcz
>       >
>       > ==================== Summary ====================
>       >
>       > Name: System-ul.1148
>       > Author: ul
>       > Time: 24 March 2020, 9:15:28.593463 pm
>       > UUID: 79bbf013-47f5-4f3c-9e42-8f169fe22a6c
>       > Ancestors: System-ul.1147
>       >
>       > - create a deep copy of the keys read from the file in SecurityManager >> #loadSecurityKeys, so that they can be modified later
>       >
>       > =============== Diff against System-ul.1147 ===============
>       >
>       > Item was changed:
>       >  ----- Method: SecurityManager>>loadSecurityKeys (in category 'fileIn/out') -----
>       >  loadSecurityKeys
>       >      "SecurityManager default loadSecurityKeys"
>       >      "Load the keys file for the current user"
>       >      | fd loc file keys |
>       >      self isInRestrictedMode ifTrue:[^self]. "no point in even trying"
>       >      loc := self secureUserDirectory. "where to get it from"
>       >      loc last = FileDirectory pathNameDelimiter ifFalse:[
>       >          loc := loc copyWith: FileDirectory pathNameDelimiter.
>       >      ].
>       >      fd := FileDirectory on: loc.
>       >      file := [fd readOnlyFileNamed: keysFileName]
>       >              on: FileDoesNotExistException do:[:ex| nil].
>       >      file ifNil:[^self]. "no keys file"
>       > +    keys := (Object readFrom: file) veryDeepCopy. "Use #veryDeepCopy to make the non-writable literal objects created by the Compiler writable."
>       > -    keys := Object readFrom: file.
>       >      privateKeyPair := keys first.
>       >      trustedKeys := keys last.
>       >      file close.!
>
>       Why not change Object class>>readFrom: to answer mutable objects?
>
>
>
> --
> _,,,^..^,,,_
> best, Eliot
>
>