Bug 1274948

Taking.

Those two comments are superfluous. Just leave that one comment about license and remove those two remaining.

Be consistent about spacing between section. One (or two lines) are usually used. You use 6 lines before %files and 0 lines before %prep. 
Additionally putting new lines randomly in spec make it hard to read (e.g before BR and Source0).

New line in %description counts as space. No need to put space at the beginning of line. You are missing dot at the and of sentence in description.

Please use:
  /etc/pki/pki-usgov-dod-cacerts 
as directory name.

> %config(noreplace) /etc/pki/usgov_dod/cacerts/
This is not config - despite being in /etc/ which is usually for config. If you update some cert you want to overwrite it on user machine.
So please omit %config(noreplace).

Some certificates (e.g. DOD_CA-18-32-58468.pem) contain outdated certificates. What is the reason for including such files?

I can remove the extra comments of why it's public domain out easily.

OK I see your point about spacing, I did have haphazard spacing that was made worse by sed usage in https://github.com/pollei/fedora-rpm-specs/blob/master/make_git_spec.sh . I'll change the tspec and change the sed .

I'll fix the description, and use /etc/pki/pki-usgov-dod-cacerts without noreplace.

Expired certs can still be used in the process of checking old signatures on files and email. A lot of these expired certs are really bad as they use rsa1024 instead of rsa2048 or better, and they use sha1 not sha256 or better. So they are included only for completeness not as endorsement.

The newer certs use rsa2048, but still use sha1.
http://news.netcraft.com/archives/2016/01/08/us-military-still-shackled-to-outdated-dod-pki-infrastructure.html
http://tech.slashdot.org/story/15/10/27/0230228/us-military-websites-still-relying-on-sha-1
http://news.netcraft.com/archives/2015/10/26/u-s-military-cyber-security-fails-to-make-the-grade.html
http://news.netcraft.com/archives/2014/02/04/nist-continues-using-sha-1-algorithm-after-banning-it.html

So something to watch is that some of the certs are future dated and will fingers-crossed be still-born.

http://www.pcworld.com/article/2877672/the-end-for-1024bit-ssl-certificates-is-near-mozilla-kills-a-few-more.html
https://blog.mozilla.org/security/2015/01/28/phase-2-phasing-out-certificates-with-1024-bit-rsa-keys/
https://blog.mozilla.org/security/2015/10/20/continuing-to-phase-out-sha-1-certificates/
http://tech.slashdot.org/story/15/11/05/2332206/microsoft-follows-mozilla-in-considering-early-ban-on-sha-1-certificates by 2016-06

It's actually because these certs suck so much that they have to be packaged separately and not used by default. Ideally DOD would update their certs to use acceptable cryptographic standards and use "Name Constraints" . Then they could be properly included in firefox CA list by default.

https://tools.ietf.org/html/rfc5280#section-4.2.1.10
Internet X.509 PKI Certificate -- Name Constraints
ASN1 OID 2.5.29.30

I'm in the middle of a few things, but I'll have new version by tomorrow.
Thanks for your review.

spollei's scratch build of pki-usgov-dod-cacerts-0.0.5-1.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12508401

Spec URL: https://github.com/pollei/fedora-rpm-specs/blob/cf3013fd5ad6e63053bfc15ab991debefdeaff7e/pki-usgov-dod-cacerts.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/247/12510247/pki-usgov-dod-cacerts-0.0.6-2.fc23.src.rpm
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/spollei/dod_firefox_cfg/fedora-23-x86_64/00152960-pki-usgov-dod-cacerts/pki-usgov-dod-cacerts-0.0.6-2.fc23.src.rpm

Fixed the issues mentioned except I couldn't bare to use /etc/pki/pki-usgov-dod-cacerts/cacerts ; too superfluously and excessively redundant .

Also made a few changes to it so that it works from epel6 to rawhide .

