Page MenuHomeFreeBSD

Generate /etc/services from the IANA registry (plus local mods)
Needs ReviewPublic

Authored by vangyzen on Sep 10 2018, 9:47 PM.

Details

Reviewers
dab
Summary

Generate /etc/services from

https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xml

and a locally maintained local.xml.

This required creating the initial version of local.xml, which was also automated. (The code for this can be removed when we are sure it is no longer needed.) All of our current /etc/services names still exist, although the primary name and comment for a given number/protocol tuple are determined by the IANA registry by default, so they might have changed. The primary name can be overridden by local.xml, which we have done for port 2049.

/etc/services grows from 2,367 lines and 2,501 names to 11,538 lines and 12,005 names. /var/db/services.db grows from 2 MB to 4.2 MB.

The service-names-port-numbers.xml file from IANA is omitted from this review in order to make it more practical. Whether this file should be committed is an open question.

Test Plan

I manually read through the generated file. It looks reasonable.

I parsed the file with services_count.c. It parsed 12,005 names, the same number printed by services_parser.py when creating the file. I used the same program to benchmark the parser. Time grew linearly as the number of lines, and memory usage (RSS) grew by 1%.

I ran services_mkdb -q. It seemed happy.

Diff Detail

Lint
Lint ErrorsExcuse: foo
SeverityLocationCodeMessage
Errorusr.sbin/services_mkdb/services_parser.py:30E265PEP8 E265
Errorusr.sbin/services_mkdb/services_parser.py:35E302PEP8 E302
Errorusr.sbin/services_mkdb/services_parser.py:42E301PEP8 E301
Errorusr.sbin/services_mkdb/services_parser.py:44E301PEP8 E301
Errorusr.sbin/services_mkdb/services_parser.py:46E301PEP8 E301
Errorusr.sbin/services_mkdb/services_parser.py:48E301PEP8 E301
Errorusr.sbin/services_mkdb/services_parser.py:50E301PEP8 E301
Errorusr.sbin/services_mkdb/services_parser.py:52E301PEP8 E301
Errorusr.sbin/services_mkdb/services_parser.py:54E301PEP8 E301
Errorusr.sbin/services_mkdb/services_parser.py:56E301PEP8 E301
Errorusr.sbin/services_mkdb/services_parser.py:74E302PEP8 E302
Errorusr.sbin/services_mkdb/services_parser.py:81E501PEP8 E501
Errorusr.sbin/services_mkdb/services_parser.py:95E302PEP8 E302
Errorusr.sbin/services_mkdb/services_parser.py:97E128PEP8 E128
Errorusr.sbin/services_mkdb/services_parser.py:99E302PEP8 E302
Errorusr.sbin/services_mkdb/services_parser.py:149E128PEP8 E128
Errorusr.sbin/services_mkdb/services_parser.py:153E128PEP8 E128
Errorusr.sbin/services_mkdb/services_parser.py:160E302PEP8 E302
Errorusr.sbin/services_mkdb/services_parser.py:189E302PEP8 E302
Warningusr.sbin/services_mkdb/services:1216SPELL1Possible Spelling Mistake
Warningusr.sbin/services_mkdb/services:1217SPELL1Possible Spelling Mistake
Warningusr.sbin/services_mkdb/services:9534SPELL1Possible Spelling Mistake
Warningusr.sbin/services_mkdb/services:9535SPELL1Possible Spelling Mistake
Unit
No Unit Test Coverage
Build Status
Buildable 19511
Build 19103: arc lint + arc unit

Event Timeline

This review replaces D17106. This review is more practical because it omits the service-names-port-numbers.xml file from IANA. Whether this file should be committed is an open question. This review also omits whitespace changes in the generated services file. These will be committed separately, before the other changes.

  • Elide useless comments; avoid trailing whitespace

Two annoying questions:

  1. The Wayback machine shows me that IANA sometimes removes a port assignment. Sadly, I haven't been able to find any official historical information. What will FreeBSD's policy regarding removing assignments be?
  2. I've seen mailing list comments to the effect that /etc/services should only contain entries needed by the base system and ports. For all the obscure stuff, one can use the database in security/nmap. What's your opinion about that?
usr.sbin/services_mkdb/services
867

Do you think it's worth trying to wrap long lines like this?

3470

nfsd is popular enough that it should probably be the default for these ports, not an alias.

7386

I would say that lockd should also be the default here.

usr.sbin/services_mkdb/services_parser.py
133

What will mergemaster do if somebody edits this file on an installed system?

Two annoying questions:

  1. The Wayback machine shows me that IANA sometimes removes a port assignment. Sadly, I haven't been able to find any official historical information. What will FreeBSD's policy regarding removing assignments be?

I doubt there has ever been such a policy. By default, we will remove it on the next update. I think this de facto policy will be sufficient. Administrators who need to keep the assignment can simply add it to this file on their systems.

  1. I've seen mailing list comments to the effect that /etc/services should only contain entries needed by the base system and ports. For all the obscure stuff, one can use the database in security/nmap. What's your opinion about that?

I don't think the base system and ports have much bearing on defining what is useful in this file. A lot of other software (hopefully) runs on FreeBSD. A lot of humans would probably find a complete database to be useful, too. I expect that adding all of the official services will provide the most convenience for the most users, and will be a minor inconvenience for a small minority. That minority can edit the file to suit their needs.

