Message ID | 20160718115941.17707-4-ricardo.wurmus@mdc-berlin.de |
---|---|
State | New |
Headers | show |
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? Too bad IceTea’s build system doesn’t take care of that. > + (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 …)). > + (format (current-error-port) > + "Failed to import certificate.\n")))) Rather (error "failed to import" cert) so the process stops here. > + ;; 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. I would prefer to unconditionally install all the certificates if that doesn’t cause any problems. WDYT? Thank you for working on it! Ludo’.
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
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? > > Too bad IceTea’s build system doesn’t take care of that. > >> + (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 …)). > >> + (format (current-error-port) >> + "Failed to import certificate.\n")))) > > Rather (error "failed to import" cert) so the process stops here. > >> + ;; 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. > > I would prefer to unconditionally install all the certificates if that > doesn’t cause any problems. WDYT? I removed the validation (because I expect certs to be validated at runtime). I also added a comment explaining why this is needed and made the suggested changes. (I pushed from my workstation without signing key, because I forgot that I normally push from my laptop. Sorry, won’t happen again! Key replacement is on my list, and then I’ll get myself a subkey for the office workstation.) Thanks for the review! ~~ Ricardo
Hello, Ricardo! Icedtea@1 in master now fails to build in the install-keystore phase. http://hydra.gnu.org:3000/build/1309224 http://hydra.gnu.org:3000/build/1308950 Could you have a look, please? Andreas
Andreas Enge <andreas@enge.fr> writes: > Hello, Ricardo! > > Icedtea@1 in master now fails to build in the install-keystore phase. > http://hydra.gnu.org:3000/build/1309224 > http://hydra.gnu.org:3000/build/1308950 > Could you have a look, please? Hmm, that’s strange. I ran “guix build icedtea” after removing the validation filter and built out all three versions of icedtea before pushing this. I don’t have the very same version of the “keytool” binary on my machine right now (with the very same version of nss-certs as on hydra), but in principle this works without errors: ~~~~~~~~~~~~ /gnu/store/r63vag0814nz79xr9g2ph6fvhq5xp2f3-icedtea-2.6.6/bin/keytool \ -import \ -alias ACCVRAIZ1:2.8.94.195.183.166.67.127.164.224.pem \ -keystore /tmp/keystore \ -storepass changeit \ -file /gnu/store/lp7s9x1llgw1rc675yvslxsnpcyy05ld-nss-certs-3.23/etc/ssl/certs/ACCVRAIZ1:2.8.94.195.183.166.67.127.164.224.pem … Trust this certificate? [no]: yes Certificate was added to keystore ~~~~~~~~~~~~ The pem file looks like a valid X.509 certificate to me. I cannot build icedtea@1 on my machine right now as I’m traveling, but I just started a build remotely on my workstation in the office and it failed. I used to have an additional stripping phase that I removed at some point. As I continued to refine the new phase I must have used the cached build of icedtea@1 without ever rebuilding it. Sorry! The keytool from icedtea@1 doesn’t like this certificate. My hunch is that we may need to remove comments from the certificate files, only leaving the certificate block. I’ll fix this as soon as I can. ~~ Ricardo
diff --git a/gnu/packages/java.scm b/gnu/packages/java.scm index faa6e5b..95e273e 100644 --- a/gnu/packages/java.scm +++ b/gnu/packages/java.scm @@ -30,6 +30,7 @@ #:use-module (gnu packages autotools) #:use-module (gnu packages base) #:use-module (gnu packages bash) + #:use-module (gnu packages certs) #:use-module (gnu packages cpio) #:use-module (gnu packages cups) #:use-module (gnu packages compression) @@ -47,6 +48,7 @@ #:use-module (gnu packages pkg-config) #:use-module (gnu packages perl) #:use-module (gnu packages mit-krb5) + #:use-module (gnu packages tls) #:use-module (gnu packages xml) #:use-module (gnu packages xorg) #:use-module (gnu packages zip) @@ -262,7 +264,8 @@ build process and its dependencies, whereas Make uses Makefile format.") #:modules ((guix build utils) (guix build gnu-build-system) (ice-9 popen) - (ice-9 rdelim)) + (ice-9 rdelim) + (srfi srfi-19)) #:configure-flags (let* ((gcjdir (assoc-ref %build-inputs "gcj")) @@ -521,7 +524,58 @@ build process and its dependencies, whereas Make uses Makefile format.") (jdk (assoc-ref outputs "jdk"))) (copy-recursively "openjdk.build/docs" doc) (copy-recursively "openjdk.build/j2re-image" jre) - (copy-recursively "openjdk.build/j2sdk-image" jdk))))))) + (copy-recursively "openjdk.build/j2sdk-image" jdk)))) + (add-after 'install 'install-keystore + (lambda* (#:key inputs outputs #:allow-other-keys) + (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 #\=))))) + (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)))) + (format (current-error-port) + "Failed to import certificate.\n")))) + + ;; 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$"))) + (mkdir-p (string-append (assoc-ref outputs "out") + "/lib/security")) + (mkdir-p (string-append (assoc-ref outputs "jdk") + "/jre/lib/security")) + (install-file keystore + (string-append (assoc-ref outputs "out") + "/lib/security")) + (install-file keystore + (string-append (assoc-ref outputs "jdk") + "/jre/lib/security")) + #t)))))) (native-inputs `(("ant" ,ant) ("alsa-lib" ,alsa-lib) @@ -544,6 +598,7 @@ build process and its dependencies, whereas Make uses Makefile format.") ("libxslt" ,libxslt) ;for xsltproc ("mit-krb5" ,mit-krb5) ("nss" ,nss) + ("nss-certs" ,nss-certs) ("libx11" ,libx11) ("libxcomposite" ,libxcomposite) ("libxt" ,libxt) @@ -554,6 +609,7 @@ build process and its dependencies, whereas Make uses Makefile format.") ("libjpeg" ,libjpeg) ("libpng" ,libpng) ("giflib" ,giflib) + ("openssl" ,openssl) ("perl" ,perl) ("procps" ,procps) ;for "free", even though I'm not sure we should use it ("openjdk6-src" @@ -789,6 +845,9 @@ build process and its dependencies, whereas Make uses Makefile format.") (delete 'patch-paths) (delete 'set-additional-paths) (delete 'patch-patches) + ;; FIXME: This phase is needed but fails with this version of + ;; IcedTea. + (delete 'install-keystore) (replace 'install (lambda* (#:key outputs #:allow-other-keys) (let ((doc (string-append (assoc-ref outputs "doc")
From: Ricardo Wurmus <rekado@elephly.net> * gnu/packages/java.scm (icedtea-6)[arguments]: Add phase "install-keystore". [native-inputs]: Add nss-certs and openssl. --- gnu/packages/java.scm | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-)