Re: Signed archives (preliminary patch)

[Nikita Karetnikov]

>> I think the current docstring of ‘assert-valid-signature’ is not correct
>> since ‘signature’ must be a string (as produced by
>> ‘canonical-sexp->string’), not an sexp.

> In guix/nar.scm, the comment is:

>   (define (assert-valid-signature signature hash file)
>     ;; Bail out if SIGNATURE, an sexp, doesn't match HASH, a bytevector
>     ;; containing the expected hash for FILE.

> and indeed, SIGNATURE must be a string here.

>> Similarly, the “signature is not a valid s-expression” and “corrupt
>> signature data” messages are a bit confusing due to the way
>> ‘string->canonical-sexp’ works (try ‘string->canonical-sexp "foo"’).
>> But I may be wrong about the latter.

> Ah right, you could get “corrupt signature data” when
> (string->canonical-sexp signature) returns the null canonical sexp,
> whereas you’d want “not a valid s-expression”.

> Well, we can fix that in a separate patch if you want.

Do you have time for this?  I think it would be much easier for you than
for me because you wrote the bindings.

>> +(define* (assert-valid-signature signature hash port
>> +                                 #:optional (acl (current-acl)))
>> +  ;; Bail out if SIGNATURE, a string, doesn't match HASH, a bytevector
>> +  ;; containing the expected hash for PORT.

> Make it a docstring.

> Also, please make this change a separate patch.

OK.

>> +  (let* ((file      (port-filename port))

> I don’t think this will work, because most of the time PORT is a pipe
> (an input port), whereas FILE is supposed to be the name of the file
> being restored.

What can we do about it?  Should the function accept ‘file’ and ‘port’?

> > +                (raise (condition (&message (message "invalid hash"))
>> +                                  (&nar-invalid-hash-error
>> +                                   (port port) (file file)
>> +                                   (signature signature)
>> +                                   (expected (hash-data->bytevector data))
>> +                                   (actual hash)))))
>> +            (raise (condition (&message (message "unauthorized public key"))
>> +                              (&nar-signature-error
>> +                               (signature signature) (file file) (port 
>> port)))))
>> +        (raise (condition
>> +                (&message (message "corrupt signature data"))
>> +                (&nar-signature-error
>> +                 (signature signature) (file file) (port port)))))))

> Actually, the problem with making ‘assert-valid-signature’ public is
> that it raises &nar error conditions.

> It could be changed to raise a more generic &signature-error, but then
> ‘restore-file-set’ would have to guard against it to re-throw it along
> with a &nar-error (making a compound condition.)  And then ui.scm would
> figure it out.  Blech.

> It’s worth factorizing, but I don’t see how to do it nicely.  Thoughts?

Haven’t thought of it yet.  But I’ll try to take care of this one.

pgpsMg_mM46kL.pgp
Description: PGP signature