usr.sbin/services_mkdb/services
867

Yes. Earlier today, I added it to my to-do list.

3470

I agree. I figured this would happen eventually. I'll add that capability to the script.

7386

This was added by r7685, which claims to "Upgrade to RFC1700 (IANA)". However, RFC 1700 does not mention that port.

The current registry assigns it only to npp, not lockd.

lockd on FreeBSD actually uses a non-fixed port < 1024 assigned by rpc.lockd.

lockd is not used by NFSv4.

Given all of the above, I don't see a need to change this. In fact, I don't see a need to keep "lockd" at all, but I'll save that for another day.

usr.sbin/services_mkdb/services_parser.py
133

The same thing it would do without my changes, and the same thing it does with other files in /etc. I'm not trying to be snarky, but this seems orthogonal to my changes.

Two annoying questions:

  1. The Wayback machine shows me that IANA sometimes removes a port assignment. Sadly, I haven't been able to find any official historical information. What will FreeBSD's policy regarding removing assignments be?

I doubt there has ever been such a policy. By default, we will remove it on the next update. I think this de facto policy will be sufficient. Administrators who need to keep the assignment can simply add it to this file on their systems.

In that case there a lot of things in there currently that we should remove. Unfortunately, without any kind of version control info from IANA, it's hard to tell which of our local aliases were added by us versus which of them are holdovers of things that IANA removed.

  1. I've seen mailing list comments to the effect that /etc/services should only contain entries needed by the base system and ports. For all the obscure stuff, one can use the database in security/nmap. What's your opinion about that?

I don't think the base system and ports have much bearing on defining what is useful in this file. A lot of other software (hopefully) runs on FreeBSD. A lot of humans would probably find a complete database to be useful, too. I expect that adding all of the official services will provide the most convenience for the most users, and will be a minor inconvenience for a small minority. That minority can edit the file to suit their needs.

Works for me.

usr.sbin/services_mkdb/services
7386

Good point. Sounds like removing it would be best, then.

usr.sbin/services_mkdb/services_parser.py
133

The ongoing pkgbase work is throwing a wrench into things. Mergemaster now blindly overwrites some files in /etc. It would be good to double check that this is not one of those.

  • If the description is too long, wrap it to multiple lines and print it above the service
  • Let local services set the primary name
vangyzen edited the test plan for this revision. (Show Details)
vangyzen edited the summary of this revision. (Show Details)
usr.sbin/services_mkdb/services
9975

These leading spaces are super-ugly. But I suppose they're in the IANA original, so it's reasonable to leave them in place.

11455

Also a really unfortunate use of whitespace in the original.

  • Update the documentation for local.xml.

I think I'm finished with all outstanding changes. I would be grateful for more reviews. (Thanks for your ongoing review, @asomers.)

I plan to omit the big IANA XML file from the initial commit, then follow up on a mailing list to get opinions on how to handle it.

usr.sbin/services_mkdb/services
9975

I don't see any leading spaces.

usr.sbin/services_mkdb/services
9975

You don't see seven spaces between the # and targeted on line 9976?

usr.sbin/services_mkdb/services
9975

Now I see it, down on line 10164.

  • Collapse multiple spaces into a single space.

I did not review closely but am supportive of the direction taken

usr.sbin/services_mkdb/services_parser.py
37

Would anyone else care to review? I promise it won't crash your browser this time.

usr.sbin/services_mkdb/services_parser.py
37

Thanks. I hadn't heard of that.

@vangyzen -- Did you notice this:

These changes have lint problems.
These changes have failed to build.
usr.sbin/services_mkdb/local.xml
69

If <description> is empty,as here, must it be present in the record? If it could be omitted the specification would be just a bit more compact.

72

"null" and "sink" seem to be aliases and both have multiple protocols. It might be nice to all a single entry to specify multiple names and/or protocols for this (I think fairly common) usage to make the specification more compact. What do you think?

Something like:

<record>
  <names>null,sink</names>
  <protocols>sctp,tcp,udp</protocols>
  <description/>
  <number>9</number>
</record>
usr.sbin/services_mkdb/services
874

s/priviledged/privileged/

955

Why the change in comment from "Brunhoff remote file system" to "rfs server"?

2427–2428

Should the "Prospero Directory Service non-priv" comment be preserved here for the alias (same on next line, too).

2435

The "cygnus bug tracker" comment was lost here for the "support" alias.

2778

The "Layer 2 Tunnelling Protocol" comment was lost here for l2tp.

2823

Lost the "Point-to-point tunnelling protocol" comment here.

3350–3351

Lost the "MHSnet system" comment for the mshnet alias here.
Also, is it "MHSnet" or "MSHnet"?

3377

Lost "GNU finger" comment for fingerd.

IANA has indeed removed some services: I think people can de-register services they don't use/develop anymore. We should remove at least Kerberos IV and netatalk, both are not in the IANA. Also IANA lists first tcp, udp and lastly sctp so perhaps its better to jeep things consistent.

I tried to do a complete update by hand, but mjg@ complained the database was too big, wrt Ubuntu and we should be much more minimalistic. In all honesty, a bunch of the services correspond to licensing schemes for proprietary applications, anonymous startups and dead services. I ended up doing what I consider is an acceptable compromise in D23671.