Subject: Re: [PATCH v3 1/2] services: Add secret-service-type.

From: Jan Nieuwenhuizen <[email-masked]>
Date: Tue, 01 Sep 2020 15:38:51 +0200

Ludovic Courtès writes:

Hello,

> "Jan (janneke) Nieuwenhuizen" <[email-masked]> skribis:
>
>> This adds a "secret-service" that can be added to a Childhurd VM to receive
>> out-of-band secrets (keys) sent from the host.
>>
>> Co-authored-by: Ludovic Courtès <[email-masked]>
>>
>> * gnu/services/virtualization.scm (secret-service-activation): New procedure.
>> (secret-service-type): New variable.
>> * gnu/build/secret-service.scm: New file.
>> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
>
> Very nice!  Minor suggestions:

Great!

>> +  (format (current-error-port) "secret-service-send-secrets\n")
>
> Perhaps write “sending secrets to ~a:~a...~%” or similar.

Ok.

>> +  (let ((sock (socket AF_INET SOCK_STREAM 0))
>> +        (addr (make-socket-address AF_INET INADDR_LOOPBACK port)))
>> +    ;; connect to wait for port
>> +    (let loop ((retry retry))
>> +      (if (zero? retry)
>> +          (error "connecting to childhurd failed")
>
> s/childhurd/secret server/

Ah, sure.

>> +          (catch 'system-error
>> +            (lambda _
>> +              (connect sock addr))
>> +            (lambda (key . args)
>> +              (format (current-error-port) "connect failed: ~a ~s\n" key args)
>
> Perhaps remove print “retrying connection” (or similar), and re-throw
> the exception when RETRY is zero, so that it goes through as is (and
> thus you can remove the call to ‘error’ above.)

Ah yes, changed it to

      (catch 'system-error
        (cut connect sock addr)
        (lambda (key . args)
          (when (zero? retry)
            (apply throw key args))
          (format (current-error-port) "retrying connection~%")
          (sleep 1)
          (loop (1- retry)))))

>> +    ;; copy tree
>> +    (let* ((files (if secret-root (find-files secret-root) '()))
>> +           (files-sizes-modes (map file->file+size+mode files))
>> +           (secrets `(secrets
>> +                      (version 0)
>> +                      (files ,files-sizes-modes))))
>> +      (write secrets sock)
>> +      (for-each (compose (cute display <> sock)
>> +                         (cute with-input-from-file <> read-string))
>
> Instead of loading it all in memory, we can use ‘dump-port’ from (guix
> build utils) here.

Nice, changed to

      (for-each (compose (cute dump-port <> sock)
                         (cute open-input-file <>))
                files))))

> That’s it!

Thanks for your suggestions,
Janneke

-- 
Jan Nieuwenhuizen <[email-masked]> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com