(In reply to Stephen J Pollei from comment #4)
> Fixed the issues mentioned except I couldn't bare to use
> /etc/pki/pki-usgov-dod-cacerts/cacerts ; too superfluously and excessively
> redundant .

You could not? This error (or warning?) comes from where?

>%global commit0 8dc419c5644fc7305f757ec571406f5b2e0a96af
>%global shortcommit0 %(c=%{commit0}; echo ${c:0:7})

It is good habit to declare macros at the very top of the file.

># This package is security sensitive,
>#   certs are used to authenticate military websites
># security.org

I'm not sure if my comment about comments were clear.
I would remove this, but leave the comment why the license is public domain.

You are missing 
  BuildRequires: perl
needed for kcs7_split.pl
While it is at the end tranistively loaded via other BR it should be there.

Otherwise it looks good and once we settle on that /etc/pki path I think this can be approved.

spollei's scratch build of pki-usgov-dod-cacerts-0.0.6-3.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12517170

version 0.0.6-3 with changes requested
 
http://koji.fedoraproject.org/koji/taskinfo?taskID=12517171
https://copr.fedoraproject.org/coprs/spollei/dod_firefox_cfg/build/153039/

Spec URL: https://raw.githubusercontent.com/pollei/fedora-rpm-specs/bac0459b4cfdc65253bac6cede197c7d717dcfd6/pki-usgov-dod-cacerts.spec

Also still need sponsor.

Here comes full formal review. I found three more minor things.

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[-]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: License field in the package spec file matches the actual license.
[x]: Package requires other packages for directories it uses.
[!]: Package must own all directories that it creates.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[x]: %config files are marked noreplace or the reason is justified.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[!]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane.
[x]: Package functions as described.
[?]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[!]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).


Issues:
You are missing:
  %dir %{_sysconfdir}/pki/pki-usgov-dod-cacerts
so the directory is not owned by your package.

In %install and %files you should use %{_sysconfdir} macro instead of /etc/

In %install you should use "cp -a" to preserve original timestamp of files.

spollei's scratch build of pki-usgov-dod-cacerts-0.0.6-4.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12521299

OK I made a 0.0.6-4 with those changes

https://raw.githubusercontent.com/pollei/fedora-rpm-specs/a57785d3113eb7dc84865218298ee14778fb548f/pki-usgov-dod-cacerts.spec

https://copr.fedoraproject.org/coprs/spollei/dod_firefox_cfg/build/153177/
http://koji.fedoraproject.org/koji/taskinfo?taskID=12521300
copr and koji ran

I tried running my own review but it failed
`fedora-review --prebuilt -rn pki-usgov-dod-cacerts-0.0.6-4.fc23.src.rpm --define DISTTAG=f23`

ERROR: 'Multiple srpms found for pki-usgov-dod-cacerts'

This:
fedora-review -rn pki-usgov-dod-cacerts-0.0.6-4.fc24.src.rpm
works for me.

APPROVED

Now follow the process. 
I sponsored you into packager group.
If you ever have questions and you need some guidenance about fedora processes or infrastructure do not hesitate to contact me directly.

That's great news. I do need to learn a lot more about the process, I've never used fedpkg or bodhi before.

https://fedoraproject.org/wiki/Join_the_package_collection_maintainers
I think I can carefully follow the instructions.
1) https://admin.fedoraproject.org/pkgdb/ "Request new package"
2) mkdir -p ~/devel/fedora-scm ; cd ~/devel/fedora-scm ; fedpkg clone pki-usgov-dod-cacerts
3) fedpkg import foo.src.rpm ; git commit -m "Initial import (#1274948)." ; git push ; fedpkg build

I know almost nothing about bodhi, but I shouldn't need it to get it into rawhide. I also don't think I need comps.xml as it's a niche package.

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/pki-usgov-dod-cacerts

pki-usgov-dod-cacerts-0.0.6-4.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-1a87db5f8e

pki-usgov-dod-cacerts-0.0.6-4.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-1a87db5f8e

pki-usgov-dod-cacerts-0.0.6-4.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.