Re: [PATCH 3/3] gnu: icedtea-6: Generate keystore.

[Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>]

Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:
>
>> From: Ricardo Wurmus <rekado@elephly.net>
>>
>> * gnu/packages/java.scm (icedtea-6)[arguments]: Add phase
>> "install-keystore".
>> [native-inputs]: Add nss-certs and openssl.
>
> [...]
>
>> +         (add-after 'install 'install-keystore
>> +           (lambda* (#:key inputs outputs #:allow-other-keys)
>
> Could you add a comment to explain what’s going on here?

Okay, I’ll add a comment.

> Too bad IceTea’s build system doesn’t take care of that.

Yeah, there is an old bug report about this, but its resolution has been
pushed to later releases repeatedly.  All distributions have their own
bash scripts to generate a keystore.

>> +             (let* ((keystore  "cacerts")
>> +                    (certs-dir (string-append (assoc-ref inputs "nss-certs")
>> +                                              "/etc/ssl/certs"))
>> +                    (keytool   (string-append (assoc-ref outputs "jdk")
>> +                                              "/bin/keytool"))
>> +                    (openssl   (which "openssl"))
>> +                    (recent    (date->time-utc (string->date "2016-1-1"
>> +                                                             "~Y-~m-~d"))))
>> +               (define (valid? cert)
>> +                 (let* ((port (open-pipe* OPEN_READ openssl
>> +                                          "x509" "-enddate" "-in" cert "-noout"))
>> +                        (str  (read-line port))
>> +                        (end  (begin (close-pipe port)
>> +                                     ;; TODO: use match?
>> +                                     (cadr (string-split str #\=)))))
>
> Why not use ‘match’, indeed.  :-)  No big deal though.
>
>> +                   (time>? (date->time-utc
>> +                            (string->date end "~b ~d ~H:~M:~S ~Y")) recent)))
>> +
>> +               (define (import-cert cert)
>> +                 (format #t "Importing certificate ~a\n" (basename cert))
>> +                 (let* ((port (open-pipe* OPEN_WRITE keytool
>> +                                          "-import"
>> +                                          "-alias" (basename cert)
>> +                                          "-keystore" keystore
>> +                                          "-storepass" "changeit"
>> +                                          "-file" cert)))
>> +                   (display "yes\n" port)
>> +                   (when (not (eqv? 0 (status:exit-val (close-pipe port))))
>
> Maybe (zero? (status:exit-val …)).

Okay.

>> +                     (format (current-error-port)
>> +                             "Failed to import certificate.\n"))))
>
> Rather (error "failed to import" cert) so the process stops here.

Yes, that’s better.  I changed this for testing purposes and forgot to
change it back.

>> +               ;; This is necessary because the certificate directory contains
>> +               ;; files with non-ASCII characters in their names.
>> +               (setlocale LC_ALL "en_US.utf8")
>> +               (setenv "LC_ALL" "en_US.utf8")
>> +
>> +               (for-each import-cert
>> +                         (filter valid? (find-files certs-dir "\\.pem$")))
>
> Why do we need to filter out invalid certificates?
>
> The problem I see is that the result of ‘valid?’, and thus the output of
> the build process, depends on the build time, which isn’t great.

It actually depends on the arbitrary value of “recent”, which I set to
2016-1-1, but I must admit that I don’t know if we really must filter
out invalid certs at all.  I don’t know if it is a problem if invalid
certs are part of the keystore.  Maybe it’s not an issue.

~~ Ricardo